pytest-xdist
pytest-xdist copied to clipboard
Loadgroup: Don't modify test nodeids any longer.
The loadgroup scheduler used to modify the nodeids of tests that were placed manually in a specific scope. This was used as a side-channel from test collection (on the pytest-running process) to the worker process. Instead, make that side-channel explicit by passing a parameter around. That's pretty ugly, but no less ugly than modifying and parsing (!) node IDs later on.
I'm not so keen on the "protocol" between the test hook and the schedulers, as we're currently just writing into a field of the currently active scheduler with this implementation.
I previously considered adding this as an extra argument to add_node_collection, but that then requires touching all schedulers for something that's only relevant to a single one. Other ideas are welcome here in code review!
This is motivated by https://github.com/pytest-dev/pytest-xdist/pull/1118.
Thanks for submitting a PR, your contribution is really appreciated!
Here's a quick checklist that should be present in PRs:
-
[ ] Make sure to include reasonable tests for your change if necessary
-
[ ] We use towncrier for changelog management, so please add a news file into the
changelogfolder following these guidelines:-
Name it
$issue_id.$typefor example588.bugfix; -
If you don't have an issue_id change it to the PR id after creating it
-
Ensure type is one of
removal,feature,bugfix,vendor,docortrivial -
Make sure to use full sentences with correct case and punctuation, for example:
Fix issue with non-ascii contents in doctest text files.
-
I'll fix the mypy complaints once we've decided on a viable approach for passing the info into the scheduler. Changelog is coming then, too.
It's maybe worth saying that I first wanted to put the scopes from the test collection into config.stash, but that's not possible, as there's the execnet indirection between the process where the pytest test collection runs on (presumably a "worker"), and the actual process that runs the test loop. That's been a bit frustrating, and as someone who's just using -nauto, that's a feature that's complicating the implementation of xdist needlessly.
A communication channel for those details has to be created, it's time consuming and coordination heavy
Hence the lack of a implementation of that so far
I just pushed a change that renames the parameter. I thought that we can make this information available to all schedulers, and we could even change the whole load* family to use the grouping information by default, and deprecate theloadgroup strategy.
I'd also love to respect the grouping in worksteal, but that looks a little bit trickier to implement.