Skip to content

Commit d18cb96

Browse files
committed
assertion/rewrite: fix internal error on collection error due to decorated function
For decorated functions, the lineno of the FunctionDef AST node points to the `def` line, not to the first decorator line. On the other hand, in code objects, the `co_firstlineno` points to the first decorator line. Assertion rewriting inserts some imports to code it rewrites. The imports are inserted at the lineno of the first statement in the AST. In turn, the code object compiled from the rewritten AST uses the lineno of the first statement (which is the first inserted import). This means that given a module like this, ```py @foo @bar def baz(): pass ``` the lineno of the code object without assertion rewriting (`--assertion=plain`) is 1, but with assertion rewriting it is 3. And *this* causes some issues for the exception repr when e.g. the decorator line is invalid and raises during collection. The code becomes confused and crashes with INTERNALERROR> File "_pytest/_code/code.py", line 638, in get_source INTERNALERROR> lines.append(space_prefix + source.lines[line_index].strip()) INTERNALERROR> IndexError: list index out of range Fix it by special casing decorators. Maybe there are other cases like this but off hand I can't think of another Python construct where the lineno of the item would be after its first line, and this is the only such issue we have had reported.
1 parent 35350e1 commit d18cb96

File tree

3 files changed

+23
-1
lines changed

3 files changed

+23
-1
lines changed

changelog/4984.bugfix.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fixed an internal error crash with ``IndexError: list index out of range`` when
2+
collecting a module which starts with a decorated function, the decorator
3+
raises, and assertion rewriting is enabled.

src/_pytest/assertion/rewrite.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,12 @@ def run(self, mod: ast.Module) -> None:
695695
else:
696696
break
697697
pos += 1
698-
lineno = item.lineno
698+
# Special case: for a decorated function, set the lineno to that of the
699+
# first decorator, not the `def`. Issue #4984.
700+
if isinstance(item, ast.FunctionDef) and item.decorator_list:
701+
lineno = item.decorator_list[0].lineno
702+
else:
703+
lineno = item.lineno
699704
imports = [
700705
ast.Import([alias], lineno=lineno, col_offset=0) for alias in aliases
701706
]

testing/test_collection.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,3 +1393,17 @@ def test_modules_not_importable_as_side_effect(self, testdir):
13931393
"* 1 failed in *",
13941394
]
13951395
)
1396+
1397+
1398+
def test_does_not_crash_on_error_from_decorated_function(testdir: Testdir) -> None:
1399+
"""Regression test for an issue around bad exception formatting due to
1400+
assertion rewriting mangling lineno's (#4984)."""
1401+
testdir.makepyfile(
1402+
"""
1403+
@pytest.fixture
1404+
def a(): return 4
1405+
"""
1406+
)
1407+
result = testdir.runpytest()
1408+
# Not INTERNAL_ERROR
1409+
assert result.ret == ExitCode.INTERRUPTED

0 commit comments

Comments
 (0)