Repository navigation
ci: shard native conformance jobs and trim Apple setup - #492
Merged
Merged
Conversation
The android/iOS/linux/windows/macos jobs each ran all 13 OIDC conformance plans sequentially in one job (~13-14min of real IdP round trips; Hybrid RP alone is ~262s on iOS). Split them into 4 parallel matrix shards (Basic RP, Implicit RP, Hybrid RP, everything else) so the slowest shard bounds the job instead of the sum of all plans. - patrol_test/app_test.dart reads --dart-define=CONFORMANCE_SHARD and tags every conformance patrolTest with a shard via a new _registerPlan helper; an unknown shard or a shard that selects zero plans throws immediately, so a skipped/absent plan can never pass silently. android/iOS/linux/windows pass CONFORMANCE_SHARD per matrix leg and give each shard its own coverage filename so upload-coverage's merge-multiple download doesn't clobber them. - macos shards via `flutter test --name` from the workflow instead, since its harness lives under packages/oidc/example/integration_test/, which is being changed on another in-flight branch; each shard asserts an exact test count so a missed/duplicated plan fails loudly there too. - iOS now kicks off `simctl boot` in the background right after selecting Xcode, overlapping it with environment setup and `flutter precache`, with a `simctl bootstatus -b` wait moved to right before the tests step. - `flutter doctor -v` (148s on iOS, similar on android) moved off the normal path to `if: failure()` in both jobs. No required status check on main currently names "ios"/"android"/etc. (confirmed via the branch protection API), so the new per-shard job names are not a breaking change today. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126i8ymDVXbifp6LrbCiyQL
Live run 37394788100 caught a real bug in the macos (rest) shard:
`flutter test -d macos --name '<10-way alternation>'` matched all 10
plans but only actually drove the last one, silently reporting the
other 9 as skipped ("+1 ~9: All tests passed!"). The exact-count guard
added for this caught it and failed the job loudly instead of letting
it pass -- but the underlying --name multi-match behavior itself
isn't usable here.
Fix: loop one `flutter test --plain-name "<exact plan>"` invocation
per plan instead of a single combined regex. Each invocation's result
(ran exactly 1, exit 0) is checked individually, which also simplifies
away the text-parsed aggregate count check. Per-plan coverage files
are now indexed (macos-app-<shard>-<n>-coverage.info) and the upload
glob widened to match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0126i8ymDVXbifp6LrbCiyQL
Run 37398194429 measured iOS shards queueing (started 01:15/01:18/
01:29/01:39, finished 01:57 -- 42min, worse than the ~29min unsharded
baseline) because ios and macos each sharded 4 ways, together asking
for 8 concurrent macOS-pool slots against GitHub's documented cap of
5 (docs.github.com/en/actions/reference/limits, "Usage limits":
concurrent macOS jobs are capped separately from, and far below, the
general concurrent-job limit on Free/Pro/Team).
That same run's macos (rest) failure was a false positive, not a
selector bug: 9 of its 10 plans are legitimately markTestSkipped() on
macOS for platform reasons already documented in shared_e2e.dart (no
origin authority for logout plans' redirect URI, no HTTP endpoint to
receive a form POST, etc). My --name/--plain-name exact-count checks
didn't distinguish "skipped" from "never ran" and failed a healthy
result.
Redesign:
- ios now shards 2 ways ("heavy" = Hybrid+Implicit RP, "light" = Basic
RP + everything else), time-balanced from the baseline's per-plan
numbers. patrol_test/app_test.dart's CONFORMANCE_SHARD now takes a
comma-separated set of shard tags so one matrix leg can request
several at once; android/linux/windows's existing single-tag usage
is unaffected.
- macos is un-sharded entirely: baseline run 37346215172 shows it
already finishes in 7m51s (vs ios's ~29min for the same plan set),
so there was nothing worth parallelizing, and every shard would
have cost more pool contention for no benefit. It keeps its
original, untouched steps.
- This workflow's own macOS-pool demand is now ios(2) + macos(1) = 3,
comfortably under the cap of 5.
android/linux/windows keep their 4-way CONFORMANCE_SHARD split
unchanged -- they run on ubuntu-latest/windows-latest, which draw from
GitHub's much larger general-purpose concurrent-job pool, not the
macOS-specific cap.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0126i8ymDVXbifp6LrbCiyQL
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 54 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Patrol conformance tests now select plans through ChangesConformance test sharding
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant Workflow as GitHub Actions workflow
participant Patrol
participant AppTest as app_test.dart
participant RegisterPlan as _registerPlan
Workflow->>Patrol: Set CONFORMANCE_SHARD
Patrol->>AppTest: Start test process
AppTest->>RegisterPlan: Register plan with shard tag
RegisterPlan-->>AppTest: Register selected plan
Merge Risk: ⚪ Minimal · up to No confirmed issue remains before merge. The iOS simulator startup failure path was not verified; normal CI validation remains appropriate. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #492 +/- ##
=======================================
Coverage 92.41% 92.41%
=======================================
Files 130 130
Lines 7474 7474
Branches 2650 2650
=======================================
Hits 6907 6907
Misses 567 567 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Resolves the tests.yaml conflict between this branch's iOS 2-way sharding and #469's new "Start/Stop simulator log capture" + "Upload simulator log" steps: both are kept, and the upload artifact name is made per-shard (ios-${{ matrix.shard }}-simulator-log) since actions/upload-artifact requires unique names within one run and this job now has two parallel shards. No other files conflicted (#491's conformance-log-since-race fix and #469's harness/test changes landed in integration_test/ and test/, none of which this branch had touched). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126i8ymDVXbifp6LrbCiyQL
Contributor
🔥 Firebase Hosting previews
Channels expire 7 days after their last deploy. |
ahmednfwela
pushed a commit
that referenced
this pull request
Oct 6, 2026
Rebuilt on main's 2-shard iOS job. The harness (packages/oidc/example) is taken from main as-is, so the earlier probes, controls and the module rerun leave this branch's diff against main. What stays: * .github/scripts/ios_ci_metrics.py, plus cheap default diagnostics: a 3s ps/vm_stat host sampler while the tests run. The app's lines and runningboardd's WebContent launch lines are read back from the simulator's persisted log afterwards. This replaces main's debug-level live log stream, which itself cost 40-60% CPU on a starved host. Heavy diagnostics (live streams, screen recording) are opt-in via workflow_dispatch. * IOS_REDUCE_LOAD (default on): Spotlight indexing off, other simulators shut down, smallest iPhone preferred. * ios_runner dispatch input, to measure macos-26 (3 vCPU / 7 GB arm64) against macos-26-intel (4 vCPU / 14 GB x64, same Xcode 26.6 and iOS 26.4 runtime). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126i8ymDVXbifp6LrbCiyQL
ahmednfwela
pushed a commit
that referenced
this pull request
Oct 6, 2026
Booting alongside the Flutter install slowed that install 3-4x on the 3-vCPU arm64 runner. The Flutter setup script took 65s with a synchronous boot and 178-273s with a concurrent one; cache-pub's hashFiles took 0.5s vs 27-125s. That is a net loss of 80-240s per iOS leg after #492. The boot now overlaps only flutter precache and the Patrol CLI activation, which moves ahead of the wait. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126i8ymDVXbifp6LrbCiyQL
ahmednfwela
pushed a commit
that referenced
this pull request
Oct 6, 2026
… suite Re-applied onto main's harness (after #492), unchanged in behaviour from 3903937. Load reduction did not make the iOS simulator's browser stalls go away: they stayed intermittent across every runner configuration measured. When, and only when, the suite's own log proves it received NO authorization request for an instance, the harness stops that instance and runs the module once more on a fresh one. The suite observed nothing there and judged nothing, so no verdict can be hidden. It never reruns when any authorization request arrived (every negative module), never reruns a rerun, and never reruns on an unreadable log. The rerun is printed to the job log and carried into the module's verdict line. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126i8ymDVXbifp6LrbCiyQL
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.
Splits the conformance plans across parallel matrix legs where runners allow it: Android/Linux/Windows 4-way (basic/implicit/hybrid/rest), iOS 2-way (heavy = Hybrid+Implicit, light = the rest) to stay within GitHub's 5 concurrent macOS jobs limit (docs.github.com/en/actions/reference/limits); macOS stays unsharded. The shard is selected by
--dart-define=CONFORMANCE_SHARD=...inpatrol_test/app_test.dart; an unknown tag or a shard selecting zero plans fails the job, so a missing plan can't pass. Also boots the iOS simulator in the background during setup and runsflutter doctor -vonly on failure.Measured vs main run 37346215172 on run 37435461617: iOS 30m49s → 24m56s, Android 14m15s → 10m35s, Linux 13m43s → 9m39s, Windows 12m59s → 9m59s, macOS unchanged; whole run 44m35s → 38m46s (still bounded by unit_tests). Check names change (e.g.
ios→ios (heavy)/ios (light)); no required checks are configured on main.🤖 Generated with Claude Code
https://claude.ai/code/session_0126i8ymDVXbifp6LrbCiyQL
Summary by CodeRabbit