cacheprovider: pytest_collection_modifyitems: copy items #6411
Conversation
| @@ -258,7 +258,7 @@ def pytest_collection_modifyitems(self, session, config, items): | |||
| self._report_status = "no previously failed tests, " | |||
| if self.config.getoption("last_failed_no_failures") == "none": | |||
| self._report_status += "deselecting all items." | |||
| config.hook.pytest_deselected(items=items) | |||
| config.hook.pytest_deselected(items=items[:]) | |||
There was a problem hiding this comment.
Good catch!
I wonder if it is a cache provider bug though... I mean, should every caller of pytest_deselected(items) always send a copy over? 🤔
Sure, pytest_deselected is kind of a weird hook, but I thought I would raise this.
There was a problem hiding this comment.
Not sure myself, but here it is clear that the list is cleared just below at least.
FWIW, I've found an issue with auto-deselecting of tests when I was passing a set there (and trying to items[:] it).
As for the terminal plugin only the length is relevant anyway I think (#6409), but thought to keep stats there like before.
There was a problem hiding this comment.
for sanity we should actually fail if session.items is passed by accident
this is a great and horrifying catch
the typical/presumed usage is collecting keep and drop in new lists, then passing drop after changing items, so the error is a honest misstake
There was a problem hiding this comment.
for sanity we should actually fail if session.items is passed by accident
Makes sense, please consider creating a new issue for it.
the typical/presumed usage is collecting keep and drop in new lists, then passing drop after changing items, so the error is a honest misstake
I could not really parse this.
do not copy items (ref: pytest-dev#6411)
blueyed
force-pushed
the
cacheprovider-copy-items
branch
from
54b9938 to
2d2c67d
Compare
Noticed this with #6409, where
itemsis stored/used as-is when there are no "deselected" stats yet.