Skip to content

test: retry the Keychain store-wide reads that raced their own writes #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
StuartMeeks merged 1 commit into from
Aug 26, 2026
Merged

test: retry the Keychain store-wide reads that raced their own writes #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- **Six Keychain tests could read the store before their writes were visible.** `main` went
red on `macos-14` with `GetProviderNamesAsync_ReturnsDistinctProviders` reporting
*"Expected: 2, Actual: 0"* — the same commit having passed that leg minutes earlier on the
PR. `AddCredentialAsync` confirms visibility through an exact service+account query, while
`ListCredentialsAsync` and `GetProviderNamesAsync` go through the class-wide query, so one
being visible does not imply the other. Fourteen reads in that file already used
`RetryHelper`; these six did not. Each now retries on the condition it actually asserts,
not merely on something having appeared — the distinction #61 turned on.

### Changed

- **CI now runs three macOS versions and splits the libsecret tests into their own job**
Expand Down
41 changes: 34 additions & 7 deletions tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/KeychainCredentialManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ public async Task ListCredentialsAsync_ReturnsAddedCredential()
var manager = NewManager();
var accountId = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{}");

var list = (await manager.ListCredentialsAsync("Adobe")).ToList();
// Retried like its siblings: AddCredentialAsync confirms visibility through an
// exact service+account query, while these reads go through the class-wide
// query, so one being visible does not imply the other (#61's pattern).
var list = (await RetryHelper.UntilAsync(
() => manager.ListCredentialsAsync("Adobe"), r => r.Any(c => c.AccountId == accountId))).ToList();

var credential = Assert.Single(list);
Assert.Equal(accountId, credential.AccountId);
Expand All @@ -167,8 +171,13 @@ public async Task ListCredentialsAsync_FiltersByProvider()
_ = await manager.AddCredentialAsync("Adobe", "a1", "Production", "{}");
_ = await manager.AddCredentialAsync("Airtable", "b1", "Production", "{}");

var adobe = (await manager.ListCredentialsAsync("Adobe")).ToList();
var airtable = (await manager.ListCredentialsAsync("Airtable")).ToList();
// Retried like its siblings: AddCredentialAsync confirms visibility through an
// exact service+account query, while these reads go through the class-wide
// query, so one being visible does not imply the other (#61's pattern).
var adobe = (await RetryHelper.UntilAsync(
() => manager.ListCredentialsAsync("Adobe"), r => r.Count() == 1)).ToList();
var airtable = (await RetryHelper.UntilAsync(
() => manager.ListCredentialsAsync("Airtable"), r => r.Count() == 1)).ToList();

var adobeCredential = Assert.Single(adobe);
var airtableCredential = Assert.Single(airtable);
Expand Down Expand Up @@ -255,7 +264,12 @@ public async Task GetCredentialByIdAsync_DoesNotMutateSelection()

_ = await manager.GetCredentialByIdAsync("Adobe", otherId);

var listings = (await manager.ListCredentialsAsync("Adobe")).ToList();
// Retried like its siblings: AddCredentialAsync confirms visibility through an
// exact service+account query, while these reads go through the class-wide
// query, so one being visible does not imply the other (#61's pattern).
var listings = (await RetryHelper.UntilAsync(
() => manager.ListCredentialsAsync("Adobe"),
r => r.Count() == 2 && r.Any(c => c.IsSelected))).ToList();
Assert.True(listings.Single(c => c.AccountId == selectedId).IsSelected);
Assert.False(listings.Single(c => c.AccountId == otherId).IsSelected);
}
Expand All @@ -282,7 +296,11 @@ public async Task DeleteCredentialAsync_RemovesCredential()
var accountId = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{}");

var deleted = await RetryHelper.UntilTrueAsync(() => manager.DeleteCredentialAsync(accountId));
var list = (await manager.ListCredentialsAsync("Adobe")).ToList();
// Retried like its siblings: AddCredentialAsync confirms visibility through an
// exact service+account query, while these reads go through the class-wide
// query, so one being visible does not imply the other (#61's pattern).
var list = (await RetryHelper.UntilAsync(
() => manager.ListCredentialsAsync("Adobe"), r => !r.Any())).ToList();

Assert.True(deleted);
Assert.Empty(list);
Expand Down Expand Up @@ -334,7 +352,12 @@ public async Task GetProviderNamesAsync_ReturnsDistinctProviders()
_ = await manager.AddCredentialAsync("Adobe", "a2", "Sandbox", "{}");
_ = await manager.AddCredentialAsync("Airtable", "b1", "Production", "{}");

var names = (await manager.GetProviderNamesAsync()).ToList();
// This is the read that actually failed on macos-14, "Expected: 2, Actual: 0",
// and the one most exposed to the lag: GetProviderNamesAsync goes through the
// class-wide query while the adds confirmed themselves through an exact
// service+account query.
var names = (await RetryHelper.UntilAsync(
() => manager.GetProviderNamesAsync(), r => r.Count() == 2)).ToList();

Assert.Equal(2, names.Count);
Assert.Contains("Adobe", names);
Expand Down Expand Up @@ -415,7 +438,11 @@ public async Task ListCredentialsAsync_IncludesDisplayFields_FromSummaryProvider
var manager = NewManager([summaryProvider]);

_ = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{\"apiKey\":\"xyz\"}");
var list = (await manager.ListCredentialsAsync("Adobe")).ToList();
// Retried like its siblings: AddCredentialAsync confirms visibility through an
// exact service+account query, while these reads go through the class-wide
// query, so one being visible does not imply the other (#61's pattern).
var list = (await RetryHelper.UntilAsync(
() => manager.ListCredentialsAsync("Adobe"), r => r.Count() == 1)).ToList();

var credential = Assert.Single(list);
var field = Assert.Single(credential.DisplayFields);
Expand Down