Skip to content

fix(cli/sessionstore): don't run Windows keyring tests in parallel#22937

Merged
kylecarbs merged 1 commit intomainfrom
fix/test-keyring-flake
Mar 11, 2026
Merged

fix(cli/sessionstore): don't run Windows keyring tests in parallel#22937
kylecarbs merged 1 commit intomainfrom
fix/test-keyring-flake

Conversation

@kylecarbs
Copy link
Member

@kylecarbs kylecarbs commented Mar 11, 2026

Removes t.Parallel() from TestKeyring and TestWindowsKeyring_WriteReadDelete. The OS keyring is a shared system resource that's flaky under concurrent access, especially Windows Credential Manager in CI.

Fixes coder/internal#1370

@kylecarbs kylecarbs marked this pull request as draft March 11, 2026 11:48
@kylecarbs kylecarbs force-pushed the fix/test-keyring-flake branch 2 times, most recently from 0058cfa to fe08214 Compare March 11, 2026 11:56
@kylecarbs kylecarbs changed the title fix(cli/sessionstore): decouple TestKeyring from OS keyring to fix flake fix(cli/sessionstore): don't run Windows keyring tests in parallel Mar 11, 2026
@kylecarbs kylecarbs requested a review from mafredri March 11, 2026 12:00
@kylecarbs kylecarbs marked this pull request as ready for review March 11, 2026 12:00
@kylecarbs kylecarbs force-pushed the fix/test-keyring-flake branch from fe08214 to 2d04f50 Compare March 11, 2026 12:02
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Once nolints are fixed, 👍🏻

Comment on lines +25 to +26
// Not parallel: subtests hit the OS keyring which is flaky under
// concurrent access (especially Windows Credential Manager in CI).
Copy link
Member

Choose a reason for hiding this comment

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

This should be:

//nolint:tparallel // Subtests hit ...
func TestKeyring(t *testing.T) {

Same for the other one.

Copy link
Member

Choose a reason for hiding this comment

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

tparallel or paralleltest, not sure which.

@kylecarbs kylecarbs force-pushed the fix/test-keyring-flake branch from 2d04f50 to d741516 Compare March 11, 2026 12:17
The OS keyring is a shared system resource that's flaky under
concurrent access, especially Windows Credential Manager in CI.

Fixes coder/internal#1370
@kylecarbs kylecarbs force-pushed the fix/test-keyring-flake branch from d741516 to 96809b7 Compare March 11, 2026 12:33
@kylecarbs kylecarbs requested a review from mafredri March 11, 2026 12:44
@kylecarbs kylecarbs enabled auto-merge (squash) March 11, 2026 13:19
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Oops, thought I approved earlier.

@kylecarbs kylecarbs merged commit 71b132b into main Mar 11, 2026
24 checks passed
@kylecarbs kylecarbs deleted the fix/test-keyring-flake branch March 11, 2026 13:19
@github-actions github-actions bot locked and limited conversation to collaborators Mar 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flake: TestKeyring "file does not exist"

2 participants