refactor(e2e): centralize command environment profiles - #6363
Conversation
📝 WalkthroughWalkthroughIntroduces a new e2e fixture module centralizing environment-building logic (commandEnvironment, installedCommandEnvironment, testHomeEnvironment, sandboxCommandEnvironment), a corresponding test suite, and refactors six existing live e2e test files' testEnv helpers to delegate to the new testHomeEnvironment function instead of building environments inline. ChangesEnvironment Profile Centralization
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e/support/e2e-environment-profiles.test.ts (1)
44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMixed use of
path.sep/path.delimiterand POSIX literals.Line 44 builds
homewithpath.join(path.sep, ...)and line 48 splits onpath.delimiter, both platform-aware, which is reasonable for a cross-platform fixture. However per a prior repo learning, e2e/unit tests here run only on Linux (Windows is WSL-only and already POSIX-compliant), so the established convention in this test suite is to use literal://rather thanpath.delimiter/path.sep. Consider aligning with that convention for consistency with other test files, or confirm this new test file intentionally departs from it because the fixture itself is designed to be portable.Based on learnings, "prefer the established POSIX PATH separator
:... Do not replace it withpath.delimiterin these unit/integration tests, because they only run on Linux runners in CI here".Also applies to: 48-48
🤖 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 `@test/e2e/support/e2e-environment-profiles.test.ts` at line 44, The test fixture is using platform-aware path APIs where this suite’s convention is to use POSIX literals. In the e2e environment profile test, update the `home` construction in the test setup and the PATH splitting logic to use literal `/` and `:` consistently instead of `path.sep`/`path.delimiter`, matching the rest of the Linux-only test suite and keeping the fixture aligned with existing expectations.Source: Learnings
🤖 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.
Nitpick comments:
In `@test/e2e/support/e2e-environment-profiles.test.ts`:
- Line 44: The test fixture is using platform-aware path APIs where this suite’s
convention is to use POSIX literals. In the e2e environment profile test, update
the `home` construction in the test setup and the PATH splitting logic to use
literal `/` and `:` consistently instead of `path.sep`/`path.delimiter`,
matching the rest of the Linux-only test suite and keeping the fixture aligned
with existing expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e992e95c-e451-4b2a-8ab3-da99928c8371
📒 Files selected for processing (8)
test/e2e/fixtures/environment-profiles.tstest/e2e/live/bedrock-runtime-compatible-anthropic.test.tstest/e2e/live/cloud-inference.test.tstest/e2e/live/credential-migration.test.tstest/e2e/live/credential-sanitization.test.tstest/e2e/live/diagnostics.test.tstest/e2e/live/openclaw-skill-cli.test.tstest/e2e/support/e2e-environment-profiles.test.ts
cv
left a comment
There was a problem hiding this comment.
Maintainer review: the command environment profiles centralize existing E2E setup without changing precedence or credential boundaries, and the focused migrations are covered by the refreshed suite. Current-main synchronization cleared the stale growth failure; all exact-head checks, DCO, signatures, and review threads are clean.
## Summary Add immutable E2E command-environment profiles on top of the existing filtered availability boundary, and migrate the repeated test-HOME builders used by six live scenarios. ## Related Issue Closes NVIDIA#6356 Parent epic: NVIDIA#6346 ## Changes - Add filtered command, installed-CLI, test-HOME, and sandbox environment profiles. - Centralize noninteractive acceptance flags, default OpenShell gateway selection, and deduplicated `.local/bin` / `.npm-global/bin` PATH entries. - Preserve caller-override precedence and avoid `process.env` mutation. - Keep secret-bearing inference overlays as explicit caller data rather than embedding inference-mode semantics. - Migrate six exact/near-exact test-HOME environment builders. - Add support tests for filtering, precedence, sandbox identity, PATH composition, immutability, and secret overlays. ## Verification - [x] Signed/Verified commit; pre-commit, commit-msg, and pre-push hooks passed - [x] `npm run build:cli` - [x] `npm run typecheck:cli` - [x] `npm run lint` - [x] `e2e-environment-profiles.test.ts` (3 passed) - [x] No user-facing docs required - [x] No secrets or credentials committed --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added shared environment presets for CLI, home-directory, and sandboxed command runs. * Standardized test command environments so local CLI paths and required settings are applied consistently. * **Bug Fixes** * Improved end-to-end test reliability by removing duplicated environment setup across multiple scenarios. * **Tests** * Added coverage for environment profile behavior, including command setup, home-path handling, and sandbox-specific variables. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Summary
Add immutable E2E command-environment profiles on top of the existing filtered availability boundary, and migrate the repeated test-HOME builders used by six live scenarios.
Related Issue
Closes #6356
Parent epic: #6346
Changes
.local/bin/.npm-global/binPATH entries.process.envmutation.Verification
npm run build:clinpm run typecheck:clinpm run linte2e-environment-profiles.test.ts(3 passed)Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests