Repository navigation
ci: let the pull-request macOS lane move pools without breaking Xcode selection #13923
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
684b837
0762d8e
694e241
b013039
d2496a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3920,17 +3920,45 @@ def test_r2_transport_is_an_explicit_optional_remote_broker() -> None: | |||||||||||
| assert "CI_ARTIFACT_R2_URL: ${{ vars.CI_ARTIFACT_R2_URL }}" in r2_step | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| PR_LANE_XCODE_PIN = ( | ||||||||||||
| "${{ github.event_name == 'pull_request' " | ||||||||||||
| "&& (vars.CMUX_CI_XCODE_APP_PR || vars.CMUX_CI_XCODE_APP_MACOS_15) " | ||||||||||||
| "|| vars.CMUX_CI_XCODE_APP_MACOS_15 }}" | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def test_macos_jobs_use_lane_specific_xcode_pin_vars() -> None: | ||||||||||||
| # A pull-request job picks its pool through MACOS_RUNNER_PR, and the two | ||||||||||||
| # macOS images carry different Xcodes: macos-15 ships CMUX_CI_XCODE_APP_MACOS_15 | ||||||||||||
| # and macos-26 ships CMUX_CI_XCODE_APP_MACOS_26. scripts/select-ci-xcode.sh | ||||||||||||
| # exits non-zero on a pinned path that is not installed, so a pin that does | ||||||||||||
| # not follow the same lane turns a routing change into a failed job rather | ||||||||||||
| # than a queued one. Require the pin to resolve through the pull-request | ||||||||||||
| # escape hatch exactly as runs-on does, with the macos-15 pin as the default | ||||||||||||
| # on both branches so an unset variable keeps today's behavior. | ||||||||||||
| for job_name in [ | ||||||||||||
| "app-host-unit-tests", | ||||||||||||
| "macos-compile-admission", | ||||||||||||
| "swift-package-tests", | ||||||||||||
| "tests-build-and-lag", | ||||||||||||
| ]: | ||||||||||||
| block = workflow_job_block(job_name, MACOS_WORKFLOW) | ||||||||||||
| assert "CMUX_CI_XCODE_APP: ${{ vars.CMUX_CI_XCODE_APP_MACOS_15 }}" in block | ||||||||||||
| assert f"CMUX_CI_XCODE_APP: {PR_LANE_XCODE_PIN}" in block, job_name | ||||||||||||
| assert "vars.CMUX_CI_XCODE_APP_MACOS_26" not in block, job_name | ||||||||||||
|
Comment on lines
+3923
to
+3946
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '3910,3960p' tests/test_ci_change_areas.py
sed -n '2150,2200p' .github/workflows/ci-macos.yml
rg -n 'CMUX_CI_HELPER_XCODE_APP_PR|CMUX_CI_HELPER_XCODE_APP_MACOS_15|release_build' tests/test_ci_change_areas.pyRepository: manaflow-ai/cmux Length of output: 10116 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- relevant test helpers and assertions ---'
rg -n -C 4 'CMUX_CI_(HELPER_)?XCODE_APP|test_macos_jobs_use_lane_specific_xcode_pin_vars|workflow_job_block|release_build' tests/test_ci_change_areas.py
printf '%s\n' '--- workflow job declarations and pin envs ---'
rg -n -C 6 '^[[:space:]]{2}(swift-package-tests|release-build|app-host-unit-tests|macos-compile-admission|tests-build-and-lag):|CMUX_CI_(HELPER_)?XCODE_APP|MACOS_RUNNER_PR' .github/workflows/ci-macos.yml
printf '%s\n' '--- nearby release_build fixture/path tests ---'
sed -n '1,190p' tests/test_ci_change_areas.py
sed -n '430,510p' tests/test_ci_change_areas.py
sed -n '740,795p' tests/test_ci_change_areas.py
sed -n '2650,2700p' tests/test_ci_change_areas.py
sed -n '3780,3860p' tests/test_ci_change_areas.pyRepository: manaflow-ai/cmux Length of output: 41909 🤖 get_repo_knowledge executed:
Length of output: 47485 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- exact helper-pin occurrences in tests ---'
rg -n -C 2 'CMUX_CI_HELPER_XCODE_APP|HELPER_XCODE|helper.*Xcode|XCODE_APP_PR|XCODE_APP_MACOS_15' tests --glob '*.py'
printf '%s\n' '--- exact workflow pin occurrences ---'
rg -n -C 3 'CMUX_CI_(HELPER_)?XCODE_APP' .github/workflows/ci-macos.yml
printf '%s\n' '--- complete swift-package-tests block ---'
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/ci-macos.yml')
lines = p.read_text(encoding='utf-8').splitlines()
start = next(i for i, line in enumerate(lines) if line == ' swift-package-tests:')
end = next(i for i in range(start + 1, len(lines)) if lines[i].startswith(' ') and not lines[i].startswith(' '))
print('\n'.join(f'{i+1}:{lines[i]}' for i in range(start, end)))
PY
printf '%s\n' '--- relevant diff summary and changed hunks ---'
git diff --stat -- tests/test_ci_change_areas.py .github/workflows/ci-macos.yml
git diff -- tests/test_ci_change_areas.py .github/workflows/ci-macos.yml | sed -n '1,240p'Repository: manaflow-ai/cmux Length of output: 31169 Assert the lane-specific helper Xcode expression. The topology test only checks that Suggested fix PR_LANE_XCODE_PIN = (
"${{ github.event_name == 'pull_request' "
"&& (vars.CMUX_CI_XCODE_APP_PR || vars.CMUX_CI_XCODE_APP_MACOS_15) "
"|| vars.CMUX_CI_XCODE_APP_MACOS_15 }}"
)
+PR_LANE_HELPER_XCODE_PIN = (
+ "${{ github.event_name == 'pull_request' "
+ "&& (vars.CMUX_CI_HELPER_XCODE_APP_PR || vars.CMUX_CI_HELPER_XCODE_APP_MACOS_15) "
+ "|| vars.CMUX_CI_HELPER_XCODE_APP_MACOS_15 }}"
+)
...
+ package_block = workflow_job_block("swift-package-tests", MACOS_WORKFLOW)
+ assert f"CMUX_CI_HELPER_XCODE_APP: {PR_LANE_HELPER_XCODE_PIN}" in package_block
+
release_block = workflow_job_block("release-build", MACOS_WORKFLOW)🤖 Prompt for AI Agents |
||||||||||||
| assert 'CMUX_CI_REQUIRED_MACOS_SDK_MAJOR: "26"' in block | ||||||||||||
|
|
||||||||||||
| # swift-package-tests links the Release Ghostty CLI helper with Zig, which | ||||||||||||
| # Zig 0.15.2 cannot do on macOS 26, so it stays on the macos-15 pool on | ||||||||||||
| # every event and keeps the unconditional macos-15 pins. Moving it onto the | ||||||||||||
| # pull-request lane would hand MACOS_RUNNER_PR a job it must not move. | ||||||||||||
| package_block = workflow_job_block("swift-package-tests", MACOS_WORKFLOW) | ||||||||||||
| assert "vars.MACOS_RUNNER_PR" not in package_block | ||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '2148,2200p' .github/workflows/ci-macos.yml
sed -n '3910,3970p' tests/test_ci_change_areas.py
sed -n '270,335p' tests/test_ci_self_hosted_guard.sh
rg -n 'workflow_job_block|check_release_helper_artifact_from_package_lane|saw_pr_lane' tests/test_ci_change_areas.py tests/test_ci_self_hosted_guard.shRepository: manaflow-ai/cmux Length of output: 16867 🏁 Script executed: printf '%s\n' '--- workflow_job_block ---'
sed -n '1578,1605p' tests/test_ci_change_areas.py | nl -ba -v1578
printf '%s\n' '--- Python guard ---'
sed -n '3948,3961p' tests/test_ci_change_areas.py | nl -ba -v3948
printf '%s\n' '--- shell guard ---'
sed -n '284,321p' tests/test_ci_self_hosted_guard.sh | nl -ba -v284
printf '%s\n' '--- workflow job ---'
awk 'BEGIN { found=0; n=0 } /^ swift-package-tests:/ { found=1 } found { printf "%5d %s\n", NR, $0; n++; if (n==40) exit }' .github/workflows/ci-macos.ymlRepository: manaflow-ai/cmux Length of output: 7836 Restrict PR-runner checks to the The package block includes a comment naming 🐛 Suggested fix--- a/tests/test_ci_change_areas.py
+++ b/tests/test_ci_change_areas.py
@@
package_block = workflow_job_block("swift-package-tests", MACOS_WORKFLOW)
- assert "vars.MACOS_RUNNER_PR" not in package_block
+ assert (
+ "runs-on: ${{ vars.MACOS_RUNNER_DUAL_XCODE || 'blacksmith-6vcpu-macos-15' }}"
+ in package_block
+ )
--- a/tests/test_ci_self_hosted_guard.sh
+++ b/tests/test_ci_self_hosted_guard.sh
@@
- in_job && /vars\.MACOS_RUNNER_PR/ { saw_pr_lane=1 }
+ in_job && /^[[:space:]]*runs-on:/ && /vars\.MACOS_RUNNER_PR/ { saw_pr_lane=1 }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
| assert "CMUX_CI_XCODE_APP: ${{ vars.CMUX_CI_XCODE_APP_MACOS_15 }}" in package_block | ||||||||||||
| assert ( | ||||||||||||
| "CMUX_CI_HELPER_XCODE_APP: ${{ vars.CMUX_CI_HELPER_XCODE_APP_MACOS_15 }}" | ||||||||||||
| in package_block | ||||||||||||
| ) | ||||||||||||
| assert 'CMUX_CI_REQUIRED_MACOS_SDK_MAJOR: "26"' in package_block | ||||||||||||
|
|
||||||||||||
| release_block = workflow_job_block("release-build", MACOS_WORKFLOW) | ||||||||||||
| assert "CMUX_CI_XCODE_APP: ${{ vars.CMUX_CI_XCODE_APP_MACOS_26 }}" in release_block | ||||||||||||
| assert 'CMUX_CI_REQUIRED_MACOS_SDK_MAJOR: "26"' in release_block | ||||||||||||
|
|
||||||||||||
Uh oh!
There was an error while loading. Please reload this page.