Repository navigation
[#366] Added macOS to scaffold devtools CI matrix. - #373
Conversation
📝 WalkthroughWalkthroughThis PR extends CI support to macOS runners for the scaffold-test-devtools job. It adds OS-conditional dependency setup (Homebrew SQLite for macOS, apt for Linux), refactors Ahoy CLI installation with platform detection, enables Selenium+Chromium startup for Linux testing, and gates resource-intensive functional JavaScript tests on macOS via the SCAFFOLD_SKIP_FUNCTIONAL_JAVASCRIPT environment variable. ChangesmacOS CI Support with Conditional Test Gating
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #373 +/- ##
=======================================
Coverage 55.88% 55.88%
=======================================
Files 4 4
Lines 467 467
=======================================
Hits 261 261
Misses 206 206 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…te when Selenium is unavailable.
![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
🧹 Nitpick comments (1)
.github/workflows/scaffold-test.yml (1)
27-28: ⚡ Quick winConsider setting
persist-credentials: falsefor security.The checkout action does not explicitly set
persist-credentials: false. This means the GitHub token persists in the local git config and remains available to subsequent steps, which increases the attack surface if a later step is compromised.🔒 Proposed fix
- name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + persist-credentials: falseAs per static analysis, the zizmor tool flagged this as a credential persistence risk through GitHub Actions artifacts (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/scaffold-test.yml around lines 27 - 28, The checkout step named "Checkout code" currently uses actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd without disabling credential persistence; update that step (the actions/checkout usage) to include persist-credentials: false so the GITHUB_TOKEN is not written to the repo git config and cannot be used by later steps.
🤖 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/scaffold-test.yml:
- Around line 60-71: The "Install Ahoy" step incorrectly remaps macOS names and
lacks explicit download-failure handling; update the os/architecture mapping so
`os` remains the raw `uname -s` lowercased (allowing `darwin`), keep the
architecture logic for `architecture`, and wrap the `sudo wget
"https://github.com/ahoy-cli/ahoy/releases/download/v2.1.1/ahoy-bin-${os}-${architecture}"
-O /usr/local/bin/ahoy` invocation with an explicit failure check (e.g., test
wget exit and echo a clear error + exit 1) so download errors abort clearly;
reference the "Install Ahoy" step and the variables `os`, `architecture`, and
the `wget` invocation when making the change.
---
Nitpick comments:
In @.github/workflows/scaffold-test.yml:
- Around line 27-28: The checkout step named "Checkout code" currently uses
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd without disabling
credential persistence; update that step (the actions/checkout usage) to include
persist-credentials: false so the GITHUB_TOKEN is not written to the repo git
config and cannot be used by later steps.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e71f9b5c-a15e-49fe-a0a8-a6277ac538ed
📒 Files selected for processing (3)
.github/workflows/scaffold-test.yml.scaffold/tests/src/Functional/AhoyTest.php.scaffold/tests/src/Functional/MakeTest.php
Closes #366
Summary
Adds
macos-latestas a second OS dimension to thescaffold-test-devtoolsCI job matrix, doubling the combinations from 5 (1 OS × 5 groups) to 10 (2 OS × 5 groups). Because macOS GitHub Actions runners do not support Docker, the Selenium/Chromium container is moved from a job-levelservices:block to a conditional step that runs only on Linux when the group requires it (p3 or p4). macOS runs receiveSCAFFOLD_SKIP_FUNCTIONAL_JAVASCRIPT=1so the test harness skips both the dedicated FunctionalJavascript step and the broaderahoy test/make testsmoke step (which would otherwise try to reach Selenium atlocalhost:4444). The sqlite3 upgrade is split into a Linux step (source build withSQLITE_ENABLE_COLUMN_METADATA) and a macOS step (brew install sqlite3with build flags exported via$GITHUB_ENV). The pre-existingInstall Ahoystep's inlinecasesyntax inside$(...)is split across lines so it parses under macOS's bash 3.2. Coverage artifact names are disambiguated by OS so the two runners do not collide on upload.Changes
.github/workflows/scaffold-test.ymlruns-on: ubuntu-latestwithruns-on: ${{ matrix.os }}and addedos: [ubuntu-latest, macos-latest]to the matrix.services:block (Docker service containers are unsupported on macOS runners).Start Chromium (Linux only)step that runsdocker runonly whenrunner.os == 'Linux'andmatrix.groupisp3orp4, with a 30-second readiness poll before proceeding.wget+ source build) and a macOS step (brew install sqlite3withLDFLAGS/CPPFLAGS/PKG_CONFIG_PATHexported via$GITHUB_ENV).Install Ahoystep'sos/architecturedetection: split the inlinecase $(uname -m) in ... esacinto a multi-linecaseblock. The single-line form parses under bash 4+ on Linux but errors on macOS bash 3.2 (syntax error near unexpected token ';;').SCAFFOLD_SKIP_FUNCTIONAL_JAVASCRIPT: ${{ runner.os == 'macOS' && '1' || '0' }}to the test step'senvblock....-${{ matrix.group }}to...-${{ matrix.os }}-${{ matrix.group }}..scaffold/tests/src/Functional/AhoyTest.phpandMakeTest.phprunFunctionalJavascriptTests()that checksgetenv('SCAFFOLD_SKIP_FUNCTIONAL_JAVASCRIPT') === '1'and writes a marker line to STDERR before returning.ahoy test/make testcall intestWorkflow().ahoy testandmake testinvoke PHPUnit with no filter and therefore run the FunctionalJavascript suite as well; without Selenium reachable, that step errors withCould not connect to localhost port 4444. The guard skips both that command and itsbrowser_outputdirectory assertion when the env var is set.Before / After
Before - 1 OS × 5 groups = 5 matrix jobs, Selenium always pulled as a job-level service:
After - 2 OS × 5 groups = 10 matrix jobs, Selenium started only where needed:
X= group runs.+ DOCK= Selenium container started in a Linux-only step before the test step.SKIP= bothahoy test/make testand the dedicatedtest-functional-javascriptstep are skipped via env var; every other workflow step (assemble, start, provision, lint, test-unit, test-kernel, test-functional) still runs.