test(e2e): add environment phase fixture#5002
Conversation
|
Warning Review limit reached
More reviews will be available in 29 minutes and 7 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an EnvironmentPhaseFixture that checks Nemoclaw CLI and Docker runtime availability, a filtered probe environment helper, HostCliClient API updates, test-framework wiring, re-exports, and comprehensive tests validating install/runtime expectation behavior and probe env scoping. ChangesEnvironment Phase Fixture
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: None Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Scenario Advisor RecommendationRequired scenario E2E: Dispatch required scenario E2E:
Full scenario advisor summaryE2E Scenario AdvisorBase: Required scenario E2E
Optional scenario E2E
Relevant changed files
|
PR Review AdvisorFindings: 0 needs attention, 0 worth checking, 0 nice ideas Consider writing more tests for
This is an automated advisory review. A human maintainer must make the final merge decision. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e-scenario/framework-tests/e2e-phase-environment.test.ts (1)
163-180: 💤 Low valueConsider adding test coverage for optional Docker succeeding.
The test suite comprehensively covers required/missing Docker scenarios, but the case where an optional Docker runtime probe succeeds (e.g.,
macos-docker-optionalwith Docker available) is not explicitly tested. The logic is straightforward (would return withavailable: true), but explicit coverage would complete the matrix.Optional test case
+ it("records optional Docker as available when present", async () => { + const runner = new FakeRunner(); + runner.enqueue(shellResult(0, "nemoclaw v0.0.0\n")); + runner.enqueue(shellResult(0, "Docker is available\n")); + const environment = new EnvironmentPhaseFixture(new HostCliClient(runner)); + + const ready = await environment.assertReady({ + ...cloudOpenClawEnvironment, + platform: "macos-local", + runtime: "macos-docker-optional", + }); + + expect(ready.docker).toMatchObject({ + id: "macos-docker-optional", + expectation: "optional", + available: true, + }); + }); + it("rejects unsupported install and runtime IDs", async () => {🤖 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-scenario/framework-tests/e2e-phase-environment.test.ts` around lines 163 - 180, Add a new test that verifies an optional Docker runtime probe can succeed: create a FakeRunner and EnvironmentPhaseFixture (using HostCliClient) with the scenario config spread from cloudOpenClawEnvironment but set runtime to "macos-docker-optional", enqueue a shellResult that simulates a successful docker response (e.g., version output), call environment.assertReady(...) and assert it resolves and the returned environment/runtime probe indicates available: true; reference EnvironmentPhaseFixture, HostCliClient, FakeRunner, environment.assertReady and cloudOpenClawEnvironment to locate where to add the assertion.
🤖 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-scenario/framework-tests/e2e-phase-environment.test.ts`:
- Around line 163-180: Add a new test that verifies an optional Docker runtime
probe can succeed: create a FakeRunner and EnvironmentPhaseFixture (using
HostCliClient) with the scenario config spread from cloudOpenClawEnvironment but
set runtime to "macos-docker-optional", enqueue a shellResult that simulates a
successful docker response (e.g., version output), call
environment.assertReady(...) and assert it resolves and the returned
environment/runtime probe indicates available: true; reference
EnvironmentPhaseFixture, HostCliClient, FakeRunner, environment.assertReady and
cloudOpenClawEnvironment to locate where to add the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 36e898e7-7081-4592-bf47-e0c68a844dfc
📒 Files selected for processing (6)
test/e2e-scenario/framework-tests/e2e-clients.test.tstest/e2e-scenario/framework-tests/e2e-phase-environment.test.tstest/e2e-scenario/framework/clients/host.tstest/e2e-scenario/framework/e2e-test.tstest/e2e-scenario/framework/phases/environment.tstest/e2e-scenario/framework/phases/index.ts
2 participants
Summary
Adds a typed environment phase fixture for Vitest E2E scenarios so install/runtime readiness can move out of shell-only orchestration. This first stack layer verifies the NemoClaw CLI and Docker runtime modes from typed scenario environment metadata.
Related Issue
Refs #4941
Refs #4990
Changes
EnvironmentPhaseFixturewith typed install and Docker runtime readiness checks.environmenton the shared Vitest E2E scenario context.HostCliClientreport its configured CLI path and run CLI availability checks with a scoped allowlisted host env.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit