From c76ae74bd79e8db9ff08996a86bc4f1cfdc1be48 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 30 May 2023 09:29:19 +0300 Subject: [PATCH 1/3] cacheprovider: fix file-skipping functionality across packages Continuation of fc538c5766a1c67bfcd704288279ceac5e20070a. Fixes #11054 again. --- src/_pytest/cacheprovider.py | 26 +++++++++++++++++++------- testing/test_cacheprovider.py | 27 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index 84ca2c688..3d4fb1d6c 100755 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -226,10 +226,18 @@ class LFPluginCollWrapper: # Sort any lf-paths to the beginning. lf_paths = self.lfplugin._last_failed_paths + # Use stable sort to priorize last failed. + def sort_key(node: Union[nodes.Item, nodes.Collector]) -> bool: + # Package.path is the __init__.py file, we need the directory. + if isinstance(node, Package): + path = node.path.parent + else: + path = node.path + return path in lf_paths + res.result = sorted( res.result, - # use stable sort to priorize last failed - key=lambda x: x.path in lf_paths, + key=sort_key, reverse=True, ) return @@ -272,9 +280,8 @@ class LFPluginCollSkipfiles: def pytest_make_collect_report( self, collector: nodes.Collector ) -> Optional[CollectReport]: - # 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. + # Packages are Modules, but we only want to skip test-bearing Modules, + # so don't filter Packages. if isinstance(collector, Module) and not isinstance(collector, Package): if collector.path not in self.lfplugin._last_failed_paths: self.lfplugin._skipped_files += 1 @@ -305,9 +312,14 @@ class LFPlugin: ) def get_last_failed_paths(self) -> Set[Path]: - """Return a set with all Paths()s of the previously failed nodeids.""" + """Return a set with all Paths of the previously failed nodeids and + their parents.""" rootpath = self.config.rootpath - result = {rootpath / nodeid.split("::")[0] for nodeid in self.lastfailed} + result = set() + for nodeid in self.lastfailed: + path = rootpath / nodeid.split("::")[0] + result.add(path) + result.update(path.parents) return {x for x in result if x.exists()} def pytest_report_collectionfinish(self) -> Optional[str]: diff --git a/testing/test_cacheprovider.py b/testing/test_cacheprovider.py index 7c6606e2b..cb8011036 100644 --- a/testing/test_cacheprovider.py +++ b/testing/test_cacheprovider.py @@ -854,6 +854,33 @@ class TestLastFailed: ] ) + def test_lastfailed_skip_collection_with_nesting(self, pytester: Pytester) -> None: + """Check that file skipping works even when the file with failures is + nested at a different level of the collection tree.""" + pytester.makepyfile( + **{ + "test_1.py": """ + def test_1(): pass + """, + "pkg/__init__.py": "", + "pkg/test_2.py": """ + def test_2(): assert False + """, + } + ) + # first run + result = pytester.runpytest() + result.stdout.fnmatch_lines(["collected 2 items", "*1 failed*1 passed*"]) + # second run - test_1.py is skipped. + result = pytester.runpytest("--lf") + result.stdout.fnmatch_lines( + [ + "collected 1 item", + "run-last-failure: rerun previous 1 failure (skipped 1 file)", + "*= 1 failed in *", + ] + ) + def test_lastfailed_with_known_failures_not_being_selected( self, pytester: Pytester ) -> None: From 3de43e51025a75369c2560623c5e7fc50c0b5137 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 2 Jun 2023 16:06:35 +0300 Subject: [PATCH 2/3] python: remove redundant methods from Package They are already inherited exactly the same from FSCollector. --- src/_pytest/python.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 94f000939..3b1253cf4 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -57,7 +57,6 @@ from _pytest.config import ExitCode from _pytest.config import hookimpl from _pytest.config.argparsing import Parser from _pytest.deprecated import check_ispytest -from _pytest.deprecated import FSCOLLECTOR_GETHOOKPROXY_ISINITPATH from _pytest.deprecated import INSTANCE_COLLECTOR from _pytest.deprecated import NOSE_SUPPORT_METHOD from _pytest.fixtures import FuncFixtureInfo @@ -700,14 +699,6 @@ class Package(Module): func = partial(_call_with_optional_argument, teardown_module, self.obj) self.addfinalizer(func) - def gethookproxy(self, fspath: "os.PathLike[str]"): - warnings.warn(FSCOLLECTOR_GETHOOKPROXY_ISINITPATH, stacklevel=2) - return self.session.gethookproxy(fspath) - - def isinitpath(self, path: Union[str, "os.PathLike[str]"]) -> bool: - warnings.warn(FSCOLLECTOR_GETHOOKPROXY_ISINITPATH, stacklevel=2) - return self.session.isinitpath(path) - def _recurse(self, direntry: "os.DirEntry[str]") -> bool: if direntry.name == "__pycache__": return False From fda8024622df64802f9670bfe2ba658b40f6674d Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 2 Jun 2023 11:41:36 +0300 Subject: [PATCH 3/3] cacheprovider: make file-skipping work with any File, not just Modules No reason for `--lf`'s whole-file-skipping feature to not for for non-Python files. Fix #11068. --- changelog/11068.bugfix.rst | 1 + src/_pytest/cacheprovider.py | 8 ++++---- testing/conftest.py | 2 +- testing/test_cacheprovider.py | 22 ++++++++++++++++++++++ 4 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 changelog/11068.bugfix.rst diff --git a/changelog/11068.bugfix.rst b/changelog/11068.bugfix.rst new file mode 100644 index 000000000..45cdb105f --- /dev/null +++ b/changelog/11068.bugfix.rst @@ -0,0 +1 @@ +Fixed the ``--last-failed`` whole-file skipping functionality ("skipped N files") for :ref:`non-python test files `. diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index 3d4fb1d6c..855716d81 100755 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -27,7 +27,7 @@ from _pytest.deprecated import check_ispytest from _pytest.fixtures import fixture from _pytest.fixtures import FixtureRequest from _pytest.main import Session -from _pytest.python import Module +from _pytest.nodes import File from _pytest.python import Package from _pytest.reports import TestReport @@ -242,7 +242,7 @@ class LFPluginCollWrapper: ) return - elif isinstance(collector, Module): + elif isinstance(collector, File): if collector.path in self.lfplugin._last_failed_paths: out = yield res = out.get_result() @@ -280,9 +280,9 @@ class LFPluginCollSkipfiles: def pytest_make_collect_report( self, collector: nodes.Collector ) -> Optional[CollectReport]: - # Packages are Modules, but we only want to skip test-bearing Modules, + # Packages are Files, but we only want to skip test-bearing Files, # so don't filter Packages. - if isinstance(collector, Module) and not isinstance(collector, Package): + if isinstance(collector, File) and not isinstance(collector, Package): if collector.path not in self.lfplugin._last_failed_paths: self.lfplugin._skipped_files += 1 diff --git a/testing/conftest.py b/testing/conftest.py index a83552fd2..8e77fcae5 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -105,7 +105,7 @@ def tw_mock(): @pytest.fixture -def dummy_yaml_custom_test(pytester: Pytester): +def dummy_yaml_custom_test(pytester: Pytester) -> None: """Writes a conftest file that collects and executes a dummy yaml test. Taken from the docs, but stripped down to the bare minimum, useful for diff --git a/testing/test_cacheprovider.py b/testing/test_cacheprovider.py index cb8011036..6f3cccbf1 100644 --- a/testing/test_cacheprovider.py +++ b/testing/test_cacheprovider.py @@ -1085,6 +1085,28 @@ class TestLastFailed: result = pytester.runpytest("--lf") result.assert_outcomes(failed=3) + def test_non_python_file_skipped( + self, + pytester: Pytester, + dummy_yaml_custom_test: None, + ) -> None: + pytester.makepyfile( + **{ + "test_bad.py": """def test_bad(): assert False""", + }, + ) + result = pytester.runpytest() + result.stdout.fnmatch_lines(["collected 2 items", "* 1 failed, 1 passed in *"]) + + result = pytester.runpytest("--lf") + result.stdout.fnmatch_lines( + [ + "collected 1 item", + "run-last-failure: rerun previous 1 failure (skipped 1 file)", + "* 1 failed in *", + ] + ) + class TestNewFirst: def test_newfirst_usecase(self, pytester: Pytester) -> None: