Skip to content

[Backport 5.4] Merge pull request #6991 from blueyed/fix-lf-cmdline-args-upstream #7219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/6991.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix regressions with `--lf` filtering too much since pytest 5.4.
1 change: 1 addition & 0 deletions changelog/6991.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Collected files are displayed after any reports from hooks, e.g. the status from ``--lf``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should drop this change from the backport? (the changes in src/_pytest/terminal.py)

58 changes: 38 additions & 20 deletions src/_pytest/cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,27 +183,35 @@ def pytest_make_collect_report(self, collector) -> Generator:
res.result = sorted(
res.result, key=lambda x: 0 if Path(str(x.fspath)) in lf_paths else 1,
)
out.force_result(res)
return

elif isinstance(collector, Module):
if Path(str(collector.fspath)) in self.lfplugin._last_failed_paths:
out = yield
res = out.get_result()

filtered_result = [
x for x in res.result if x.nodeid in self.lfplugin.lastfailed
result = res.result
lastfailed = self.lfplugin.lastfailed

# Only filter with known failures.
if not self._collected_at_least_one_failure:
if not any(x.nodeid in lastfailed for x in result):
return
self.lfplugin.config.pluginmanager.register(
LFPluginCollSkipfiles(self.lfplugin), "lfplugin-collskip"
)
self._collected_at_least_one_failure = True

session = collector.session
result[:] = [
x
for x in result
if x.nodeid in lastfailed
# Include any passed arguments (not trivial to filter).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one makes sense.

or session.isinitpath(x.fspath)
# Keep all sub-collectors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I don't really understand this one, would you mind explaining what it does?

or isinstance(x, nodes.Collector)
]
if filtered_result:
res.result = filtered_result
out.force_result(res)

if not self._collected_at_least_one_failure:
self.lfplugin.config.pluginmanager.register(
LFPluginCollSkipfiles(self.lfplugin), "lfplugin-collskip"
)
self._collected_at_least_one_failure = True
return res
return
yield


Expand Down Expand Up @@ -234,8 +242,8 @@ def __init__(self, config: Config) -> None:
self.lastfailed = config.cache.get(
"cache/lastfailed", {}
) # type: Dict[str, bool]
self._previously_failed_count = None
self._report_status = None
self._previously_failed_count = None # type: Optional[int]
self._report_status = None # type: Optional[str]
self._skipped_files = 0 # count skipped files during collection due to --lf

if config.getoption("lf"):
Expand Down Expand Up @@ -269,7 +277,12 @@ def pytest_collectreport(self, report):
else:
self.lastfailed[report.nodeid] = True

def pytest_collection_modifyitems(self, session, config, items):
@pytest.hookimpl(hookwrapper=True, tryfirst=True)
def pytest_collection_modifyitems(
self, config: Config, items: List[nodes.Item]
) -> Generator[None, None, None]:
yield
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change (and in pytest_collection_modifyitems) needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe this is to ensure the actual code runs as late as possible

in this case this means, its running after most normal hook-wrappers finish (only the last bit of a earlier hook-wrapper could interact with it after that


if not self.active:
return

Expand Down Expand Up @@ -334,9 +347,12 @@ def __init__(self, config):
self.active = config.option.newfirst
self.cached_nodeids = config.cache.get("cache/nodeids", [])

@pytest.hookimpl(hookwrapper=True, tryfirst=True)
def pytest_collection_modifyitems(
self, session: Session, config: Config, items: List[nodes.Item]
) -> None:
self, items: List[nodes.Item]
) -> Generator[None, None, None]:
yield

new_items = OrderedDict() # type: OrderedDict[str, nodes.Item]
if self.active:
other_items = OrderedDict() # type: OrderedDict[str, nodes.Item]
Expand All @@ -358,11 +374,13 @@ def pytest_collection_modifyitems(
def _get_increasing_order(self, items):
return sorted(items, key=lambda item: item.fspath.mtime(), reverse=True)

def pytest_sessionfinish(self, session):
def pytest_sessionfinish(self) -> None:
config = self.config
if config.getoption("cacheshow") or hasattr(config, "slaveinput"):
return

if config.getoption("collectonly"):
return
config.cache.set("cache/nodeids", self.cached_nodeids)


Expand Down
8 changes: 5 additions & 3 deletions src/_pytest/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -661,15 +661,17 @@ def pytest_report_header(self, config):
def pytest_collection_finish(self, session):
self.report_collect(True)

if self.config.getoption("collectonly"):
self._printcollecteditems(session.items)

lines = self.config.hook.pytest_report_collectionfinish(
config=self.config, startdir=self.startdir, items=session.items
)
self._write_report_lines_from_hooks(lines)

if self.config.getoption("collectonly"):
if session.items:
if self.config.option.verbose > -1:
self._tw.line("")
self._printcollecteditems(session.items)

failed = self.stats.get("failed")
if failed:
self._tw.sep("!", "collection failures")
Expand Down
Loading