chore(e2e): delete deprecated bash entrypoints and orphan helper libs#4996
Merged
Conversation
PR #4380 made the TypeScript runner (test/e2e-scenario/scenarios/run.ts) the canonical scenario executor, collapsed run-scenario.sh and run-suites.sh to fail-fast deprecation stubs, and deleted the runtime/resolver/ tree. Several bash files were left behind with no production callers. This PR removes them. Tombstone bash entrypoints (no callers; bodies were just exit-2 stubs): - test/e2e-scenario/runtime/run-scenario.sh - test/e2e-scenario/runtime/run-suites.sh Orphan helper libs (no production callers): - test/e2e-scenario/runtime/lib/artifacts.sh - test/e2e-scenario/runtime/lib/cleanup.sh - test/e2e-scenario/runtime/lib/negative.sh - test/e2e-scenario/runtime/lib/port-holder.sh Test cleanup riding along with the deletions: - Drop the 'artifact_helper_should_collect_known_logs_*' test in e2e-lib-helpers.test.ts: it was the only caller of artifacts.sh. - Drop 'gateway_helper_should_report_unhealthy_gateway_clearly' in e2e-lib-helpers.test.ts: it sourced ${RUNTIME_LIB}/gateway.sh, which does not exist on main and was passing only by accident (the missing-file stderr happens to match /gateway/i). - Drop the dead E2E_DRY_RUN: '1' env arg in 'older_base_image_should_emit_dockerfile_pointing_at_tagged_base' in e2e-lib-helpers.test.ts: PR #4380 deleted the dry-run short-circuit from older-base-image.sh, so the env var is now unread test scaffolding. Comment hygiene: - sandbox-absent.sh: drop the 'Typed replacement for the legacy run-scenario.sh inline check' note now that the legacy file is deleted. Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThis PR removes deprecated Bash E2E runtime helpers (artifact collection, cleanup, negative output validation, port management, and script runners) and adjusts corresponding test fixtures to no longer depend on those utilities. ChangesE2E Test Infrastructure Modernization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
Contributor
E2E Advisor RecommendationRequired E2E: None 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, 3 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
cv
approved these changes
Jun 9, 2026
15 tasks
This was referenced Jun 9, 2026
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.
Summary
Bash is not the direction the E2E scenario stack is heading, and the vitest-runner migration underway makes that even clearer — none of these bash files have a future role. This PR sweeps out a set of inert tombstones and orphan helpers under
test/e2e-scenario/runtime/that have no production callers, so the migration lands on a smaller surface. Net: 6 file deletions + minor test trimming. ~250 lines removed. No production behavior change.Changes
Tombstone bash entrypoints (deleted; bodies were just
exit 2stubs):test/e2e-scenario/runtime/run-scenario.shtest/e2e-scenario/runtime/run-suites.shOrphan helper libs (deleted; zero callers in production):
test/e2e-scenario/runtime/lib/artifacts.sh— only caller was a self-test ine2e-lib-helpers.test.ts.test/e2e-scenario/runtime/lib/cleanup.sh— was a thin re-export ofsandbox-teardown.sh.test/e2e-scenario/runtime/lib/negative.sh— superseded byscenarios/orchestrators/negative-matcher.ts.test/e2e-scenario/runtime/lib/port-holder.sh— superseded by typed probes.Test cleanup in
test/e2e-scenario/framework-tests/e2e-lib-helpers.test.ts(rides along):artifact_helper_should_collect_known_logs_without_failing_when_missing— it was the only caller ofartifacts.sh.gateway_helper_should_report_unhealthy_gateway_clearly— it sourced${RUNTIME_LIB}/gateway.sh, which does not exist on main. The test was passing only by accident: the missing-file stderr happens to match/gateway/i.E2E_DRY_RUN: "1"env arg inolder_base_image_should_emit_dockerfile_pointing_at_tagged_base. PR test(e2e): execute real shell assertions; delete dry-run, --validate-only, and the bash runner #4380 deleted thee2e_env_is_dry_runshort-circuit fromolder-base-image.sh, so the env var is unread test scaffolding now.Comment hygiene:
nemoclaw_scenarios/probes/sandbox-absent.sh— dropped the "Typed replacement for the legacyrun-scenario.shinline check" note now that the legacy file is deleted.Intentionally untouched:
test/e2e-scenario-advisor.test.tsstill referencestest/e2e-scenario/runtime/run-scenario.shas a string fixture. The advisor never reads the file — it filters changed-file paths by shape — so the fixture continues to validate the same plumbing. Refreshing those fixtures is out of scope.test/e2e-scenario/docs/{README,MIGRATION}.mdreference the old bash entrypoints. Doc updates are out of scope; the docs already mark these paths as deprecated.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesnpm run docsbuilds without warnings (doc changes only)Notes on verification:
npx vitest run test/e2e-scenario/framework-tests/e2e-lib-helpers.test.ts test/e2e-scenario-advisor.test.ts test/e2e-scenario/— 3 test files, 106 tests, 0 failures.npx prek run --all-filesreports the sameTest (CLI)flakes that exist on a cleanorigin/maincheckout (timing-sensitive tests innemoclaw-start.test.ts,doctor-gateway-token.test.ts,e2e-fixture-context.test.ts,runtime-hermes-secret-boundary-behavioural.test.ts). All pass on isolated re-run; none reference any of the deleted bash files. Pre-existing — not caused by this PR. Leaving the verification box unchecked because the local prek run wasn't fully clean even though the issue is unrelated.Signed-off-by: Julie Yaunches jyaunches@nvidia.com