diff --git a/changelog/15045.bugfix.rst b/changelog/15045.bugfix.rst new file mode 100644 index 00000000000..57f4cce1a2d --- /dev/null +++ b/changelog/15045.bugfix.rst @@ -0,0 +1,2 @@ +Fixed ``--last-failed`` silently skipping custom items whose names contain +brackets. diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index 5d4a528077d..f806589f794 100644 --- a/src/_pytest/cacheprovider.py +++ b/src/_pytest/cacheprovider.py @@ -284,7 +284,7 @@ def sort_key(node: nodes.Item | nodes.Collector) -> bool: # Only filter with known failures. if not self._collected_at_least_one_failure: - if not any(x.id in lastfailed for x in result): + if not any(str(x.id) in lastfailed for x in result): return res self.lfplugin.config.pluginmanager.register( LFPluginCollSkipfiles(self.lfplugin), "lfplugin-collskip" @@ -295,7 +295,7 @@ def sort_key(node: nodes.Item | nodes.Collector) -> bool: result[:] = [ x for x in result - if x.id in lastfailed + if str(x.id) in lastfailed # Include any passed arguments (not trivial to filter). or session.isinitpath(x.path) # Keep all sub-collectors. @@ -329,9 +329,8 @@ def __init__(self, config: Config) -> None: active_keys = "lf", "failedfirst" self.active = any(config.getoption(key) for key in active_keys) assert config.cache - self.lastfailed: dict[NodeId, bool] = { - NodeId.parse(k): v - for k, v in config.cache.get("cache/lastfailed", {}).items() + self.lastfailed: dict[str, bool] = { + str(k): v for k, v in config.cache.get("cache/lastfailed", {}).items() } self._previously_failed_count: int | None = None self._report_status: str | None = None @@ -349,7 +348,7 @@ def get_last_failed_paths(self) -> set[Path]: rootpath = self.config.rootpath result = set() for nodeid in self.lastfailed: - path = rootpath / nodeid.path + path = rootpath / NodeId.parse(nodeid).path result.add(path) result.update(path.parents) return {x for x in result if x.exists()} @@ -360,20 +359,21 @@ def pytest_report_collectionfinish(self) -> str | None: return None def pytest_runtest_logreport(self, report: TestReport) -> None: + report_id = str(report.id) if (report.when == "call" and report.passed) or report.skipped: - self.lastfailed.pop(report.id, None) + self.lastfailed.pop(report_id, None) elif report.failed: - self.lastfailed[report.id] = True + self.lastfailed[report_id] = True def pytest_collectreport(self, report: CollectReport) -> None: passed = report.outcome in ("passed", "skipped") if passed: - report_id = report.id + report_id = str(report.id) if report_id in self.lastfailed: self.lastfailed.pop(report_id) - self.lastfailed.update((item.id, True) for item in report.result) + self.lastfailed.update((str(item.id), True) for item in report.result) else: - self.lastfailed[report.id] = True + self.lastfailed[str(report.id)] = True @hookimpl(wrapper=True, tryfirst=True) def pytest_collection_modifyitems( @@ -388,7 +388,7 @@ def pytest_collection_modifyitems( previously_failed = [] previously_passed = [] for item in items: - if item.id in self.lastfailed: + if str(item.id) in self.lastfailed: previously_failed.append(item) else: previously_passed.append(item) @@ -433,7 +433,7 @@ def pytest_sessionfinish(self, session: Session) -> None: return assert config.cache is not None - current_lastfailed = {str(k): v for k, v in self.lastfailed.items()} + current_lastfailed = self.lastfailed saved_lastfailed = config.cache.get("cache/lastfailed", {}) if saved_lastfailed != current_lastfailed: config.cache.set("cache/lastfailed", current_lastfailed) diff --git a/testing/test_cacheprovider.py b/testing/test_cacheprovider.py index a38a72c6011..da4248aa3e8 100644 --- a/testing/test_cacheprovider.py +++ b/testing/test_cacheprovider.py @@ -8,13 +8,19 @@ import os from pathlib import Path import shutil +from types import SimpleNamespace from typing import Any +from typing import cast from unittest.mock import mock_open +from _pytest.cacheprovider import LFPlugin from _pytest.compat import assert_never from _pytest.config import ExitCode +from _pytest.main import Session from _pytest.monkeypatch import MonkeyPatch from _pytest.pytester import Pytester +from _pytest.reports import CollectReport +from _pytest.reports import TestReport from _pytest.tmpdir import TempPathFactory import pytest @@ -414,6 +420,97 @@ def test_3(): assert 0 result = pytester.runpytest("--lf", "--cache-clear") result.stdout.fnmatch_lines(["*1 failed*2 passed*"]) + def test_lastfailed_custom_item_name_with_brackets( + self, pytester: Pytester + ) -> None: + """Keep custom item names containing brackets intact for --last-failed.""" + pytester.makeconftest( + """ + import json + import pytest + + def pytest_collect_file(parent, file_path): + if file_path.name == "test_cases.json": + return Cases.from_parent(parent, path=file_path) + + class Cases(pytest.File): + def collect(self): + for name, passed in json.loads( + self.path.read_text(encoding="utf-8") + ).items(): + yield Case.from_parent(self, name=name, passed=passed) + + class Case(pytest.Item): + def __init__(self, *, passed, **kwargs): + super().__init__(**kwargs) + self.passed = passed + + def runtest(self): + assert self.passed + """ + ) + cases = pytester.path / "test_cases.json" + cases.write_text('{"a_bad[one]": false, "b_fixed": false}', encoding="utf-8") + + result = pytester.runpytest_inprocess("-q") + assert result.ret == 1 + + cases.write_text('{"a_bad[one]": false, "b_fixed": true}', encoding="utf-8") + result = pytester.runpytest_inprocess("-q", "--lf") + assert result.ret == 1 + result.stdout.fnmatch_lines(["FAILED test_cases.json::a_bad[[]one[]]*"]) + + def test_lastfailed_stores_string_nodeids(self, pytester: Pytester) -> None: + config = pytester.parseconfigure("--lf") + plugin = config.pluginmanager.getplugin("lfplugin") + assert isinstance(plugin, LFPlugin) + + failed_report = SimpleNamespace( + id="test_file.py::test_case[one]", + when="call", + passed=False, + skipped=False, + failed=True, + ) + plugin.pytest_runtest_logreport(cast(TestReport, failed_report)) + assert plugin.lastfailed == {"test_file.py::test_case[one]": True} + + passed_report = SimpleNamespace( + id="test_file.py::test_case[one]", + when="call", + passed=True, + skipped=False, + failed=False, + ) + plugin.pytest_runtest_logreport(cast(TestReport, passed_report)) + assert plugin.lastfailed == {} + + plugin.lastfailed["test_file.py::test_group"] = True + plugin.pytest_collectreport( + cast( + CollectReport, + SimpleNamespace( + id="test_file.py::test_group", + outcome="passed", + result=[SimpleNamespace(id="test_file.py::test_case[one]")], + ), + ) + ) + assert plugin.lastfailed == {"test_file.py::test_case[one]": True} + + plugin.pytest_collectreport( + cast( + CollectReport, + SimpleNamespace( + id="test_file.py::test_broken_group", + outcome="failed", + result=[], + ), + ) + ) + assert plugin.lastfailed["test_file.py::test_broken_group"] is True + plugin.pytest_sessionfinish(cast(Session, SimpleNamespace())) + def test_failedfirst_order(self, pytester: Pytester) -> None: pytester.makepyfile( test_a="def test_always_passes(): pass",