fix(tests): eliminate the two cross-suite unit test flakes - #448
Merged
sozercan merged 3 commits intoAug 22, 2026
Conversation
…queue WebKitManager.clearAllData() called CookieArchiveWriteQueue.shared .invalidateAndDelete(), so any suite clearing a test manager wiped the archive another suite had just written. Seven test files call clearAllData() and Swift Testing runs their suites concurrently; .serialized only orders tests within a suite, so WebKitCookieRestoreTests failed whenever it lost the race (100% with --filter WebKit, green alone). Inject the queue into WebKitManager (defaulting to .shared, so production keeps a single archive) and give makeTestInstance() its own queue over new in-memory storage. Adds a regression test that fails without the injection. Extract AuthCookieOperationFence/LiveAuthCookieStoreClearer into their own file to keep WebKitManager.swift under the 900-line lint limit.
loadMoreBlockedWhileRefreshRewindsHistoryCursor simulated an in-flight refresh with a 100ms getHistory delay, then polled every 25ms before calling loadMore. On loaded runners the poll returned after refresh had already completed, so the !isRefreshingHistory guard no longer blocked loadMore: 3 continuation calls instead of 1, "Older" appended. Replace the delay with an explicit continuation gate on the mock, matching the existing shouldWaitForRemoveSongFromPlaylistResponse pattern, so refresh cannot finish before loadMore is attempted.
tsibog
changed the title
Contributor
Author
|
@codex review |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
senshinya
added a commit
to senshinya/kaset
that referenced
this pull request
Aug 21, 2026
…elay generationMismatchRollbackRestoresSnapshot simulated an in-flight continuation with a 150 ms delay, waited for loadingState to reach .loadingMore, then cancelled. On a loaded runner the response came back and appended "c" before the cancellation landed, so the rollback restored a snapshot of three tracks: ✘ (viewModel.playlistDetail?.tracks.map(\.videoId) → ["a","b","c"]) == ["a","b"] It blocked two release builds on 2026-08-21 and is a third flake beyond the two that upstream sozercan#448 addresses. The response now waits on an AsyncGate through the mock's existing beforePlaylistContinuationReturn hook, so the test cancels while the request is provably still in flight and releases it afterwards. What the test asserts is unchanged — applyRemainingTracksResponse still discards the batch through its own !Task.isCancelled guard — but the ordering is now a fact rather than a bet on scheduling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
httperry
added a commit
to httperry/kaset
that referenced
this pull request
Aug 22, 2026
httperry
added a commit
to httperry/kaset
that referenced
this pull request
Aug 22, 2026
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
2 participants
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.
Description
Fixes the two pre-existing flaky unit tests that have been failing
macOS Unit Testson unrelated PRs. Both are test-only issues; no production behaviour changes.1.
WebKitCookieRestoreTests— shared cookie archive (macos-26)WebKitManager.clearAllData()callsCookieArchiveWriteQueue.shared.invalidateAndDelete()— a process-wide actor overKeychainCookieStorage. Seven test files callclearAllData(), and Swift Testing runs their suites concurrently, so one suite deletes the cookie archive another suite has just written..serializedon the individual suites doesn't help: it only orders tests within a suite.Repro on current
main:2.
HistoryViewModelTests— refresh finishing beforeloadMore(macos-15)loadMoreBlockedWhileRefreshRewindsHistoryCursorsimulated an in-flight refresh with a 100msgetHistorydelay, then polled every 25ms before callingloadMore(). On loaded runners the poll returned after refresh had already completed, so the!isRefreshingHistoryguard no longer blockedloadMore(): 3 continuation calls instead of 1, with"Older"appended.Replaced the delay with an explicit continuation gate on the mock, matching the existing
shouldWaitForRemoveSongFromPlaylistResponsepattern, so refresh cannot finish beforeloadMore()is attempted.Type of Change
Changes Made
WebKitManagertakes an injectablecookieArchiveQueue, defaulting to.shared— production behaviour is unchanged, still a single process-wide archivemakeTestInstance()gets its ownCookieArchiveWriteQueueover a newCookieArchiveStorage.inMemory(), so parallel suites can no longer clear or overwrite each other's archiveCookieArchiveWriteQueue.sharedcall sites (all insideextension WebKitManager) now go through the injected queueAuthCookieOperationFence/LiveAuthCookieClearResult/LiveAuthCookieStoreClearerintoWebKitManager+AuthCookieClearing.swiftto keepWebKitManager.swiftunder the 900-linefile_lengthlimitCookieArchiveStorage.inMemory()returns.allowedfromrestoreDecision, which matches what the live storage already does under unit tests (CookieArchiveRestorePolicy.restoreDecisionshort-circuits onUITestConfig.isRunningUnitTests), so no test coverage is weakened.Testing
swift test --skip KasetUITests— 6 consecutive full-suite runs, 0 failuresswift test --skip KasetUITests --filter WebKit— 5/5 green (was 5/5 red)main, 0/7 red with the fixswiftlint --strict && swiftformat .cleanChecklist
swiftlint --strict && swiftformat .Additional Notes
Production behaviour is unchanged —
WebKitManager.sharedstill usesCookieArchiveWriteQueue.shared. The injection point exists only so tests stop sharing one global archive.The
HistoryViewModelTestsfix is cherry-picked from #442, where it was authored as part of an unrelated feature branch. It's shared test infrastructure, so it's carried here to unblock every branch rather than only that one — #442 can drop its copy once this lands.Unrelated flakes remain in
ScrobblingCoordinator*andPlaylistDetailViewModelTests; those look like separate shared-state issues and aren't touched here.