ci: split test matrix into privileged and non-privileged groups#272
Merged
Conversation
Move tests with zero Docker dependency (context, ide, readconfiguration, extends, outdated, machine, machineprovider) into a new integration-tests-unprivileged job that runs without sudo. This reduces the sudo surface to only tests that actually require root for Docker operations. The job name pattern is preserved so branch protection status checks remain compatible.
✅ Deploy Preview for devsydev canceled.
|
📝 WalkthroughWalkthroughGitHub Actions CI workflow is reorganized to split integration test execution: a new ChangesIntegration Test Job Split
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
These tests require root access despite having no Docker dependency. Both failed when running without sudo in CI. Move them back to the privileged integration-tests job.
![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 (2)
.github/workflows/pr-ci.yml (2)
153-158: 💤 Low valueConsider adding macOS to darwin conversion for future extensibility.
The OS detection doesn't convert "macos" to "darwin" like the privileged job does (lines 399-401). While all current unprivileged runners use ubuntu-latest, adding this conversion would maintain consistency and prevent issues if macOS runners are added to this job's matrix in the future.
♻️ Proposed alignment with privileged job
- name: get operating system lowercase id: os shell: bash run: | OS=$(echo "${{ runner.os }}" | tr '[:upper:]' '[:lower:]') + if [ "$OS" == "macos" ]; then + OS="darwin" + fi echo "runner_os=$OS" >> "$GITHUB_OUTPUT"🤖 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/pr-ci.yml around lines 153 - 158, The OS detection step "get operating system lowercase" only lowercases runner.os into the OS variable and writes runner_os, but it doesn't map "macos" to "darwin" like the privileged job does; update the shell logic in that step to check if the lowercased value equals "macos" and if so set OS="darwin" (otherwise keep the lowercased value) before echoing "runner_os=$OS" to $GITHUB_OUTPUT so macOS runners are normalized to darwin.
167-176: 💤 Low valueConsider adding Windows path handling for future extensibility.
The executable setup step doesn't handle Windows path conversions (forward slash conversion) like the privileged job does (lines 427-429). While all current unprivileged runners use ubuntu-latest, adding this handling would maintain consistency and prevent issues if Windows runners are added to this job's matrix in the future.
♻️ Proposed alignment with privileged job
- name: setup executable shell: bash working-directory: ./e2e run: | TEMP_DIR="${{ runner.temp }}" + if [ "${{ runner.os }}" == "Windows" ]; then + TEMP_DIR="${TEMP_DIR//\\//}" + fi + ls -R "$TEMP_DIR/devsy-bin/" mkdir -p ./bin/🤖 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/pr-ci.yml around lines 167 - 176, The step "setup executable" currently uses TEMP_DIR and Unix find/cp commands without handling Windows path formats; update this step to mirror the privileged job's Windows handling by detecting runner.os == 'Windows' and converting TEMP_DIR to a Windows-friendly path before using it (e.g., set a TEMP_DIR_WIN or converted_temp_dir variable via cygpath or equivalent) and then use that converted variable in the subsequent ls, find and cp commands so the find "$TEMP_DIR/..." calls work on Windows runners as well.
🤖 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/pr-ci.yml:
- Around line 122-187: Add an explicit confirmation that the five unprivileged
test labels (context, readconfiguration, extends, outdated, machine) were run
and passed without sudo by updating the PR description or adding a follow-up PR
comment referencing the integration-tests-unprivileged job; state that you
executed the matrix job locally or on CI, list each label as "passed (no sudo)",
and if you used DEVSY-105 as the source of truth include a direct reference/link
to DEVSY-105 and briefly note the verification method (CI run ID or local
reproduction) so reviewers can trace the claim.
---
Nitpick comments:
In @.github/workflows/pr-ci.yml:
- Around line 153-158: The OS detection step "get operating system lowercase"
only lowercases runner.os into the OS variable and writes runner_os, but it
doesn't map "macos" to "darwin" like the privileged job does; update the shell
logic in that step to check if the lowercased value equals "macos" and if so set
OS="darwin" (otherwise keep the lowercased value) before echoing "runner_os=$OS"
to $GITHUB_OUTPUT so macOS runners are normalized to darwin.
- Around line 167-176: The step "setup executable" currently uses TEMP_DIR and
Unix find/cp commands without handling Windows path formats; update this step to
mirror the privileged job's Windows handling by detecting runner.os == 'Windows'
and converting TEMP_DIR to a Windows-friendly path before using it (e.g., set a
TEMP_DIR_WIN or converted_temp_dir variable via cygpath or equivalent) and then
use that converted variable in the subsequent ls, find and cp commands so the
find "$TEMP_DIR/..." calls work on Windows runners as well.
🪄 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: 394d029b-e2e5-4d5b-aa2d-c7da1c7706a5
📒 Files selected for processing (1)
.github/workflows/pr-ci.yml
Comment on lines
+122
to
+187
| integration-tests-unprivileged: | ||
| name: Test ${{ matrix.label }} on ${{ matrix.runner }} | ||
| needs: [can-read-secret, build-cli] | ||
| needs: [build-cli] | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| - label: context | ||
| runner: ubuntu-latest | ||
| free-disk-space: false | ||
| install-kind: false | ||
| requires-secret: false | ||
|
|
||
| - label: ide | ||
| - label: readconfiguration | ||
| runner: ubuntu-latest | ||
| free-disk-space: false | ||
| install-kind: false | ||
| requires-secret: false | ||
|
|
||
| - label: integration | ||
| - label: extends | ||
| runner: ubuntu-latest | ||
|
|
||
| - label: outdated | ||
| runner: ubuntu-latest | ||
|
|
||
| - label: machine | ||
| runner: ubuntu-latest | ||
|
|
||
| runs-on: ${{ matrix.runner }} | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: setup Go | ||
| uses: actions/setup-go@v6 | ||
| with: | ||
| go-version-file: go.mod | ||
|
|
||
| - name: get operating system lowercase | ||
| id: os | ||
| shell: bash | ||
| run: | | ||
| OS=$(echo "${{ runner.os }}" | tr '[:upper:]' '[:lower:]') | ||
| echo "runner_os=$OS" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: download CLI artifacts | ||
| uses: actions/download-artifact@v8 | ||
| with: | ||
| pattern: devsy-${{ steps.os.outputs.runner_os }} | ||
| path: ${{ runner.temp }}/devsy-bin/ | ||
| merge-multiple: true | ||
|
|
||
| - name: setup executable | ||
| shell: bash | ||
| working-directory: ./e2e | ||
| run: | | ||
| TEMP_DIR="${{ runner.temp }}" | ||
| ls -R "$TEMP_DIR/devsy-bin/" | ||
| mkdir -p ./bin/ | ||
| find "$TEMP_DIR/devsy-bin/" -type f -name "devsy-*" -exec cp {} ./bin \; | ||
| find ./bin -name "devsy-*" -exec chmod +x {} \; | ||
| ls -R ./bin/ | ||
|
|
||
| - name: run test | ||
| shell: bash | ||
| working-directory: ./e2e | ||
| env: | ||
| GH_USERNAME: ${{ github.repository_owner }} | ||
| GH_ACCESS_TOKEN: ${{ github.token }} | ||
| GH_CREDENTIAL_USERNAME: x-access-token | ||
| run: | | ||
| go test -v -ginkgo.v -timeout 1500s --ginkgo.label-filter="${{ matrix.label }}" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check git log for evidence of test execution and results for moved labels
echo "=== Checking commit messages for test execution evidence ==="
git log --oneline --grep="test\|fail\|pass" -i --since="2 weeks ago" | head -20
echo -e "\n=== Checking for test result artifacts or CI logs mentioning these labels ==="
for label in context readconfiguration extends outdated machine; do
echo "Searching for: $label"
rg -i "test.*$label|$label.*test" --type=yaml --type=md -C 2 || echo " No matches found"
done
echo -e "\n=== Checking PR description or comments for verification notes ==="
gh pr view 272 --json body,comments --jq '.body, .comments[].body' | rg -i "verify|test.*without.*sudo|unprivileged" -C 2 || echo " No verification notes found"Repository: devsy-org/devsy
Length of output: 1565
Request explicit confirmation that the five unprivileged tests pass without sudo.
The PR summary indicates these tests are based on DEVSY-105 investigation, which found ~60% of test labels have zero Docker dependency. The selective movement of ide and machineprovider back to the privileged job confirms that tests are being executed and failures are detected. However, there is no explicit documentation confirming that context, readconfiguration, extends, outdated, and machine have been verified to pass without sudo.
Please confirm in a comment that these five tests execute successfully in the unprivileged job, or link to DEVSY-105 for reference.
🤖 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/pr-ci.yml around lines 122 - 187, Add an explicit
confirmation that the five unprivileged test labels (context, readconfiguration,
extends, outdated, machine) were run and passed without sudo by updating the PR
description or adding a follow-up PR comment referencing the
integration-tests-unprivileged job; state that you executed the matrix job
locally or on CI, list each label as "passed (no sudo)", and if you used
DEVSY-105 as the source of truth include a direct reference/link to DEVSY-105
and briefly note the verification method (CI run ID or local reproduction) so
reviewers can trace the claim.
This was referenced May 25, 2026
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.
Summary
integration-tests-unprivileged(no sudo) andintegration-tests(keeps sudo for Docker-dependent tests)Test <label> on <runner>) is preserved for branch protection compatibilitySummary by CodeRabbit
Note: This release contains no user-facing changes or feature updates.