Skip to content

Commit afaabdd

Browse files
committed
cacheprovider: fix some files in packages getting lost from --lf
--lf has an optimization where it skips collecting Modules (python files) which don't contain failing tests. The optimization works by getting the paths of all cached failed tests and skipping the collection of Modules whose path is not included in that list. In pytest, Package nodes are Module nodes with the fspath being the file `<package dir>/__init__.py`. Since it's a Module the logic above triggered for it, and because it's an `__init__.py` file which is unlikely to have any failing tests in it, it is skipped, which causes its entire directory to be skipped, including any Modules inside it with failing tests. Fix by special-casing Packages to never filter. This means entire Packages are never filtered, the Modules themselves are always checked. It is reasonable to consider an optimization which does filter entire packages bases on parent paths etc. but this wouldn't actually save any real work so is really not worth it.
1 parent f453460 commit afaabdd

File tree

3 files changed

+37
-1
lines changed

3 files changed

+37
-1
lines changed

changelog/7758.bugfix.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed an issue where some files in packages are getting lost from ``--lf`` even though they contain tests that failed. Regressed in pytest 5.4.0.

src/_pytest/cacheprovider.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from _pytest.fixtures import FixtureRequest
2929
from _pytest.main import Session
3030
from _pytest.python import Module
31+
from _pytest.python import Package
3132
from _pytest.reports import TestReport
3233

3334

@@ -232,7 +233,10 @@ def __init__(self, lfplugin: "LFPlugin") -> None:
232233
def pytest_make_collect_report(
233234
self, collector: nodes.Collector
234235
) -> Optional[CollectReport]:
235-
if isinstance(collector, Module):
236+
# Packages are Modules, but _last_failed_paths only contains
237+
# test-bearing paths and doesn't try to include the paths of their
238+
# packages, so don't filter them.
239+
if isinstance(collector, Module) and not isinstance(collector, Package):
236240
if Path(str(collector.fspath)) not in self.lfplugin._last_failed_paths:
237241
self.lfplugin._skipped_files += 1
238242

testing/test_cacheprovider.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import pytest
99
from _pytest.config import ExitCode
10+
from _pytest.pytester import Pytester
1011
from _pytest.pytester import Testdir
1112

1213
pytest_plugins = ("pytester",)
@@ -982,6 +983,36 @@ def test_pass(): pass
982983
)
983984
assert result.ret == 0
984985

986+
def test_packages(self, pytester: Pytester) -> None:
987+
"""Regression test for #7758.
988+
989+
The particular issue here was that Package nodes were included in the
990+
filtering, being themselves Modules for the __init__.py, even if they
991+
had failed Modules in them.
992+
993+
The tests includes a test in an __init__.py file just to make sure the
994+
fix doesn't somehow regress that, it is not critical for the issue.
995+
"""
996+
pytester.makepyfile(
997+
**{
998+
"__init__.py": "",
999+
"a/__init__.py": "def test_a_init(): assert False",
1000+
"a/test_one.py": "def test_1(): assert False",
1001+
"b/__init__.py": "",
1002+
"b/test_two.py": "def test_2(): assert False",
1003+
},
1004+
)
1005+
pytester.makeini(
1006+
"""
1007+
[pytest]
1008+
python_files = *.py
1009+
"""
1010+
)
1011+
result = pytester.runpytest()
1012+
result.assert_outcomes(failed=3)
1013+
result = pytester.runpytest("--lf")
1014+
result.assert_outcomes(failed=3)
1015+
9851016

9861017
class TestNewFirst:
9871018
def test_newfirst_usecase(self, testdir):

0 commit comments

Comments
 (0)