Preserve duplicate collected items in grouped scheduling - #1383
Open
AdemVessell wants to merge 1 commit into
Open
AdemVessell wants to merge 1 commit into
AdemVessell wants to merge 1 commit into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1212.
The grouped schedulers use nodeids as work-unit keys, so duplicate collected
items collapse into one entry. Looking those names up with
list.index()alsoalways selects the first occurrence and repeatedly scans the collection.
This change keeps collection indices as work-unit keys through dispatch,
completion and worker replacement. Nodeids still determine the groups and the
text of crash reports. The worker protocol is unchanged.
Reproduction
Given
test_repeat.py:Run:
python -m pytest --keep-duplicates test_repeat.py test_repeat.py -n2 --dist=loadscopeOn the current base, both workers collect two items, but only one test runs and
the command exits successfully. With this patch, both occurrences run.
loadfileandloadgrouphave the same problem; ordinary pytest andloadalready execute both occurrences.
Tests
separate completion, unfinished work after worker replacement, and repeated
file selection through the normal CLI. They fail before the production fix
and pass after it.
tests pass.
source/test files; changed Python files pass Ruff lint and formatting.
There is one full-suite caveat. Two runs passed 231 tests with 6 skips and 10
expected failures. The final run had 230 passes and one failure in
test_keyboardinterrupt_hooks_issue79: debug logging in the worker receiverthread raised
OSError: [Errno 9] Bad file descriptor, followed by a missingworkeroutputerror. That test uses the unchanged defaultloadscheduler.A fixed 12-round comparison reproduced the same failure on unmodified source
(1/12 failures; patched 0/12). I have not changed or suppressed that test, and
those counts are not evidence that this patch changes shutdown reliability.
These checks were local on macOS ARM64, not a complete platform matrix.
Prepared with AI assistance; no independent human review is claimed.
CI follow-up: all 18 released-pytest test jobs passed on Linux and Windows.
The two
py311-pytestmainjobs failed five existingTestGroupScopecases.I reproduced those same five failures on both unmodified xdist at
eba6a447and this patch, using pytest commit
aceb21ab2locally on macOS ARM64.That pytest revision reads
nodeidfrom_id, while xdist's unchanged grouphook writes
_nodeid. I have left that separate compatibility issue out ofthis patch. CI results.
Compatibility note
workqueueandassigned_worknow store collection-index keys within eachgroup instead of nodeid keys. Subclasses that manipulate those dictionaries
directly may need adjustment. Public hook signatures and the wire protocol
are unchanged.