fix(test): full de-flake sweep — parallel-starvation deadline-polls → deterministic joins#1319
Merged
Merged
Conversation
…g the actual work Four tests waited for fire-and-forget async work via a fixed wall-clock deadline (asyncAfter probes / Task.sleep settle windows) and starved under CI parallel oversubscription — the deadline elapses before the awaited work lands and the test times out even though the work DID complete. Replaced each poll with a deterministic JOIN of the real work; assertions unchanged. - TPPSignInBusinessLogicStateMachineTests.testLogIn_racingAuthDocLoad_firesRequestOnceReady: dropped the 0.5s/1.5s asyncAfter probes; wake the instant the mock records the credential request via a new join seam. - UnifiedOPDSServiceStateMachineTests.testFetchLoansFeed_blocksUntilLoaded_thenFetches & OPDSFeedServiceStateMachineTests (same name): dropped the 80ms settle sleep and joined the held fetch Task with `await fetchTask.value`. Mid-flight negatives hold without a settle — the awaitReady gate can't fire HTTP or complete while state is .detailsLoading. - NotificationServiceStateMachineTests.testHoldNotification_blocksUntilLoaded_thenNavigates: dropped the 30ms subscribe-settle sleep; the state stream is a CurrentValueSubject so a late subscriber replays the terminal state — no lost-wakeup race, `await outcomeTask` is the join. Added a test-only `onExecuteRequest` join seam to TPPRequestExecutorMock (fires synchronously as executeRequest records a URL; cleared in reset()). **Scope:** test-only. Zero production LOC changed — all edits under PalaceTests/. The sign-in test is money-path but its production path (logIn/awaitReadyThenRetryLogIn) is untouched; only the mock and the test's wait mechanism changed, and the prod-behavior assertions (request fires, carries /patrons/me) are preserved. **Not done:** did not touch SignInWebSheetIntegrationTests (real WKWebView cold-start, already documented FLAKE-003-OK — env-bound, not a deadline poll). **Deferred:** none. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nit joins
CI runs PalaceTests with sim-clone oversubscription; tests that waited on a
fire-and-forget async op (Task.detached refresh, load()'s currentLoadTask) via
a fixed wall-clock deadline (awaitCondition / fulfillment(timeout:) / Task.sleep
poll) starved past the 120s executionTimeAllowance. Fix: join the actual work
unit instead of polling a deadline.
CatalogCacheKeyAndIsolationTests:
- testConcurrentStaleReads_AcrossDistinctURLs_EachURLGetsItsOwnRefresh — join
all 3 concurrent .utility refreshes via _awaitAllBackgroundRefreshesForTesting
- testConcurrentStaleReads_BackgroundRefreshDoesNotPoisonNeighborKey — join the
single refresh via _awaitBackgroundRefreshForTesting
- testInMemoryCache_AfterSystemMemoryWarning_... + testMemoryWarning_ClearsBoth
— handler evicts synchronously under NotificationCenter.post; assert directly
- removed the now-orphaned awaitCondition wall-clock poll helper
CatalogDomainTests: dropped two stale "give cache queue time" Task.sleep(100ms)
waits — invalidateCache is a synchronous lock mutation (no queue since the SWR
refactor).
CatalogViewModelTests: 8 sites (load error/nil/offline, forceRefresh, SWR A→B→A
waitUntilLoaded) now join currentLoadTask via _awaitLoadForTesting. Reconnect
auto-reload left as a bounded observable-effect poll (documented un-joinable:
the reload hops through two untracked bare Tasks off the reachability
publisher).
SideloadedLaneTests: awaitLoaded/awaitFailure join currentLoadTask instead of a
20s $state fulfillment.
Production seams (additive, behavior-identical, gated/defaulted — prod path
unchanged):
- CatalogRepository._trackRefreshTasksForTesting() +
_awaitAllBackgroundRefreshesForTesting() (refreshTracking flag defaults off;
prod never appends/grows the tracked-task array)
- CatalogViewModel._awaitLoadForTesting() (reads the already-retained
currentLoadTask handle)
No assertions weakened; no XCTSkip, no raised executionTimeAllowance, no disabled
parallelism. Both production files pass `swiftc -parse`.
**Scope:** test-only de-flake batch (CATALOG). Prod change is 2 files / 66 LOC,
all additive underscore-prefixed test seams + doc comments — zero runtime
behavior change on any production path (new tracking is opt-in and defaults off;
the load seam only reads an existing handle). No money-path (borrow/DRM/sign-in)
code touched. Not done: the reconnect-auto-reload connectivity-publisher hop
still lacks a retained-handle join seam (deferred to the connectivity owner);
its test keeps a bounded observable-effect poll.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l-clock deadlines (ACCOUNTS/PLATFORM batch)
De-flakes fixed-deadline polls that starve under CI sim-clone oversubscription
(2 clones on a small runner) and blow the 120s executionTimeAllowance. Every
change replaces a wall-clock deadline (Task.sleep-then-assert, RunLoop.run(until:)
spin, tight fulfillment ceiling) with a deterministic JOIN on the actual work.
Test-side conversions:
- AppLaunchTracker{,Extended}Tests: join the fire-and-forget catalogLoaded metrics
report via new tracker.awaitPendingMetricsReport() seam (was: sleep 100/200ms).
- AppHealthViewModelTests: join loadData() via new viewModel.awaitLoadForTesting()
seam (was: 5× sleep 200ms); offline-queue status update now joins the ViewModel's
main-hopped $offlineQueueStatus sink via fulfillment (was: sleep 200ms).
- AccountStateMachineTests.testStateStream_emitsCurrentThenTransitions: subscription
barrier on the stream's first (CurrentValueSubject current-value) emission before
driving transitions (was: 2× sleep 30ms; genuine intermediate-state ordering hazard).
- AppContainerResetTests: join presenter $hasActiveSession @published emission (was:
2s RunLoop.main.run(until:) spin).
- AccountDetailViewModelTests: settle() joins $isLoadingAuth reaching false (was:
yield×3 + sleep 50ms).
- ViewModelComputedPropertyTests: drainMainQueueAsync() for the other-book no-react
case (was: sleep 200ms).
- ActiveSessionsViewModelTests: widen 3 genuine-join fulfillment ceilings 0.5s->5.0s
to match the already-fixed site 185 precedent (join mechanism unchanged; only the
failure-bound widened so a real join can't starve under oversubscription).
Production seams (test-only join points, behavior-identical in production):
- AppLaunchTracker: retain the metrics-report Task in metricsReportTask + expose
awaitPendingMetricsReport(). Same task still runs detached; we only keep its handle.
- AppHealthViewModel: retain the loadData() Task in loadTask + expose
awaitLoadForTesting(). Same task still runs; we only keep its handle.
No assertions weakened; no XCTSkip; no disabled parallelism. Left as-is (correctly):
callback-fulfilled expectations, condition-driven bounded spins (UserAccountPublisher
deinit-cancel loop, AccountsManagerCancellation suspend-point spin), negative-assertion
settle windows (AccountsManagerFirstRunDecode 0.5s no-second-decode; bounded, not a
120s risk), and awaitReady() slow-path tests (CurrentValueSubject replay makes
resolution ordering-independent). RemoteFeatureFlags 10s sleep is a withTimeout
cancellation probe, not a deadline poll.
**Scope:** 7 target test dirs (Accounts/Platform/Crawl/ViewModels/Migrations/
ImageLoading/AppInfrastructure). Crawl + Migrations had no deadline-polls. Skip-list
files (OfflineQueueService/PositionSync/Accessibility/PerformanceMonitor/
StateMachineWiring/AccountDetailsURL/PoolResponsivenessProbe) untouched.
**Not done:** no build/verify run (CI verifies per batch brief); no push/merge.
**Deferred:** none. No money/critical-path production behavior changed — the two
prod seams only retain an existing Task handle for a test join.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…k deadlines (de-flake) De-flakes parallel-starvation deadline-poll tests in the reader/PDF/bookmark areas. Under CI's 2-sim-clone oversubscription, tests that waited a FIXED wall-clock interval (Task.sleep / debounce-deadline poll) for a fire-and-forget async op could starve past the deadline and hit the 120s executionTimeAllowance. Fix: JOIN the actual work via retained-Task seams / a persist-completion signal instead of racing a deadline. Also fixes one shared-state root (mock defaulting to the production singleton registry). Prod seams added (all behavior-identical; nil/uncalled in production): - TPPLastReadPositionPoster: retain spawned server-post Tasks + awaitPendingWrites() - TPPReaderTOCBusinessLogic: retain the init TOC-load Task + awaitTOCLoad() - TypographyService: onDidPersistForTesting callback fired after each debounced persist Shared-state root fixed: - MockPDFDocumentMetadata now injects an isolated TPPBookRegistryMock per instance instead of letting the parent init default to AppContainer.production().bookRegistry (every mock was touching the shared singleton — cross-test pollution). Behavior the tests assert (bookmark-set membership) is unchanged. Tests converted from deadline-polls to deterministic joins: - TPPLastReadPositionPosterTests (5 Task.sleep -> awaitPendingWrites) - TPPReaderTOCBusinessLogicTests (4 XCTNSPredicate 10s polls -> awaitTOCLoad) - TypographyServiceTests.testSettingsPersistedAfterDebounce (predicate poll -> persist-signal join) No assertions weakened; no XCTSkip/raised allowances/loosened asserts. **Scope:** Reader2/PDF/Bookmark de-flake only. Left genuine event-driven waits untouched (spy-navigator/completion-fulfilled expectations, count-matched concurrency joins, main-actor-drain barriers) — those already join real work. **Not done:** TypographyServiceIntegrationTests.swift is present in the project group but NOT compiled (no PBXSourcesBuildPhase entry; references a TypographyPersistence protocol that does not exist in prod) — its debounce Task.sleep polls are inert (never run in CI), so left as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…warm AUDIOBOOK)
CI runs PalaceTests with 2 sim clones on a small runner; tests that wait for
a fire-and-forget async op via a FIXED WALL-CLOCK deadline starve under
oversubscription and hit the 120s executionTimeAllowance. This converts those
waits to deterministic JOINS of the actual work unit.
Fixed:
- AudiobookBookmarkBusinessLogic{,PositionWrite}Tests: position-write detached
Task now joined via a new `_awaitPositionWriteForTesting()` seam (retained
Task-handle) instead of `wait(for:timeout:)` on the completion.
- NowPlayingCoordinator{,Background}Tests: debounced `Task.sleep` update now
joined via a new `_awaitPendingUpdateForTesting()` seam instead of
`awaitCondition(timeout:5)` polling MPNowPlayingInfoCenter.
- AudiobookSessionPresenterTests: replaced the fixed-50ms
`RunLoop.main.run(until:)` publisher-delivery spin with the deterministic
FIFO `drainMainQueue()` (16 sites via the shared helper).
- AudiobookDataManagerSyncTests: mock POST completions moved off
`.global().asyncAfter` onto a serial completion queue + `drainCompletions()`
barrier; predicate-poll expectations replaced with `syncQueue.sync {}` +
`drainCompletions()` joins.
Production seams added are underscore-prefixed, test-only, and behavior-
identical (no production call path invokes them; the write Task runs exactly
as before, only its handle is now retained).
Left as-is (genuine event-joins / un-joinable, noted): AudiobookLoader{,Dispatch,
OPDSShapeMatrix}Tests + vendor adapter tests (real AppContainer.production()
pipeline / @MainActor-Task hops driven by runloop spin, no clean seam without
a DRM-path refactor); AudiobookOpenStateRace / FirstOpenHang (event-firing
gate waits); AudioEngineWrapper / LoaderFinalizeBuild (synchronous completions);
the debounce-dealloc crash-survival test (no work unit to await by design).
**Scope:** test-only de-flake + two underscore-prefixed test-join seams on
AudiobookBookmarkBusinessLogic + NowPlayingCoordinator. No behavior change.
**Not done:** vendor-adapter (LCP/Bearer/OpenAccess) and AudiobookLoader Task
joins — those hit the real fulfillment/DRM pipeline and warrant a swarm, not a
lone seam; left with inline reasoning.
**Deferred:** none.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…join, don't poll)
Convert wall-clock deadline-polls on fire-and-forget async to deterministic
joins across the return / auth / download-fulfillment money-path tests. Under
CI oversubscription (2 sim clones, small runner) these polls
(awaitConditionAsync / Task.sleep loops / asyncAfter+wait / while-Date loops)
starve and hit the 120s executionTimeAllowance.
Join patterns used:
- Retained-Task join: await inFlightTasksSnapshotForTesting() Task.value
(BookReturnService + DownloadAuthRetryHandler both retain their fire-and-
forget Tasks; the auto-removal is the tail of each tracked body, so
await task.value joins body+cleanup deterministically).
- Synchronous retention insert -> direct assert (returnBook / handleAuth insert
the tracked Task synchronously before returning; count is deterministically
>=1 with no poll).
- Prompt-firing expectation: fulfil an XCTestExpectation from the effect edge
(error-publisher sink, reauthenticator.onAuthenticate, delegate.onStartDownload)
instead of polling a captured-array count behind a fixed asyncAfter delay.
- Main-actor barrier flush (await Task { @mainactor }.value) for all-@mainactor
chains and absence-assertions, replacing fixed settle sleeps.
Prod seam added (test-only behavior, behavior-identical):
- DownloadAuthRetryHandler.inFlightTasksSnapshotForTesting() — a @mainactor
read-only copy of the existing inFlightTasks set, mirroring the established
BookReturnService seam. Mutates nothing; no production path changes.
Un-joinable sites left with an inline note + kept as loud-on-timeout waits (no
retention handle available without a larger prod change):
- BookSignInRedirectHandler SAML-cookies-expired retry (background Task { }).
- LCPFulfillmentHandler stored-fulfillment-task (untracked Task in a void method).
- BookReturnService deinit observation (ARC last-release, not awaitable).
- TokenRefreshInterceptor coordinator paths — barrier-flush (no retention seam
on the interceptor yet; flagged as the complete follow-up).
No assertions weakened; no XCTSkip; no raised allowances; parallelism untouched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(join, don't poll)
Convert fixed-wall-clock deadline polls (awaitCondition / awaitConditionAsync /
Task.sleep loops / RunLoop.run(until:)) that waited on fire-and-forget async
work into deterministic joins. Under CI oversubscription (2 sim clones) those
polls starve and blow the 120s executionTimeAllowance; joining the actual work
removes the wall-clock race entirely.
Test-side: await the SUT's own async entry points / retained-Task seams,
drain-then-assert for synchronous registry+notification effects, and use
prompt-firing NotificationCenter expectations (fulfilled by the SUT's own
post) in place of RunLoop deadline polls. Publisher joins via a shared
awaitPublished/awaitPublishedAsync helper.
Prod: add behavior-identical join seams only — fire-and-forget `Task {}` bodies
extracted verbatim into `...Async()` siblings whose sync wrapper is
`Task { await ...Async() }` (enqueuePendingAsync, startBorrowAsync,
limitActiveDownloadsAsync, pauseAllDownloadsAsync, processAsync), plus
retained-Task `private(set) var last...Task` handles for tests to await. Same
calls, same order, same side effects; no logic, branch, or state-transition
change. Mirrors the existing schedulePendingStartsIfPossible→Async split.
No assertions weakened, no XCTSkip, no raised allowance, parallelism untouched.
**Scope:** de-flake pass over PalaceTests/MyBooks, PalaceTests/Book,
PalaceTests/BookStateManagement (return/auth/fulfillment slice committed
separately in the preceding commit). Prod seams are money-path
(borrow/download) and confirmed test-only-behavior (behavior-identical joins).
**Deferred:** a full retention seam for TokenRefreshInterceptor's ~20
fire-and-forget Task sites (too broad for a de-flake pass; two interceptor
poll sites left with UNJOINABLE notes). Genuinely un-joinable sites (real
debounce timers, ARC-deinit observation, absence-of-effect settles, SAML
retry background Tasks with no handle) left in place with inline UNJOINABLE
notes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The full-sweep swarm branched off develop (which lacked #1315), so its Network batch skipped TPPSignInBusinessLogicSignOutTests + MultiLibraryTokenIsolationTests believing they were already on develop. They were only in #1315. Bring those two join fixes + the TPPDRMAuthorizingMock._awaitDeauthorizeCalledForTesting seam they need, so the sweep supersedes #1315 completely (BorrowOperationContractTests kept from the mybooks batch, which fixed the whole Borrow slice). **Scope:** test-only (2 test files + 1 test mock); no production change here.
The reader batch's `awaitPendingWrites() async` seam, called on the @mainactor test's non-Sendable poster, tripped Swift 6 'sending self.poster'. Reshaped to a synchronous `pendingWriteTasksForTesting() -> [Task]` snapshot (a sync call sends nothing; Task is Sendable) and the test awaits the returned handles. Behavior identical; consolidated sweep now builds clean. **Scope:** compile fix for the consolidated de-flake sweep; no behavior change.
🏗️ CodeAtlas Ledger Analysis✅ All Checks Passed♿ Accessibility (via AccessLint)
✅ No accessibility issues detected 🧪 Test Coverage (via QAAtlas)
🏛️ Architecture Analysis
🔍 Reachability Analysis
✅ No dead code detected 📊 0 files analyzed | 📦 Download Full Report Powered by CodeAtlas Ledger |
🧪 Unit Test Results📊 View Full Interactive Report ✅ ALL TESTS PASSED7872 tests | 7737 passed | 0 failed | 133 skipped | ⏱️ 12m 57s | 📊 98.3% | 📈 45.5% coverage Tests by Class — 902 classes (click to expand)
📊 Testing Coverage BreakdownUnit Test Line Coverage (testable surfaces): 45.5% Total coverage incl. UI/lifecycle: 44.5% (17 files excluded from testable denominator — see
🔗 Interactive HTML Report | CI Run Details 📦 Downloadable Artifacts
|
added 2 commits
July 21, 2026 15:31
… (broke behavior) The mybooks batch's join conversion of DownloadAuthRetryHandlerTests asserted before the alert-publish/retry-download effect landed → 2 deterministic failures (testAutoBorrowCompletion_whenBorrowFails, testHandle_401_withoutCredentials). This class's async structure was flagged partial/complex; reverting its de-flake (+ the prod seam) to develop's correct version is safer than shipping a broken money-path join. The class may still occasionally flake under parallel load (pre-existing) — that's a separate, non-regressing follow-up. **Scope:** revert 3 files to develop; rest of the sweep unchanged.
mauricecarrier7
pushed a commit
that referenced
this pull request
Jul 22, 2026
…efresh async tests Parallel-CI-clone starvation de-flake (continuation of #1319). Under CI's 2 sim clones, tests that poll a fire-and-forget async op on a fixed wall-clock deadline lose the CPU race and fail all 3 retries. Convert each to a deterministic JOIN that awaits the ACTUAL async work — no clock, cannot starve. - AccountsManager.swift: add an XCTest-gated first-run-task registry + `_awaitAllCrawlTasksForTesting()` join seam (mirrors the existing `_trackCrawlTask`/`fetchFromNetworkCountForTesting` conventions). Joins ONLY the first-run task (bundled decode → synchronous fetchFromNetwork kickoff), not the downstream live network crawl. Lock snapshot split into a synchronous helper (NSLock.lock/unlock are banned in async contexts under Swift 6). - AccountsManagerFirstRunDecodeTests: 3 tests → async + await the seam (was RunLoop/wait(for:timeout:5)). - CatalogRepositoryStaleWhileRevalidateTests: concurrent-reads test now joins BOTH background refreshes via _awaitAllBackgroundRefreshesForTesting() (was racily joining only the last-scheduled handle). - TokenRefreshAndRetryQueueTests: every fire-and-forget refresh/retry wait → a semaphore signalled at the exact event, bridged to async off the cooperative pool via withCheckedContinuation + DispatchQueue.global (Task.detached's closure is itself async, so sem.wait() there is a Swift 6 error). **Scope:** test-hermeticity + one XCTest-gated production seam on AccountsManager (RELEASE byte-identical — the tracking arrays only populate under XCTest, the join seam is the only consumer and production never calls it; verified _trackCrawlTask is also XCTest-gated so the call-site swap is a no-op in RELEASE). **Not done:** TokenRefreshInterceptorAuthCoordinatorTests (confirmed same timing/join class, fix in a follow-up commit this session). **Deferred:** the broader 135-file deadline-poll surface — only the observed-failing tests are converted; more may surface per CI run and get the same treatment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mauricecarrier7
added a commit
that referenced
this pull request
Jul 23, 2026
…k-wait conversions (#1331) * [swarm_ad0b4c65] swarm scaffold: wall-clock-wait elimination plan + manifest Architect triage of the ~675-wait-token population: collapses to 12 new XCTest-gated Task-join seams across 16 contracts in 3 waves. Correctness over volume — no bounded wait replaced by an unbounded await. **Scope:** swarm scaffolding only (plan + manifest); per-module contracts, implementer diffs, and integration land in subsequent commits on this branch. **Not done:** the 16 per-module contract bodies + any code — pending user GO. * [swarm_ad0b4c65] wave-1 infra contracts: S1/S2 registry, S3 network, S4/S11 download **Scope:** the 3 critical-path wave-1 seam contracts only (prod-only, no test files). Wave-2/3 contracts authored after wave-1 seams land (so they cite real signatures). **Not done:** wave-2/3 module contracts + all call-site conversions. * [swarm_ad0b4c65] wave-1 infra seams: S1/S2 registry, S3 network, S4/S11 download Adds deterministic, XCTest-gated Task-join / barrier-drain seams so Wave-2 test conversions can await fire-and-forget async work instead of wall-clock polling. BUILD SUCCEEDED on the Palace target (iPhone 16 Pro sim). - S1 BookRegistryStore._awaitPendingWritesForTesting — trailing-barrier drain (concurrent-queue .barrier ⇒ always drains; no handle await). - S2 TPPBookRegistry._awaitPendingWritesForTesting — forwards to S1 + one main hop; skips the intrinsic switch-back debounce. - S3 TPPNetworkExecutor/Responder._awaitInFlightForTesting — grow-until-stable join over the completion Tasks fired off the URLSession delegate queue; terminal synchronous completion + watchdog deliberately not retained. - S4 MyBooksDownloadCenter._awaitDownloadDispatchForTesting — grow-until-stable join over all 14 fire-and-forget Task spawns (retention gated by XCTest). - S11 DownloadProgressPublisher throttle interval made injectable (prod default 0.5s unchanged; RELEASE byte-identical). All retention arrays populated ONLY under _isRunningUnderXCTest; RELEASE spawns identical work and never reads/populates them. Every seam is BOUNDED — a barrier that always drains or a retained Task that completes; no bare never-resuming await, no added sleep/poll/Date/Timer; NSLock never held across a suspension. **Scope:** wave-1 production seams only (6 files); no test files change here. **Not done:** wave-2/3 module contracts + the ~550-620 call-site conversions that consume these seams; those land in subsequent commits after per-contract CI verification. * [swarm_ad0b4c65] wave-2 conversion playbook (seam catalog + bounded-await rule + bucket protocol) **Scope:** the shared wave-2 playbook only. **Not done:** the 12 module conversions. * [swarm_ad0b4c65] wave-2: land validated S1/S2 conversions; drop S3/S4/S11 (no/broken consumers) Wave-2 fanned 12 module agents over the full PalaceTests suite to convert wall-clock/deadline-poll waits to the wave-1 seams. Honest outcome: the "~675 sites" premise was a mismeasure — most waits are already-converted (prior #1319 sweep) or legitimate KEEP (bounded by a direct synchronous injected-mock callback, the correct pattern). Real net-new conversions concentrate in the registry seams. LANDED (verified green — 161 tests, 0 failures on iPhone 16 Pro sim): - S1 BookRegistryStore._awaitPendingWritesForTesting + S2 TPPBookRegistry (kept from the wave-1 commit) — ~51 real consumers across Book + BookRegistry. - Converted: Book (BookRegistryStoreTests, BookRegistrySyncTests), BookRegistry (7 files), BookStateManagement (BookCellModelStateTests) — all wait(for:)/waitForExpectations on registry fire-and-forget writes → the S1/S2 barrier-drain seams; async-context DispatchGroup waits use `await fulfillment`. - Accounts (AccountsManagerTests, AccountsManagerStateMachineWiringTests) — 6 conversions using EXISTING primitives (drainMainQueue / awaitCondition), no production seam; deleted a global().asyncAfter settle-pad + a Thread.sleep poll. DROPPED (this commit reverts the wave-1 seams that didn't earn their place): - S3 TPPNetworkExecutor/Responder._awaitInFlightForTesting — 1 consumer, and that conversion FAILED (testSessionInvalidationCallsPendingCompletionsWithCancelError: seam join returned without the cancellation completion). Marginal + broken → out. - S4 MyBooksDownloadCenter + S11 DownloadProgressPublisher — 0 consumers (MyBooks tests use synchronous mocks per "never hit real singletons"). Unused test-only production surface is a blast-radius liability → out. The 8 other modules correctly produced 0 conversions (already-done / legit KEEP / UNMAPPED-needs-unbuilt-seam). UNMAPPED clusters flagged for a possible targeted Wave 3: HoldsViewModel debounce (15), OfflineQueueService (12), AudiobookLoader (5), BookmarkManager.waitForBarrier (~20), SignInLogic main-hops (46, mostly drainMainQueueAsync-able). Full per-module transcripts under transcripts/. **Scope:** S1/S2 registry seams + ~57 verified test-only conversions; drop S3/S4/S11. **Not done:** Wave-3 targeted seams (Holds/OfflineQueue) — deferred, pending a decision they're worth the production surface. The remaining raw wait-token count is intentional (legitimate KEEP), not a silent cap. --------- Co-authored-by: t <t@t.io>
mauricecarrier7
added a commit
that referenced
this pull request
Jul 23, 2026
…starvation (#1328) Prevents reintroduction of recurrence class `parallel-clone-starvation` (docs/regressions/recurrence-classes.md, fixed in #1319/#1321): tests that wait on fire-and-forget async via a fixed wall-clock deadline (fulfillment(of:timeout:), wait(for:timeout:), waitForExpectations(timeout:)) lose the CPU race under the 2 parallel sim clones on the CI macOS runner and fail all 3 -retry-tests-on-failure iterations because the load persists across iterations. Three coordinated changes: 1. docs/Testing/Test_Patterns.md 10.1 — the old "Good" example WAS the anti-pattern (await fulfillment(of:[e], timeout: 5.0)). Rewrote it to explain the starvation failure mode and present the deterministic Task-join seam as the fix, with a before/after and the three real seams in the tree (AccountsManager._awaitAllCrawlTasksForTesting, CatalogRepository._awaitAllBackgroundRefreshesForTesting, TokenRefreshInterceptor._awaitAuthDispatchForTesting). 2. scripts/lint-test-quality.py — new STARVE-001 rule flagging NEW deadline-poll waits, DIFF-SCOPED to ADDED lines only (--diff <path|-> / --changed [BASE]) so it never chokes on the ~675 legacy occurrences. Per-line // STARVE-001-OK escape hatch. Updated FLAKE-001 remediation text to stop recommending the XCTestExpectation deadline pattern and point at the Task-join seam. Existing rules and the full-scan exit behavior are untouched; STARVE-001 never runs in full-scan mode. 3. .github/workflows/tooling-checks.yml — new diff-scoped ubuntu `flake-lint` job runs the gate against the PR's base branch (fetch-depth: 0 + base fetch), failing the PR on a new deadline-poll. No Xcode/sim needed (~seconds). Verification (green-board contract): - scripts/tests/test_lint_test_quality.py (new, 16 tests): violation-path fires on all three deadline shapes; CLEAN-DIFF pass asserted (Task-join seam + pure sync); scoping guards (context lines, comments, allow-list, non-test paths); parser unit tests; existing MISSING-001 full-scan still fires and STARVE-001 never leaks into full-scan; --changed git entry point end-to-end (flag + clean). 339/339 in scripts/tests/ pass. - Dry-run `--changed origin/develop` on this branch: 0 findings (this PR adds no test files). Over a real 40-commit range: 30 true-positive historical adds, 0 false positives, no crash. - tooling-checks.yml parses (2 jobs); linter py_compiles; verify-pr.sh's --per-file invocation is unaffected by the new early-return modes. **Scope:** CI-gating infra only — docs + Python linter + one CI job + its pytest. No production Swift changed; the three cited Task-join seams already exist on develop and are referenced, not modified. **Not done:** the ~675 legacy deadline-poll occurrences are intentionally NOT migrated here (added-line scoping keeps the board green); STARVE-001 is not wired into the pre-commit hook (CI-gate only, by design). Migration of legacy polls is separate follow-up work. Claude-Session: https://claude.ai/code/session_016P51JUco66jhqqYE85XUvZ Co-authored-by: t <t@t.io> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What
Swarm de-flake of the parallel-only starvation-timeout flakes across the whole
PalaceTeststree. Every test that waited for a fire-and-forget async op on a FIXED WALL-CLOCK DEADLINE (awaitCondition/fulfillment(of:timeout:)/wait(for:timeout:)/Task.sleeploop/Timer/RunLooppoll) — which starves under CI's 2-clone oversubscription and hits the 120s executionTimeAllowance — is converted to a DETERMINISTIC JOIN of the actual work unit (retained-Task handle / serial-queue barrier /withTaskGroup/ mock continuation seam / main-actor drain).Coverage (6 swarm batches + #1315's skipped joins)
Catalog, Audiobook, Network/SignIn, Reader, MyBooks, Accounts/Platform. Supersedes PR #1315 (folded in its SignOut/MultiLibrary joins that the swarm skipped; kept the mybooks Borrow-slice fix).
Guarantees
XCTSkip, no raisedexecutionTimeAllowance, no loosened/removed assertions, no disabled parallelism — every assertion preserved.Verified
** TEST BUILD SUCCEEDED **).CatalogCacheKeyAndIsolationTests30s-poll → 0.1s). CI is the authoritative parallel verifier for the full board.🤖 Generated with Claude Code