Fix Android CI and flaky GATT notification test - #222
Merged
Conversation
Code Coverage OverviewLanguages: Swift Swift / code-coverage/llvm-covThe overall coverage remains at 88%, unchanged from the Updated |
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 the CI failures present on
master.Android (aarch64, build) — never passed since it was introduced
This job failed on every historical run, for two stacked reasons:
android/…_android-0.1.artifactbundle.tar.gzURL returned 404 and the job died at install.'semaphore.h' file not found. The official bundle declaressdkRootPath: "ndk-sysroot"but ships no sysroot of its own — it requires a separately installed Android NDK. The x86_64 job gets away with it becauseswift-android-actioninstalls and wires up the NDK; a bareswift sdk installin a plain toolchain container does not.Rather than add NDK provisioning, both ARM cross-compile jobs now share finagolfin's 6.1.3 SDK — the configuration the armv7 job has been passing with all along, since that bundle includes its own sysroot. The two jobs are collapsed into one matrix; the check names
Android (aarch64, build)andAndroid (armv7, build)are unchanged.Android (x86_64, test)
swift-test-flags: --no-parallelis passed verbatim to the on-device test runner binaries, and the XCTest runner rejects it (Error: Invalid option "--no-parallel"), failing the job before any test ran. Removed the flag; the race it guarded against is fixed at the source instead (below). Verified the job is genuinely green rather than vacuously green: the swift-testing invocation runs 474 tests in 30 suites, all passing.Linux (swift:6.1.2, debug) — flaky
GATTTests.notification()The test slept a fixed 1 ms after
server.writeValuebefore asserting, but notifications are delivered via detached tasks, so under CI load the assertion ran before delivery (receivedNotifications → []). Replaced the sleep with the same 10 ms/~5 s polling pattern already used for write completion inGATTClientServerTests.Verified:
swift test --filter GATTTestspasses locally; the relocated SDK URL downloads with a checksum matching the existing value; the SDK bundle'sswift-sdk.jsonwas inspected directly to confirm the missing sysroot and the available target triples.