From fba4468b7c3a3571123caa826dd573c751440a12 Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Tue, 25 Aug 2026 12:00:31 +0800 Subject: [PATCH] test: retry on the asserted property, not just the count (#61) RestoreCredentialAsync_Preserves... waited via RetryHelper.UntilAsync with a `r.Count == 1` predicate and then asserted restored.IsSelected. IsSelected is read from a separate store item -- the selection record -- written moments earlier by RestoreCredentialAsync, so the credential item can become visible before the selection item does. The count-only predicate let the assertions run against a half-visible store. Observed on the macOS runner during PR #60's CI, where the identical commit then passed on main, confirming timing rather than regression. The predicate now covers what is actually asserted, on both native backends. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 ++++++ .../Persistence/KeychainCredentialManagerTests.cs | 9 ++++++++- .../Persistence/LibsecretCredentialManagerTests.cs | 9 ++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ae9121..59df1af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`RestoreCredentialAsync_Preserves…` flaked on the OS-native backends** (#61). The test + retried until one credential was visible and then asserted `IsSelected`, which comes from a + *separate* store item written moments earlier — so the credential could become visible + before the selection did and the assertion ran against a half-visible store. Observed once + on the macOS runner. The retry predicate now covers the property being asserted. + - **`SelectionsLock`'s sentinel cleanup and exhaustion path were untested** (#57). The suite covered acquire, contend and re-acquire, but never asserted that `FileOptions.DeleteOnClose` actually removes the sentinel — one test's comment claimed it without checking — so dropping diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/KeychainCredentialManagerTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/KeychainCredentialManagerTests.cs index 3324fc0..d3ba450 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/KeychainCredentialManagerTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/KeychainCredentialManagerTests.cs @@ -115,7 +115,14 @@ public async Task RestoreCredentialAsync_PreservesAccountIdAndSelection() Assert.True(await RetryHelper.UntilTrueAsync(() => manager.DeleteCredentialAsync(id))); await manager.RestoreCredentialAsync(exported); - var restored = Assert.Single(await RetryHelper.UntilAsync(() => manager.ExportCredentialsAsync(), r => r.Count == 1)); + // Retry on the property actually being asserted, not just the count. The + // selection is a separate store item written moments earlier by + // RestoreCredentialAsync, so the credential can become visible before the + // selection does — and the count-only predicate let the assertions run against + // a half-visible store, which is what flaked on the macOS runner (#61). + var restored = Assert.Single(await RetryHelper.UntilAsync( + () => manager.ExportCredentialsAsync(), + r => r.Count == 1 && r[0].IsSelected)); Assert.Equal(id, restored.AccountId); Assert.True(restored.IsSelected); Assert.Equal("{\"apiKey\":\"secret\"}", await manager.GetSelectedCredentialAsync("Adobe")); diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/LibsecretCredentialManagerTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/LibsecretCredentialManagerTests.cs index 330855b..cc04425 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/LibsecretCredentialManagerTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/LibsecretCredentialManagerTests.cs @@ -184,7 +184,14 @@ public async Task RestoreCredentialAsync_PreservesAccountIdCreatedAtAndSelection Assert.True(await RetryHelper.UntilTrueAsync(() => manager.DeleteCredentialAsync(id))); await manager.RestoreCredentialAsync(exported); - var restored = Assert.Single(await RetryHelper.UntilAsync(() => manager.ExportCredentialsAsync(), r => r.Count == 1)); + // Retry on the property actually being asserted, not just the count. The + // selection is a separate store item written moments earlier by + // RestoreCredentialAsync, so the credential can become visible before the + // selection does — and the count-only predicate let the assertions run against + // a half-visible store, which is what flaked on the macOS runner (#61). + var restored = Assert.Single(await RetryHelper.UntilAsync( + () => manager.ExportCredentialsAsync(), + r => r.Count == 1 && r[0].IsSelected)); Assert.Equal(id, restored.AccountId); Assert.Equal(exported.CreatedAt, restored.CreatedAt); // libsecret preserves it via attribute Assert.True(restored.IsSelected);