[tests] Fix flaky DeskCase_83099_InmutableDictionary keychain test#25251
[tests] Fix flaky DeskCase_83099_InmutableDictionary keychain test#25251rolfbjarne merged 4 commits intomainfrom
Conversation
Three changes to eliminate intermittent InvalidRecord failures:
1. Use a process-unique server name (Test1-{PID}) to avoid cross-process
keychain conflicts on shared CI agents.
2. Stop attaching LAContext with InteractionNotAllowed to search/delete
records. The LAContext can cause intermittent InvalidRecord errors on
some macOS keychain states. Plain internet passwords don't require
biometric auth, so the LAContext is unnecessary for these operations.
3. Handle unexpected query status codes (e.g. InvalidRecord) by falling
through to the add path instead of silently returning false. The
previous code only handled ItemNotFound and Success, leaving any
other query result as an unrecoverable failure.
Fixes #25222
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit c9d210b.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Fixes flakiness in the DeskCase_83099_InmutableDictionary keychain test by reducing cross-run keychain collisions and making keychain save/update logic more resilient to unexpected keychain states.
Changes:
- Make keychain identity unique per process by including the PID in the server name (and clean up legacy
"Test1"entries). - Stop attaching
LAContextto keychain query/remove operations by using a minimalSecRecordfor lookups/deletes. - Restructure save logic to handle unexpected query status codes by force-removing and re-adding entries when needed.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The new helper methods bypass InitSecRecord (intentionally, to avoid LAContext), but that also skipped the macOS 11.* CI hang guard. Add the guard directly to the test method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
✅ [PR Build #e77faa8] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #e77faa8] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #e77faa8] Build passed (Build macOS tests) ✅Pipeline on Agent |
🚀 [CI Build #e77faa8] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 156 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Three changes to eliminate intermittent
InvalidRecordfailures inDeskCase_83099_InmutableDictionary:Unique keychain identity per process — server name is now
Test1-{PID}to avoid cross-process conflicts on shared CI agents.Removed LAContext from search/delete records — the
LAContextwithInteractionNotAllowedwas being attached to query and remove operations viaInitSecRecord/CreateSecRecord. This is unnecessary for plain internet passwords and is a plausible cause of the intermittentInvalidRecorderrors. Helper methods now use a minimalSecRecordfor lookups.Handle unexpected query status codes — the old
SaveUserPasswordonly handledItemNotFound(add path) andSuccess(update path). Any other status likeInvalidRecordsilently returnedfalse. The restructured logic usesSuccess→ update, else → force-remove + add.Fixes #25222