Repository navigation
fix(test): align Compose UID expectations with Linux remapping - #1455
Conversation
✅ Deploy Preview for devsydev canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to This change aligns the Compose test UID/GID expectations with Linux-only remapping and adds focused CI checks. It does not change production behavior. No merge-blocking issue was found; hosted CI results still need to confirm the new steps pass. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #1452 requires the UID expectation helper, pure 22-case coverage, and two workflow validation steps. The change summary shows those implementation requirements. The issue also requires hosted ex… |
Complete and record the required exact-head Ubuntu and macOS matrix runs, the native 1001:1001 www-data run, and the unchanged complete root Compose and final-head checks. Diagnose any failure without weakening the stated assertions or ga… |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (3 passed)
-
Check name Status Explanation Out of Scope Changes check The change summary limits the changes to e2e/tests/up-docker-compose/helper.go,e2e/tests/up-docker-compose/helper_test.go, and.github/workflows/pr-ci.yml. These paths and changes implement the…Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: correcting Compose UID expectations to match Linux UID remapping.
Full details: Linked Issues check
-
Explanation
Issue
#1452requires the UID expectation helper, pure 22-case coverage, and two workflow validation steps. The change summary shows those implementation requirements. The issue also requires hosted execution of all 22 cases on Ubuntu and macOS, the native non-sudo 1001:1001www-datacase with the JSON guard, and the unchanged complete root Compose run with final-head checks. The current PR description states that these hosted and final-head validations remain pending.Resolution
Complete and record the required exact-head Ubuntu and macOS matrix runs, the native 1001:1001
www-datarun, and the unchanged complete root Compose and final-head checks. Diagnose any failure without weakening the stated assertions or gates.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
-
✨ Simplify code
-
-
- Commit to this branch
- Create a new PR
-
-
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
✅ Deploy Preview for images-devsy-sh canceled.
|
|
@coderabbitai full review @greptileai please review this pull request at current head 77ea204. This is a review request only; the PR remains draft pending validation and review completion. |
|
 |
|
@coderabbitai full review Please review current head 77ea204 after the reported review cooldown. |
✅ Action performedFull review finished. |
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
|
@coderabbitai full review Please review the current integrated head a98aadf, including all three changed files against main 511d216. The previous review covered 77ea204 and predates this integration. |
|
@greptileai Please review the current integrated head a98aadf against main 511d216, including the combined workflow and Compose UID expectations. |
✅ Action performedFull review finished. |
1 participant
Compose UID/GID tests currently expect host IDs for every non-root host, although production deliberately remaps IDs only on Linux. For example, a Darwin host with 501:20 should keep www-data's 33:33 and vscode's 1001:1001. This corrects the test oracle while preserving production behavior, fixture images, and existing SSH username, content, and ownership assertions.
The change adds a pure expectation helper and 22 cases covering both fixtures, root/non-root and platform selection, matching/distinct IDs, and UID/GID edge cases. Two additive workflow steps execute that matrix explicitly on the existing Ubuntu/macOS unit rows and run the existing www-data case as the native non-sudo 1001:1001 Linux runner. The focused step verifies identity/access and requires exactly one passed, non-dry-run spec in one attempt. Existing complete root Compose execution, timeouts, and required checks remain unchanged.
Head
a98aadf61d37a2c4a662eb7c9231a64cc30a61f3integrates main511d21603e602d3081a7df8a19521b481b45adeb. The net change remains three files, 147 insertions and 18 deletions. Both UID Go files are unchanged from the previous published head; the merged MicroSandbox resource changes and shutdown sidecar pins are preserved exactly as on main.Local validation passed on the integrated tree: all 22 matrix cases on macOS, scoped unit/race tests for Compose helpers and Docker driver, vet, final-head CI-parity lint, formatting, YAML/actionlint/security and normal commit-message/pre-push hooks. Independent scoped reviews and fresh local CodeRabbit found no actionable issues. CodeScene checked both Go files with no new/worsened findings; helper score remains 9.09 and the test scores 10.0. Workflow YAML is unsupported by CodeScene, so no YAML score is claimed. The report guard previously passed eight synthetic accept/reject checks; those are parser validation, not integration execution. The merge commit is personally signed and GitHub verified.
The previous head
77ea20402323a5067289afe6e603b1fbc02b8df8completed the hosted Ubuntu/macOS matrices, native www-data remapping, full root Compose and final-head reviews. Those results do not establish acceptance for the new integrated head. Its Ubuntu/macOS matrix execution, native www-data 33:33 → 1001:1001, unchanged complete root Compose, all required checks and final-head reviews remain pending. Root execution does not prove non-root remapping; vscode already matches runner 1001:1001 and does not provide changed-ID coverage.Earlier integration evidence remains disclosed: focused Darwin UID cases passed 2/2; full Darwin Compose passed 51/55 with feature-permission and package-download failures. A real Linux 1000:1000 run passed www-data but failed vscode SSH because ubuntu/vscode shared UID1000. That production collision remains separate; this test-only change does not fix it or delete fixture accounts.
Closes #1452
Summary by CodeRabbit