test(e2e): migrate test-hermes-root-entrypoint-smoke.sh to vitest [IND-4]#5220
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Docker-based Hermes root-entrypoint Vitest e2e smoke test and integrates it into CI: replaces the bash smoke step in the sandbox job with a Node/Vitest flow, adds a dedicated ChangesHermes Root-Entrypoint E2E Smoke Test & CI Integration
Sequence Diagram(s)sequenceDiagram
participant TestHarness as E2E Test Harness
participant DockerProbe as DockerProbe
participant DockerDaemon as Docker Daemon
participant CleanContainer as Clean Variant
participant LegacyContainer as Legacy Variant
TestHarness->>DockerProbe: requireDocker / docker info
DockerProbe->>DockerDaemon: docker info
TestHarness->>DockerProbe: build/pull images (docker build / image inspect)
DockerProbe->>DockerDaemon: docker build / docker image inspect
TestHarness->>DockerProbe: start CleanContainer (/usr/local/bin/nemoclaw-start)
DockerProbe->>DockerDaemon: docker run
DockerDaemon->>CleanContainer: container running
TestHarness->>DockerProbe: start LegacyContainer (bootstrap stale pid/locks, then start)
DockerProbe->>DockerDaemon: docker run /bin/bash (legacy setup)
DockerDaemon->>LegacyContainer: container running
TestHarness->>CleanContainer: poll GET /health
CleanContainer->>TestHarness: health OK
TestHarness->>LegacyContainer: poll GET /health
LegacyContainer->>TestHarness: health OK
TestHarness->>DockerProbe: inspect/logs/exec for assertions
DockerProbe->>DockerDaemon: docker inspect / docker logs / docker exec
TestHarness->>TestHarness: emit scenario.json and scenario-result.json
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/e2e-scenario/live/hermes-root-entrypoint-smoke.test.ts`:
- Around line 264-275: The negative log assertions can false-pass if
/tmp/gateway.log is missing; update the expectContainerSh invocations so they
first assert the log file exists and fail if it does not, then run the negative
grep. Concretely, change the command strings passed to expectContainerSh (the
calls referencing probe, container and the messages about "PID file race lost"
and "Could not load config.yaml") to a compound shell command that checks test
-f /tmp/gateway.log (or [ -f /tmp/gateway.log ]) and exits non‑zero when
missing, then performs the ! grep -F '...' /tmp/gateway.log so absence of the
file causes the test to fail rather than silently pass.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5229f8db-e75d-4a35-82f9-86751270bee9
📒 Files selected for processing (2)
.github/workflows/sandbox-images-and-e2e.yamltest/e2e-scenario/live/hermes-root-entrypoint-smoke.test.ts
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. |
…es-root-entrypoint-smoke
Selective E2E Results — ✅ All requested jobs passedRun: 27349418414
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/e2e-vitest-scenarios.yaml:
- Around line 273-275: The hermes-root-entrypoint-smoke-vitest job currently
runs regardless of the matrix-generation outcome; update the job to depend on
and gate by the generate-matrix job by adding needs: generate-matrix and
extending its if condition to require needs.generate-matrix.result == 'success'
in addition to the existing scenarios check so the job only runs when matrix
generation succeeded.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6b432862-9d3d-45ad-9457-0adcd297848c
📒 Files selected for processing (1)
.github/workflows/e2e-vitest-scenarios.yaml
…es-root-entrypoint-smoke # Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27377301547
|
…es-root-entrypoint-smoke # Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27386801354
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/e2e-vitest-scenarios.yaml:
- Around line 1274-1293: The PR added hermes-root-entrypoint-smoke-vitest to
report-to-pr.needs but the workflow-boundary validator's required set wasn't
updated; edit the validator (the required-needs array in workflow-boundary.mts)
to include "hermes-root-entrypoint-smoke-vitest" so the validator enforces that
this job remains in report-to-pr.needs; ensure the string matches exactly the
entry added in the workflow and run the validator tests to confirm.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ddc7fbf4-4d33-46f2-a013-f56c0485d43e
📒 Files selected for processing (2)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/live/hermes-root-entrypoint-smoke.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e-scenario/live/hermes-root-entrypoint-smoke.test.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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/e2e-vitest-scenarios.yaml:
- Around line 1274-1293: The PR added hermes-root-entrypoint-smoke-vitest to
report-to-pr.needs but the workflow-boundary validator's required set wasn't
updated; edit the validator (the required-needs array in workflow-boundary.mts)
to include "hermes-root-entrypoint-smoke-vitest" so the validator enforces that
this job remains in report-to-pr.needs; ensure the string matches exactly the
entry added in the workflow and run the validator tests to confirm.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ddc7fbf4-4d33-46f2-a013-f56c0485d43e
📒 Files selected for processing (2)
.github/workflows/e2e-vitest-scenarios.yamltest/e2e-scenario/live/hermes-root-entrypoint-smoke.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e-scenario/live/hermes-root-entrypoint-smoke.test.ts
🛑 Comments failed to post (1)
.github/workflows/e2e-vitest-scenarios.yaml (1)
1274-1293: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Keep the workflow-boundary validator in sync with this new
report-to-prdependency.Line 1284 adds
hermes-root-entrypoint-smoke-vitesttoreport-to-pr.needs, buttools/e2e-scenarios/workflow-boundary.mts:1349-1384still enforces the older required set. That leaves this new reporting dependency unprotected by the boundary check, so a later edit could drop the Hermes smoke result from PR comments without any contract failure. Please update the validator list in the same change.🤖 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/e2e-vitest-scenarios.yaml around lines 1274 - 1293, The PR added hermes-root-entrypoint-smoke-vitest to report-to-pr.needs but the workflow-boundary validator's required set wasn't updated; edit the validator (the required-needs array in workflow-boundary.mts) to include "hermes-root-entrypoint-smoke-vitest" so the validator enforces that this job remains in report-to-pr.needs; ensure the string matches exactly the entry added in the workflow and run the validator tests to confirm.
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27387740191
|
…es-root-entrypoint-smoke # Conflicts: # .github/workflows/e2e-vitest-scenarios.yaml
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27391322176
|
…es-root-entrypoint-smoke
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27394270320
|
…es-root-entrypoint-smoke
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27395432796
|
…es-root-entrypoint-smoke
…es-root-entrypoint-smoke
Vitest E2E Scenario Results — ✅ All jobs passedRun: 27401749544
|
Summary
Migrate
test-hermes-root-entrypoint-smoke.shwith the simplest equivalent Vitest coverage.Related Issues
Refs #5098
Contract mapping
/health.test/e2e-scenario/live/hermes-root-entrypoint-smoke.test.tsclean-container variant./usr/local/bin/nemoclaw-start.gatewayuser and startup logs privilege separation.assertGatewayProcesschecks real containerpsand/tmp/nemoclaw-start.log./tmp/gateway.logand/sandbox/.hermes.gosu gatewaywrite/remove probes.gateway.pidsymlink/runtime state is migrated before boot./bin/bash -lcthen root entrypoint.Simplicity check
test/e2e-scenario/fixtures/docker-probe.tscentralizes the fixture-owned Docker env minimization and redacted artifact boundary for this smoke.sandbox-images-and-e2e.yamlnow runs this smoke through Vitest against the prebuilt Hermes image; legacy shell deletion/nightly retirement deferred to Epic: Migrate legacy bash E2E into the Vitest E2E system #5098 Phase 11.Verification
git diff --checknpx prek run --all-files --stage pre-push --skip tsc-plugin --skip tsc-js --skip tsc-cli --skip version-tag-sync --skip test-cli --skip test-plugin --skip source-shape-test-budget --skip test-file-size-budget --skip test-skills-yamlnpx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts test/e2e-scenario/support-tests/docker-probe.test.ts --silent=false --reporter=defaultnpx tsc --noEmit --skipLibCheck --module NodeNext --moduleResolution NodeNext --target ES2022 --types node,vitest --allowImportingTsExtensions tools/e2e-scenarios/workflow-boundary.mts test/e2e-scenario/live/hermes-root-entrypoint-smoke.test.ts test/e2e-scenario/support-tests/e2e-scenarios-workflow.test.ts test/e2e-scenario/support-tests/docker-probe.test.ts test/e2e-scenario/fixtures/docker-probe.tsNEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/hermes-root-entrypoint-smoke.test.ts --silent=false --reporter=default(local Docker daemon unavailable, live body skipped with evidence)npx vitest run test/e2e-script-workflow.test.tsnpm run build:cli && npm run typecheck:cliSummary by CodeRabbit
Tests
Chores