test(desktop): make PTT HAL liveness deterministic - #10742
Merged
Conversation
Fixes #10724. Wait until the injected HAL probe is actually wedged, then require three main-actor snapshot reads to complete. This preserves the liveness contract without treating macOS runner throughput in a fixed 300 ms window as correctness. Tests: desktop/macos/scripts/check_desktop_test_quality.py (macOS Swift CI required; this Windows host has no Xcode toolchain). Failure-Class: none
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
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.
Fixes #10724.
What changed and why
> 10timer/read throughput requirements in a 300 ms window with three explicit main-actor heartbeat completions and three explicitcurrentSnapshotcompletionsdesktop/macos/Desktop/Tests/, from the user-facing changelog gate and cover that path classification with regression casestestMainRunLoopKeepsPumpingWhileTheHALIsWedgedwas testing a binary liveness contract with host-throughput thresholds. Under macOS runner contention, the main actor and snapshot path still made progress, but the isolated suite completed too few iterations. The production contract is that the main actor and turn-start snapshot reads both keep completing while the HAL probe remains blocked off-main, so the new test observes those two completions independently instead of measuring scheduler throughput.The changelog gate already exempts internal test paths because its post-merge push run cannot see a PR-only
no-changelog-neededlabel. It covered the legacy lowercasedesktop/macos/tests/directory but not the Swift package tests underDesktop/Tests/, so this test-only patch exposed the same open failure class on the actual Swift test path.Failure evidence
main: run 30276084659,reads == 10ticker.count == 5,reads == 4ticker.count == 7,reads == 7All sibling
PTTInputDeviceProbeTestscases passed in those failures.Product invariants affected
none
How it was verified
python3 .github/scripts/test_desktop_changelog.pypassed: 4 testspython3 .github/scripts/check-desktop-changelog.py --base origin/main --head HEADpassed: no desktop changes require a changelog entrypython3 desktop/macos/scripts/check_desktop_test_quality.pypassedgit diff --check origin/main...HEADpassedstartedis fulfilled immediately beforerelease.wait(), main-actor heartbeat and snapshot-read expectations settle beforerelease.signal(), andfinishedis awaited afterwardThis contribution was prepared on Windows, so the local commit hook could not run the Xcode-only
swift-formatcommand (xcrunis unavailable). Desktop Swift CI is the authoritative compile, formatter, and XCTest verification for the Swift test change.Tests
ChangelogRequirementTestsnow covers both a top-level and nested Swift package test path.Failure class (fixes)
Failure-Class: FC-push-gate-internal-path-scope
Scoped cleanups
The changelog path exemption is a separate commit because the initial test-only head demonstrated that relying on
no-changelog-neededwould pass only the PR lane and could still redden the post-merge push lane.