test(e2e): add Vitest docs validation coverage#5185
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR migrates E2E docs validation from a legacy bash script to a Vitest-based scenario. The new Vitest test builds the CLI, validates no stale outputs, creates an isolated environment with a nemoclaw shim, and runs docs checks in two phases. The nightly CI workflow is refactored to execute the Vitest test and upload artifacts. The test expectations are updated to remove the legacy script from the allowlist and verify the new Vitest-driven flow. ChangesDocs E2E Vitest Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: 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
|
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. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/e2e-script-workflow.test.ts (1)
308-340: ⚡ Quick winAssert the remaining docs-validation workflow contract fields.
This test should also pin
job.permissions.contentsand upload artifactwith.nameto prevent silent nightly contract drift.Suggested patch
it("runs docs validation directly through Vitest artifacts", () => { const job = nightlyWorkflow.jobs["docs-validation-e2e"]; @@ const uploadStep = job.steps?.find((step) => step.name === "Upload docs validation artifacts"); + expect(job.permissions?.contents).toBe("read"); @@ expect(uploadStep?.if).toBe("always()"); + expect(uploadStep?.with?.name).toBe("docs-validation-artifacts"); expect(uploadStep?.with?.path).toBe("e2e-artifacts/vitest/docs-validation/");As per coding guidelines,
docs-validation-e2emust keeppermissions: contents: readand artifactname: docs-validation-artifactsin the nightly contract.🤖 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-script-workflow.test.ts` around lines 308 - 340, The test is missing assertions that pin the docs-validation-e2e job permissions and artifact name; update the "runs docs validation directly through Vitest artifacts" case to assert that nightlyWorkflow.jobs["docs-validation-e2e"].permissions?.contents === "read" and that the upload step (uploadStep?.with?.name) === "docs-validation-artifacts" so the contract enforces permissions.contents: read and the artifact name.Source: Coding guidelines
🤖 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-script-workflow.test.ts`:
- Around line 308-340: The test is missing assertions that pin the
docs-validation-e2e job permissions and artifact name; update the "runs docs
validation directly through Vitest artifacts" case to assert that
nightlyWorkflow.jobs["docs-validation-e2e"].permissions?.contents === "read" and
that the upload step (uploadStep?.with?.name) === "docs-validation-artifacts" so
the contract enforces permissions.contents: read and the artifact name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 76b55b65-cd0c-4ef4-8807-a2dc93409608
📒 Files selected for processing (4)
.github/workflows/nightly-e2e.yamltest/e2e-scenario/live/docs-validation.test.tstest/e2e-script-workflow.test.tstest/e2e/test-docs-validation.sh
💤 Files with no reviewable changes (1)
- test/e2e/test-docs-validation.sh
Selective E2E Results — ❌ Some jobs failedRun: 27315511808
|
…-validation-simple
Selective E2E Results — ✅ All requested jobs passedRun: 27316666885
|
Selective E2E Results — ✅ All requested jobs passedRun: 27316804504
|
Summary
Add the simplest equivalent Vitest coverage for
test/e2e/test-docs-validation.shwhile keeping the legacy shell script intest/e2efor now.Related Issues
Refs #5098
Refs #5138
Contract mapping
check-docs.sh --only-clitest/e2e-scenario/live/docs-validation.test.tsnpm run build:cli, realbin/nemoclaw.js, realcheck-docs.shcheck-docs.sh --only-links --local-onlyfrom the Vitest testdocs-validation-e2einnightly-e2e.yamle2e-scenarios-liveVitest invocationtest/e2e/; deletion is deferred.Simplicity check
Verification
npm run build:cliNEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/docs-validation.test.ts --silent=false --reporter=defaultnpx vitest run --project cli test/e2e-script-workflow.test.ts --silent=false --reporter=defaultnpm run typecheck:clinpm run test-size:check -- test/e2e-scenario/live/docs-validation.test.ts test/e2e-script-workflow.test.tsgit diff --checkSummary by CodeRabbit
Tests
Chores