Repository navigation
Speed up macOS CI with unit test sharding - #6464
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesmacOS CI Test Sharding and New Job Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related issues
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
995357c to
1d9f3a6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 609-612: The checkout step using
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd needs to include an
additional configuration parameter for security. Add persist-credentials: false
to the with section of this checkout step, placing it alongside the existing
submodules: recursive parameter to prevent the GITHUB_TOKEN from remaining in
the local git config after the step completes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b0a1cd4-1896-407c-8f08-8d911af433a6
📒 Files selected for processing (2)
.github/workflows/ci.ymlscripts/ci/cmux_unit_test_shard.py
| - name: Checkout | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| submodules: recursive |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider adding persist-credentials: false to new checkout steps.
The checkout step doesn't set persist-credentials: false, which means the GITHUB_TOKEN remains in the local git config. While consistent with existing checkout steps in this workflow, adding this for new code would improve security posture by preventing accidental credential exposure.
- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
submodules: recursive
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout | |
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | |
| with: | |
| submodules: recursive | |
| - name: Checkout | |
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | |
| with: | |
| submodules: recursive | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 609-612: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 609 - 612, The checkout step using
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd needs to include an
additional configuration parameter for security. Add persist-credentials: false
to the with section of this checkout step, placing it alongside the existing
submodules: recursive parameter to prevent the GITHUB_TOKEN from remaining in
the local git config after the step completes.
Source: Linters/SAST tools
Greptile SummaryThis PR speeds up macOS CI by running
Confidence Score: 4/5Safe to merge with one test-coverage gap to address: override func test… methods in large suites will be silently omitted from the method-level split and will not run under the sharded build. The sharding logic, post-test timeout, and job splits are all well-implemented with tests. One gap in XCTEST_METHOD_RE means override func test… methods in any suite that crosses the 40-method threshold get no -only-testing selector and are silently skipped across all shards. This is a real but narrow coverage hole — it only affects suites large enough to be method-split that also override a base-class test method. scripts/ci/cmux_unit_test_shard.py — the XCTEST_METHOD_RE modifier alternation needs override (and optionally open) added. Important Files Changed
Reviews (3): Last reviewed commit: "Speed up macOS CI with unit test shardin..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| SUITE_RE = re.compile( | ||
| r"^(?:@[A-Za-z_][A-Za-z0-9_]*(?:\([^)]*\))?\s+)*" | ||
| r"(?:(?:final|private|fileprivate|internal|public)\s+)*" |
There was a problem hiding this comment.
SUITE_RE does not include open or package as valid access-level modifiers. A declaration like open class BrowserSystemProxyMirrorTests or package class FooTests starts with a token not in the alternation, so the regex fails to match and that suite is silently excluded from every shard. Any such suite would never run via the sharded path. package is available since Swift 5.9; open has always been part of the access-control grammar.
| r"(?:(?:final|private|fileprivate|internal|public)\s+)*" | |
| r"(?:(?:final|open|package|private|fileprivate|internal|public)\s+)*" |
1d9f3a6 to
ced2fe1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ci/xcodebuild_noninteractive.py`:
- Around line 212-213: The check for EXPECTED_FAILURE_SUMMARY_RE should only be
performed after the terminal-summary phase has been detected, not throughout the
entire rolling window. Currently, the pattern matching for
saw_expected_failure_summary at the location checking
EXPECTED_FAILURE_SUMMARY_RE.search(prompt_window) and the similar check at lines
242-244 can match earlier output and incorrectly set the flag. Add a condition
to ensure the expected-failure pattern check only executes after the
terminal-summary has been detected by checking the state of
terminal_summary_found or equivalent flag before evaluating
EXPECTED_FAILURE_SUMMARY_RE, and apply this same condition to all instances
where expected-failure detection occurs.
- Around line 207-210: The post_test_deadline is being recalculated and reset
every iteration when the SELECTED_TESTS_DONE_RE marker is still present in
prompt_window, continuously extending the timeout. To fix this, only set
post_test_deadline once when the terminal test summary is first detected. Add a
condition to check whether post_test_deadline has already been set or whether
saw_terminal_test_summary was already True before the current iteration before
reassigning the deadline value in that block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f61ad7e-3272-43db-88bf-4e20163bd6a2
📒 Files selected for processing (4)
.github/workflows/ci.ymlscripts/ci/cmux_unit_test_shard.pyscripts/ci/run-in-console-session.shscripts/ci/xcodebuild_noninteractive.py
ced2fe1 to
229623d
Compare
This comment has been minimized.
This comment has been minimized.
08589b7 to
1a45023
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ci/cmux_unit_test_shard.py`:
- Around line 19-24: The current implementation counts both `@Test` and XCTest
methods in the weight calculation using TEST_TOKEN_RE, but when splitting large
suites (LARGE_SUITE_METHOD_THRESHOLD of 40 or more), it only extracts and emits
XCTest selectors via XCTEST_METHOD_RE, silently dropping any `@Test` cases. To fix
this, either prevent suite splitting for mixed suites that contain both `@Test`
decorators and XCTest methods, or extend the selector extraction logic to also
capture and emit Swift Testing method selectors (using the `@Test` pattern) in
addition to XCTest selectors. Ensure that when a suite has both types of test
methods and meets the large-suite threshold, either the entire suite remains
unsplit or all test types are included in the sharded output.
In `@scripts/ci/xcodebuild_noninteractive.py`:
- Around line 56-61: In the except ValueError block that handles the
CMUX_XCODEBUILD_NONINTERACTIVE_POST_TEST_TIMEOUT_SECONDS parsing error, the
raise SystemExit(2) statement is directly re-raising without suppressing the
exception context chain, which violates Ruff B904. Add "from None" to the raise
SystemExit(2) statement to explicitly break the exception chaining and keep the
CLI error output concise without the original ValueError traceback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 664ebe8b-e221-44c7-9b24-1c7fe94044fb
📒 Files selected for processing (4)
.github/workflows/ci.ymlscripts/ci/cmux_unit_test_shard.pyscripts/ci/run-in-console-session.shscripts/ci/xcodebuild_noninteractive.py
| TEST_TOKEN_RE = re.compile(r"(^|\s)(@Test\b|func\s+test[A-Za-z0-9_]*\s*\()") | ||
| XCTEST_METHOD_RE = re.compile( | ||
| r"^\s*(?:(?:final|private|fileprivate|internal|public)\s+)*" | ||
| r"func\s+(test[A-Za-z0-9_]*)\s*\(" | ||
| ) | ||
| LARGE_SUITE_METHOD_THRESHOLD = 40 |
There was a problem hiding this comment.
Preserve @Test cases when splitting oversized suites.
weight includes Swift Testing @Test tokens, but the large-suite branch only emits func test... XCTest selectors and then continues. A mixed suite with ≥40 XCTest methods would silently skip its @Test cases in the sharded CI run; keep mixed suites at suite level or add Swift Testing method-selector support.
🐛 Proposed guard for mixed Swift Testing/XCTest suites
TEST_TOKEN_RE = re.compile(r"(^|\s)(`@Test`\b|func\s+test[A-Za-z0-9_]*\s*\()")
+SWIFT_TEST_TOKEN_RE = re.compile(r"(^|\s)`@Test`\b")
XCTest_METHOD_RE = re.compile(
r"^\s*(?:(?:final|private|fileprivate|internal|public)\s+)*"
r"func\s+(test[A-Za-z0-9_]*)\s*\("
) body = lines[line_number - 1 : next_line - 1]
weight = max(1, sum(1 for line in body if TEST_TOKEN_RE.search(line)))
+ has_swift_testing_tests = any(
+ SWIFT_TEST_TOKEN_RE.search(line) for line in body
+ )
suite_identifier = f"cmuxTests/{name}"
method_matches = [
(line_number + offset, match.group(1))
for offset, body_line in enumerate(body)
if (match := XCTEST_METHOD_RE.match(body_line))
@@
- if len(method_matches) >= LARGE_SUITE_METHOD_THRESHOLD:
+ if (
+ not has_swift_testing_tests
+ and len(method_matches) >= LARGE_SUITE_METHOD_THRESHOLD
+ ):Also applies to: 59-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/ci/cmux_unit_test_shard.py` around lines 19 - 24, The current
implementation counts both `@Test` and XCTest methods in the weight calculation
using TEST_TOKEN_RE, but when splitting large suites
(LARGE_SUITE_METHOD_THRESHOLD of 40 or more), it only extracts and emits XCTest
selectors via XCTEST_METHOD_RE, silently dropping any `@Test` cases. To fix this,
either prevent suite splitting for mixed suites that contain both `@Test`
decorators and XCTest methods, or extend the selector extraction logic to also
capture and emit Swift Testing method selectors (using the `@Test` pattern) in
addition to XCTest selectors. Ensure that when a suite has both types of test
methods and meets the large-suite threshold, either the entire suite remains
unsplit or all test types are included in the sharded output.
| except ValueError: | ||
| print( | ||
| "CMUX_XCODEBUILD_NONINTERACTIVE_POST_TEST_TIMEOUT_SECONDS must be numeric", | ||
| file=sys.stderr, | ||
| ) | ||
| raise SystemExit(2) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
path = Path("scripts/ci/xcodebuild_noninteractive.py")
text = path.read_text()
start = text.index("def post_test_timeout_seconds")
end = text.index("\ndef terminate_child", start)
block = text[start:end]
if "except ValueError:" in block and (
"raise SystemExit(2) from None" not in block
and "raise SystemExit(2) from err" not in block
):
raise SystemExit("post_test_timeout_seconds still violates Ruff B904")
PYRepository: manaflow-ai/cmux
Length of output: 111
🏁 Script executed:
cat -n scripts/ci/xcodebuild_noninteractive.py | sed -n '50,70p'Repository: manaflow-ai/cmux
Length of output: 788
🏁 Script executed:
python3 - <<'PY'
import subprocess
import sys
# Check if ruff recognizes B904
result = subprocess.run(
["python3", "-m", "ruff", "rule", "B904"],
capture_output=True,
text=True
)
if result.returncode == 0:
print("Ruff B904 rule info:")
print(result.stdout)
else:
print("Note: ruff not installed or B904 not recognized")
print("stderr:", result.stderr)
PYRepository: manaflow-ai/cmux
Length of output: 1380
Suppress the parse exception context for Ruff B904.
Line 61 raises SystemExit directly inside except ValueError, which violates the B904 check. Use from None to explicitly break exception chaining and keep the CLI error concise.
🔧 Proposed fix
except ValueError:
print(
"CMUX_XCODEBUILD_NONINTERACTIVE_POST_TEST_TIMEOUT_SECONDS must be numeric",
file=sys.stderr,
)
- raise SystemExit(2)
+ raise SystemExit(2) from None🧰 Tools
🪛 Ruff (0.15.17)
[warning] 61-61: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/ci/xcodebuild_noninteractive.py` around lines 56 - 61, In the except
ValueError block that handles the
CMUX_XCODEBUILD_NONINTERACTIVE_POST_TEST_TIMEOUT_SECONDS parsing error, the
raise SystemExit(2) statement is directly re-raising without suppressing the
exception context chain, which violates Ruff B904. Add "from None" to the raise
SystemExit(2) statement to explicitly break the exception chaining and keep the
CLI error output concise without the original ValueError traceback.
Source: Linters/SAST tools
1a45023 to
7cba2f9
Compare
7cba2f9 to
1c14489
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/ci/cmux_unit_test_shard.py (1)
14-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMixed Swift Testing suites silently dropped during large-suite splitting.
The weight calculation in
TEST_TOKEN_REcounts both@Testandfunc test...tokens, butXCTEST_METHOD_REonly extracts XCTest methods. When a suite with ≥40 XCTest methods also contains@Testcases, the large-suite branch (lines 132-134) emits only XCTest selectors, silently dropping any Swift Testing tests.Either prevent splitting for mixed suites or keep them at suite level:
🐛 Proposed guard for mixed Swift Testing suites
TEST_TOKEN_RE = re.compile(r"(^|\s)(`@Test`\b|func\s+test[A-Za-z0-9_]*\s*\()") +SWIFT_TESTING_RE = re.compile(r"(^|\s)`@Test`\b") XCTEST_METHOD_RE = re.compile( r"^\s*(?:(?:final|private|fileprivate|internal|public)\s+)*" r"func\s+(test[A-Za-z0-9_]*)\s*\(" )Then in
discover_selectors, before splitting:+ has_swift_testing = any(SWIFT_TESTING_RE.search(line) for line in body) - if len(methods) >= LARGE_SUITE_METHOD_THRESHOLD: + if len(methods) >= LARGE_SUITE_METHOD_THRESHOLD and not has_swift_testing: selectors.extend(methods) continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/cmux_unit_test_shard.py` around lines 14 - 25, The TEST_TOKEN_RE regex counts both `@Test` and func test tokens when determining if a suite exceeds LARGE_SUITE_METHOD_THRESHOLD, but XCTEST_METHOD_RE only extracts XCTest selectors in the large-suite splitting logic around lines 132-134, silently dropping Swift Testing `@Test` cases. Detect mixed suites that contain both `@Test` annotations and XCTest methods by checking if the test count from TEST_TOKEN_RE exceeds the count from XCTEST_METHOD_RE, and either skip the large-suite splitting for these mixed suites to keep them at suite level or modify the selector extraction logic to include both `@Test` and XCTest selectors when a suite is mixed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@scripts/ci/cmux_unit_test_shard.py`:
- Around line 14-25: The TEST_TOKEN_RE regex counts both `@Test` and func test
tokens when determining if a suite exceeds LARGE_SUITE_METHOD_THRESHOLD, but
XCTEST_METHOD_RE only extracts XCTest selectors in the large-suite splitting
logic around lines 132-134, silently dropping Swift Testing `@Test` cases. Detect
mixed suites that contain both `@Test` annotations and XCTest methods by checking
if the test count from TEST_TOKEN_RE exceeds the count from XCTEST_METHOD_RE,
and either skip the large-suite splitting for these mixed suites to keep them at
suite level or modify the selector extraction logic to include both `@Test` and
XCTest selectors when a suite is mixed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d4d5ea6-75e5-4018-b77b-e77ea46526f0
📒 Files selected for processing (7)
.github/workflows/ci.ymlscripts/ci/cmux_unit_test_shard.pyscripts/ci/run-in-console-session.shscripts/ci/xcodebuild_noninteractive.pytests/test_ci_cmux_unit_test_shard.pytests/test_ci_unit_test_spm_retry.shtests/test_ci_xcodebuild_noninteractive_helper.py
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1c14489. Configure here.
| if selected_tests_result == "passed" or saw_passing_terminal_summary: | ||
| return 0 | ||
| if selected_tests_result == "failed": | ||
| return POST_TEST_FAILED_EXIT_CODE |
There was a problem hiding this comment.
Failed tests masked as success
Medium Severity
After the post-test grace timeout, exit handling treats ** TEST SUCCEEDED ** in the rolling output buffer as success before checking whether the matched Selected tests summary was failed. A stale success marker in the 4KB window can make a failed shard run return exit code 0, so CI may skip failure analysis when EXIT_CODE is zero.
Reviewed by Cursor Bugbot for commit 1c14489. Configure here.
| XCTEST_METHOD_RE = re.compile( | ||
| r"^\s*(?:(?:final|private|fileprivate|internal|public)\s+)*" | ||
| r"func\s+(test[A-Za-z0-9_]*)\s*\(" | ||
| ) |
There was a problem hiding this comment.
XCTEST_METHOD_RE silently drops override func test… methods from large-suite splits
XCTEST_METHOD_RE only allows final|private|fileprivate|internal|public as leading modifiers. A method like override func testFoo() or public override func testFoo() will not match, so it will never appear in declaration.methods. For small suites this is harmless because the whole-suite selector covers it, but when a suite crosses LARGE_SUITE_METHOD_THRESHOLD the code falls into selectors.extend(methods) without including those override methods — they get no -only-testing argument and are silently skipped across every shard.
| XCTEST_METHOD_RE = re.compile( | |
| r"^\s*(?:(?:final|private|fileprivate|internal|public)\s+)*" | |
| r"func\s+(test[A-Za-z0-9_]*)\s*\(" | |
| ) | |
| XCTEST_METHOD_RE = re.compile( | |
| r"^\s*(?:(?:final|open|override|private|fileprivate|internal|public)\s+)*" | |
| r"func\s+(test[A-Za-z0-9_]*)\s*\(" | |
| ) |
Removes the confusing name/key crossover left by #6464+#6474: a job KEYED `tests` that reported as "app-host unit tests", plus a gate keyed `tests-required-status` that reported as `tests`. Now the names line up: - `app-host-unit-tests` (key) -> reports "app-host unit tests (N/4)" — the suite - `tests` (key) -> reports "tests" — the required aggregate gate No settings change: the gate still reports under the required name `tests`. The two `needs:` references to the old matrix key (the gate and ci-status) and the gate's needs["..."] lookup are updated accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s/splits) (#6519) * ci: aggregate required gates so renames/splits stop desyncing branch protection Branch protection requires status checks by literal name, but recent CI churn (unit-test sharding renamed the `tests` job to `app-host unit tests (N/4)` in #6464; jobs split across ci.yml and test-ios.yml; new suites added) kept moving those names out from under the static required-checks list — stranding old names ("expected" forever, blocking every PR) and leaving new suites ungated. Fix: gate via aggregate summary jobs that reference suites by their job KEY (immune to display-name/shard changes), one per workflow. - ci.yml `tests-required-status` (reported as `tests`, already required): now also needs `swift-package-tests` and `agent-session-web-resources`, so a failure in either blocks merge. Skipped (path-filtered) is still allowed. - test-ios.yml: new `ios-tests` aggregate over `detect-ios-changes`, `package-conventions-lint`, `mobile-core-package`, `ios-simulator`, same skip-tolerant logic. Settings follow-up (after merge): add `ios-tests` to the main ruleset's required status checks. The `tests` change needs no settings change (same name). Optional cleanup: the individual web/release contexts can stay or be folded into the aggregates later. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: rename the sharded job key tests -> app-host-unit-tests Removes the confusing name/key crossover left by #6464+#6474: a job KEYED `tests` that reported as "app-host unit tests", plus a gate keyed `tests-required-status` that reported as `tests`. Now the names line up: - `app-host-unit-tests` (key) -> reports "app-host unit tests (N/4)" — the suite - `tests` (key) -> reports "tests" — the required aggregate gate No settings change: the gate still reports under the required name `tests`. The two `needs:` references to the old matrix key (the gate and ci-status) and the gate's needs["..."] lookup are updated accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: track the tests->app-host-unit-tests rename in workflow guards The job-key rename moved the macOS app-host matrix to `app-host-unit-tests` and gave the key `tests` to the required aggregate gate. Update the guards that asserted on the old keys: - test_ci_self_hosted_guard.sh: assert the paid-macOS-runner requirement against `app-host-unit-tests` (the matrix), not `tests` (now a linux gate). - test_ci_change_areas.py: ci-status routed-jobs list and the gate-block test now reference `app-host-unit-tests` (matrix) and `tests` (gate). All workflow-guard-tests steps pass locally (self-hosted guard, change-areas, sharding validator). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
Validation
Note
Medium Risk
Test coverage now depends on sharding plus shard-1 focused gates being complete; mis-partitioning could skip suites. Post-test xcodebuild termination is intentional but changes pass/fail timing semantics for hung app-host runs.
Overview
macOS app-host unit tests now run as a 4-shard matrix (
fail-fast: false). Each shard gets its own DerivedData path and runs only a subset ofcmuxTestsvia-only-testingargs from newscripts/ci/cmux_unit_test_shard.py(weight-balanced buckets, method-level split for suites with ≥40 tests, duplicate detection). BrowserSystemProxyMirrorTests and GhosttyOptionAsAltModsTests stay out of shards and run only on shard 1 as dedicated non-tolerant gates. The main unit step usesrun-app-host-xcodebuild.sh(per-machine GUI test lock) instead of callingxcodebuild_noninteractive.pydirectly.Job splits: Swift package
swift testmoves toswift-package-tests; agent-session web build/test + generated resource diff moves toagent-session-web-resourceson Linux. CLI no-socket regressions stay on shard 1 after the unit build.ci-statusdepends on the new jobs.Hang mitigation:
xcodebuild_noninteractive.pygainsCMUX_XCODEBUILD_NONINTERACTIVE_POST_TEST_TIMEOUT_SECONDS(45s in CI)—after the “Selected tests” summary it can terminate a stuckxcodebuildand exit 0 on pass / 125 on fail.run-in-console-session.shforwards that env var. Unit test logs use per-shard paths underRUNNER_TEMPinstead of/tmp/test-output.txt.Guards: workflow step validates sharding;
tests/test_ci_cmux_unit_test_shard.py, updated SPM retry and noninteractive helper tests. Minor workflow cleanups (batchedGITHUB_OUTPUT,findfor SPM package paths).Reviewed by Cursor Bugbot for commit 1c14489. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit