test(e2e): typed-shell-runner cutover (parity → retirement)#5106
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Refs #5098 Empty placeholder commit so the branch has a parent commit for the draft PR. Implementation lands in subsequent commits per the phased plan in the PR body. Signed-off-by: J. Yaunches <jyaunches@nvidia.com>
jyaunches
added
the
area: e2e
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
|
Warning Review limit reached
More reviews will be available in 51 minutes and 9 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 (177)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
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
|
Contributor
PR Review AdvisorFindings: 0 needs attention, 1 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
This is an automated advisory review. A human maintainer must make the final merge decision. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Refs #5098 Empty placeholder commit so the branch has a parent commit for the draft PR. Implementation lands in subsequent commits per the phased plan in the PR body. Signed-off-by: J. Yaunches <jyaunches@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
12 tasks
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 27257661280
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
approved these changes
Jun 10, 2026
This was referenced Jun 10, 2026
This was referenced Jun 10, 2026
jyaunches
added a commit
that referenced
this pull request
Jun 10, 2026
## Summary Simplifies legacy E2E migration tracking now that #5106 retired the typed-shell/YAML scenario runner. The PR removes stale repo-local JSON migration ledgers, documents GitHub issues and PRs as the source of truth for migration state, adopts the #5156-style deterministic legacy bash workflow guard, and clarifies that NemoClaw owns one Vitest E2E system with fixtures/support code rather than a second framework. ## Related Issue Refs #5098 ## Changes - Removes `test/e2e-scenario/migration/legacy-inventory.json` so it no longer acts as a detailed script-by-script migration roadmap. - Removes the orphaned generated assertion inventory at `test/e2e/docs/parity-inventory.generated.json`; its recorded generator no longer exists, and it was already stale against migrated/deleted legacy scripts. - Adds deterministic workflow contract tests that freeze the current top-level `test/e2e/test-*.sh` legacy script set, freeze scheduled `nightly-e2e.yaml` legacy script wiring, and assert every nightly-wired legacy script still exists. - Replaces inventory validation with migration policy/source-of-truth checks that keep repo-local durable taxonomy and generated assertion ledgers out of migration docs. - Removes the PR Review Advisor PR-body deletion-evidence checker/contract; the advisor still blocks reintroducing retired migration ledgers. - Updates `MIGRATION.md`, `README.md`, `RETIREMENT.md`, and release-note wording to describe one Vitest E2E system, workflow allowlist guardrails, and fixture/support code rather than a second E2E framework or runner. - Renames the fast Vitest support project from `e2e-scenario-framework` to `e2e-vitest-support`, moves tests from `framework-tests/` to `support-tests/`, and renames the shared helper path to `test/e2e-scenario/fixtures/`. - Rewords the E2E scenario advisor prompt, summary, and sticky-comment text so it recommends Vitest-backed E2E scenario dispatches without teaching “scenario E2E” as a separate E2E class. - Fixes shared sandbox fixture helpers so `SandboxClient.exec()` uses `openshell sandbox exec -n <name>` and exposes `execShell()` / `upload()` for follow-on migrations. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - `npx vitest run --project e2e-vitest-support --silent=false --reporter=default` - `npx vitest run --project cli test/pr-workflow-contract.test.ts test/e2e-scenario-advisor.test.ts --silent=false --reporter=default` - `npx vitest run --project cli test/e2e-script-workflow.test.ts test/pr-review-advisor.test.ts --reporter=default` - `npx vitest run --project e2e-vitest-support test/e2e-scenario/support-tests/e2e-migration-policy.test.ts --reporter=default` - `npm run test-size:check -- test/e2e-script-workflow.test.ts` - `npx markdownlint-cli2 docs/about/release-notes.mdx test/e2e-scenario/docs/MIGRATION.md test/e2e-scenario/docs/README.md test/e2e-scenario/docs/RETIREMENT.md "!node_modules/**" "!skills/**"` - `node -e "JSON.parse(require("fs").readFileSync("tools/e2e-advisor/scenarios-schema.json","utf8"))"` - `npm run typecheck:cli` - `git diff --check` - Commit and push hooks passed with `SSH_AUTH_SOCK=/tmp/ssh-Nu4xGJZo0F/agent.712827`. - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Consolidated E2E test infra under a Vitest-driven fixtures/support layer and renamed the E2E support project. * **Documentation** * Updated migration guidance to use GitHub issues/PRs as the source of truth and clarified fixture-owned responsibilities and migration checklist. * **Tests** * Added migration-policy/source-of-truth tests; removed legacy repo-local migration ledger; updated scenario and PR-review advisor outputs to target Vitest E2E and to block reintroduced ledgers. * **New Features** * Fixture sandbox helpers added (shell-run/upload) and command usage normalized for named sandboxes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements Phase 0 PR-1 of #5098: cut over from the typed-shell scenario runner to the Vitest scenario path in one coordinated PR.
What changed
run-plan.json, per-phase result artifacts, explicit artifact allowlist, and 14-day retention.environment.result.json,onboarding.result.json, andstate-validation.result.jsonfrom the phase fixtures.test/e2e-scenario/scenarios/run.tsto the surviving--emit-live-matrixentry point.test/e2e-scenario/docs/RETIREMENT.mdand refreshed the scenario docs/inventory tests to describe the Vitest-only scenario runner state.Verification
npx vitest run --project e2e-scenario-framework --silent=false --reporter=defaultnpm run typecheck:clinpx tsx test/e2e-scenario/scenarios/run.ts --emit-live-matrixgit diff --checkRefs #5098, #4990, #4941