Keep the iPad UI-test harness off wait-for-idle stalls (NOTAM network + terrain spinner)#12
Merged
Merged
Conversation
NOTAM retrieval went through the concrete `NOTAMLoader.shared` singleton with no test seam, so UI tests hit the live NOTAM API. On a CI runner with no outbound network the request hangs, leaving `isLoadingNOTAMs` true and an indeterminate `ProgressView` (a perpetual CoreAnimation) on the airport row. XCUITest's wait-for-idle never completes against that animation and stalls 60s before every interaction; six such stalls during weather entry pushed testWeatherUserEnteredMode and testLandingResultsValueCorrectness past the 7-minute per-test execution allowance on the iPad CI job. The tests pass locally only because real NOTAMs resolve quickly there. Introduce `NOTAMLoaderProtocol` (mirroring `WeatherLoaderProtocol`), conform `NOTAMLoader`, and inject the loader through `BasePerformanceViewModel` (and its takeoff/landing subclasses). Under `UI-TESTING`, `UITestingHelper` now supplies a `UITestingNOTAMLoader` that returns no NOTAMs immediately — the same pattern weather already uses — so the airport row settles, the app goes idle, and the harness proceeds. Production keeps using the live loader. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RISCfuture
force-pushed
the
fix/notam-loader-ui-test-stub
branch
from
a008f21 to
356806f
Compare
The takeoff/landing results screens show `ProgressView("Computing terrain
path…")` while an async terrain-path computation runs. An indeterminate
`ProgressView` is a perpetual CoreAnimation, so while it's on screen the app
never reaches idle and XCUITest's wait-for-idle stalls the full 60s before
each interaction. When a test taps through that screen while the computation
is in flight — e.g. testNOTAMClearMultipleResetsBadge tapping Back — the stalls
compound: locally the test ran 600–1274s and flaked; it is the same
wait-for-idle hazard as the NOTAM spinner but with a real, local computation
that can't be stubbed away.
Add `TerrainComputingIndicator`, which renders the `ProgressView` normally but
degrades to a static label under `UI-TESTING`. The computation still runs and
the label is unchanged; only the perpetual animation — which XCUITest discards
anyway — is dropped, so the app goes idle and the harness proceeds. Production
is untouched. Verified: testNOTAMClearMultipleResetsBadge now passes 3/3 at a
steady ~137s (was 600–1274s and flaky).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RISCfuture
changed the title
The single-NOTAM lookup has no callers anywhere in the app or tests. Routing NOTAM fetches through the new `NOTAMLoaderProtocol` surfaced it to Periphery as dead code. Remove it; the plural `fetchNOTAMs` still uses every `Errors` case and `NOTAMErrorResponse`, so nothing else becomes unused. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RISCfuture
added a commit
that referenced
this pull request
Jun 26, 2026
Two iPad UI tests exceeded the 7-minute per-test execution allowance on CI because indeterminate ProgressViews (a perpetual CoreAnimation) kept the app non-idle, stalling XCUITest's wait-for-idle 60s per interaction. - Inject a UI-testing NOTAM loader (NOTAMLoaderProtocol + UITestingNOTAMLoader), mirroring weather, so UI tests no longer hit the live NOTAM API — which hangs on a no-network CI runner and leaves the airport row spinning. - Render the terrain-path computing indicator statically under UI-TESTING (TerrainComputingIndicator), so the same wait-for-idle hazard on the results screens no longer stalls the harness. - Remove the now-unused NOTAMLoader.fetchNOTAM(id:). Production code paths are untouched; no animations are disabled in the app. Verified: local iPad suite 47/47, and CI green (Build, Unit, iPhone UI, iPad UI, Periphery, linters).
RISCfuture
added a commit
that referenced
this pull request
Jun 27, 2026
Two iPad UI tests exceeded the 7-minute per-test execution allowance on CI because indeterminate ProgressViews (a perpetual CoreAnimation) kept the app non-idle, stalling XCUITest's wait-for-idle 60s per interaction. - Inject a UI-testing NOTAM loader (NOTAMLoaderProtocol + UITestingNOTAMLoader), mirroring weather, so UI tests no longer hit the live NOTAM API — which hangs on a no-network CI runner and leaves the airport row spinning. - Render the terrain-path computing indicator statically under UI-TESTING (TerrainComputingIndicator), so the same wait-for-idle hazard on the results screens no longer stalls the harness. - Remove the now-unused NOTAMLoader.fetchNOTAM(id:). Production code paths are untouched; no animations are disabled in the app. Verified: local iPad suite 47/47, and CI green (Build, Unit, iPhone UI, iPad UI, Periphery, linters).
RISCfuture
added a commit
that referenced
this pull request
Jun 27, 2026
Two iPad UI tests exceeded the 7-minute per-test execution allowance on CI because indeterminate ProgressViews (a perpetual CoreAnimation) kept the app non-idle, stalling XCUITest's wait-for-idle 60s per interaction. - Inject a UI-testing NOTAM loader (NOTAMLoaderProtocol + UITestingNOTAMLoader), mirroring weather, so UI tests no longer hit the live NOTAM API — which hangs on a no-network CI runner and leaves the airport row spinning. - Render the terrain-path computing indicator statically under UI-TESTING (TerrainComputingIndicator), so the same wait-for-idle hazard on the results screens no longer stalls the harness. - Remove the now-unused NOTAMLoader.fetchNOTAM(id:). Production code paths are untouched; no animations are disabled in the app. Verified: local iPad suite 47/47, and CI green (Build, Unit, iPhone UI, iPad UI, Periphery, linters).
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.
Two iPad UI tests (
testWeatherUserEnteredMode,testLandingResultsValueCorrectness) were failing CI by exceeding the 7-minute per-test execution allowance — not on any assertion. Forensics on the failed run (spindump + xcresult) showed the app's main thread fully idle at the timeout, with XCUITest stalling the full 60 s wait-for-idle before each interaction ("App animations complete notification not received"). The cause in both cases is an indeterminateProgressView— a perpetualCAAnimationthat XCUITest's wait-for-idle can never settle against. This PR removes the two sources of that stall on the UI-test path.1. NOTAM loading hit the live network in UI tests
NOTAM retrieval went through the concrete
NOTAMLoader.sharedsingleton with no test seam, so UI tests hitnotams.fly.dev. On a CI runner with no outbound network the request hangs,isLoadingNOTAMsstays true, and the airport-row spinner spins forever. (Passes locally only because real NOTAMs resolve fast.)Fix — mirror the existing weather injection: add
NOTAMLoaderProtocol(returning[NOTAMResponse]), conformNOTAMLoader, inject the loader throughBasePerformanceViewModeland its takeoff/landing subclasses, and underUI-TESTINGsupply aUITestingNOTAMLoaderthat returns no NOTAMs immediately. Production uses the live loader unchanged.2. Terrain-path computing spinner
The takeoff/landing results screens show
ProgressView("Computing terrain path…")during a real, local async terrain computation. Its indeterminate spinner is the same wait-for-idle hazard — surfaced once NOTAMs resolved instantly — and madetestNOTAMClearMultipleResetsBadgeflake (600–1274 s locally).Fix —
TerrainComputingIndicatorrenders theProgressViewnormally but degrades to a static label underUI-TESTING. The computation still runs and the label is unchanged; only the perpetual animation (which XCUITest discards anyway) is dropped, so the app goes idle. Production is untouched.Both fixes preserve production fidelity (no animations disabled in the app, live code paths intact) while removing the test-environment stalls.
Verification
356806f(NOTAM fix): Build, Unit, iPhone UI, iPad UI, Linters, Periphery all ✅.testWeatherUserEnteredMode92 s,testLandingResultsValueCorrectness156 s (were killed at 7 min);testNOTAMClearMultipleResetsBadgenow 3/3 at a steady ~137 s (was 600–1274 s and flaky).🤖 Generated with Claude Code