Skip to content

Commit

Permalink
cacheprovider: fix some files in packages getting lost from --lf
Browse files Browse the repository at this point in the history
--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.
  • Loading branch information
bluetech committed Oct 19, 2020
1 parent f453460 commit afaabdd
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/7758.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +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.
6 changes: 5 additions & 1 deletion src/_pytest/cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from _pytest.fixtures import FixtureRequest
from _pytest.main import Session
from _pytest.python import Module
from _pytest.python import Package
from _pytest.reports import TestReport


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

Expand Down
31 changes: 31 additions & 0 deletions testing/test_cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import pytest
from _pytest.config import ExitCode
from _pytest.pytester import Pytester
from _pytest.pytester import Testdir

pytest_plugins = ("pytester",)
Expand Down Expand Up @@ -982,6 +983,36 @@ def test_pass(): pass
)
assert result.ret == 0

def test_packages(self, pytester: Pytester) -> None:
"""Regression test for #7758.
The particular issue here was that Package nodes were included in the
filtering, being themselves Modules for the __init__.py, even if they
had failed Modules in them.
The tests includes a test in an __init__.py file just to make sure the
fix doesn't somehow regress that, it is not critical for the issue.
"""
pytester.makepyfile(
**{
"__init__.py": "",
"a/__init__.py": "def test_a_init(): assert False",
"a/test_one.py": "def test_1(): assert False",
"b/__init__.py": "",
"b/test_two.py": "def test_2(): assert False",
},
)
pytester.makeini(
"""
[pytest]
python_files = *.py
"""
)
result = pytester.runpytest()
result.assert_outcomes(failed=3)
result = pytester.runpytest("--lf")
result.assert_outcomes(failed=3)


class TestNewFirst:
def test_newfirst_usecase(self, testdir):
Expand Down

0 comments on commit afaabdd

Please sign in to comment.