tests: serialise keyring writes across xdist workers - #214
Open
robmsmt wants to merge 1 commit into
Open
Conversation
The session fixture writes the test InitConfig on every xdist worker, and on CI its secrets land in keyrings.alt's plaintext file under ~/.local/share/python_keyring. That backend creates the directory with a bare os.makedirs, so two workers racing for it on a fresh runner leave one with FileExistsError — and since the fixture is session scoped, every test on that worker then errors (12 passed, 90 errors on main's post-merge run 33120371812). Its read-modify-write of the file isn't atomic either. Create the directory up front and hold an fcntl lock around the writes. Keyrings without a file_path (macOS Keychain, ...) are left alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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.



Problem
Main's post-merge run 33120371812 (rerun) died in 23 s with 12 passed, 90 errors, every one:
sml_config_dirruns on each of the-n 2xdist workers and writes secrets into the keyring; on CI that'skeyrings.alt's plaintext file backend, whose_ensure_file_pathcallsos.makedirswithoutexist_ok. On a fresh runner both workers race for the directory; the loser raises, the session fixture error is cached, and every test on that worker errors. The backend's read-modify-write ofkeyring_pass.cfgisn't atomic either.Fix
_keyring_write_lock()intests/integration/conftest.py: if the active keyring exposes afile_path, create its directory withexist_ok=Trueand hold anfcntl.flockfor the duration of the config writes. Backends without a file path (macOS Keychain locally) are untouched.Verified locally with
PYTHON_KEYRING_BACKEND=keyrings.alt.file.PlaintextKeyringand four concurrent processes on a freshXDG_DATA_HOME: all writes land, file stays0600.🤖 Generated with Claude Code