fix(ci): select migrated E2E parity entrypoint - #7025
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
📝 WalkthroughWalkthroughThe CI parity validation step prefers ChangesE2E parity entrypoint selection
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI parity step
participant Filesystem
participant npx tsx
CI parity step->>Filesystem: Check .mts entrypoint
Filesystem-->>CI parity step: Return available path or continue
CI parity step->>Filesystem: Check .ts fallback
Filesystem-->>CI parity step: Return available path or missing
CI parity step->>npx tsx: Run selected parity script with --base and --head
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
![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/pr-workflow-contract.test.ts (1)
978-980: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the implementation-coupled shell-text assertions.
These checks can pass even if fallback ordering or missing-entrypoint rejection is broken. The process-level suite already verifies those observable outcomes, so rely on it instead.
As per path instructions, “Prefer observable outcomes through the public boundary over source-text assertions.”
Proposed cleanup
- expect(parityStep.run).toContain("parity_check=scripts/checks/e2e-mock-parity.mts"); - expect(parityStep.run).toContain("parity_check=scripts/checks/e2e-mock-parity.ts"); - expect(parityStep.run).toContain('npx tsx "$parity_check" --base "$base" --head "$head"');🤖 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/pr-workflow-contract.test.ts` around lines 978 - 980, Remove the three shell-text assertions from the parity workflow contract test around parityStep.run, and rely on the existing process-level tests that verify fallback ordering and missing-entrypoint rejection through observable behavior.Source: Path instructions
🤖 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/pr-workflow-contract.test.ts`:
- Around line 978-980: Remove the three shell-text assertions from the parity
workflow contract test around parityStep.run, and rely on the existing
process-level tests that verify fallback ordering and missing-entrypoint
rejection through observable behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e719343f-e155-4d55-99a8-4e129a635feb
📒 Files selected for processing (3)
.github/actions/ci-cli-coverage-shard/action.yamltest/e2e-mock-parity.test.tstest/pr-workflow-contract.test.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
apurvvkumaria
left a comment
apurvvkumaria
left a comment
There was a problem hiding this comment.
Security review clean on exact head fecc4dd. The trusted action selects only fixed, quoted repository paths, prefers the migrated .mts entrypoint, retains the bounded .ts fallback, and fails closed when neither exists. Observable tests cover preference, fallback, and rejection. No secrets, permission expansion, dependency change, unsafe interpolation, or error-handling regression found; exact-head CI, CodeQL, E2E, and both automated advisors are green.
wscurran
added
area: ci
4 participants
Summary
The base-trusted CLI coverage action currently hardcodes the TypeScript E2E parity entrypoint. Select the migrated
.mtsentrypoint when present, retain the.tspath for merge commits on the earlier layout, and fail clearly when neither trusted path exists.Related Issue
Unblocks #6940.
Changes
scripts/checks/e2e-mock-parity.mtsand fall back toscripts/checks/e2e-mock-parity.tsin the trusted CLI coverage shard..mtspreference,.tscompatibility, and missing-entrypoint rejection at the composite-action boundary, replacing the prior hardcoded command-text assertion.The compatibility fallback is required because the base-trusted action validates merge commits on both sides of the
.tsto.mtsmigration.test/e2e-mock-parity.test.tsprotects that transition contract.Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run test/e2e-mock-parity.test.ts test/pr-workflow-contract.test.ts(30 passed);npm run test-conditionals:scan -- --top 25reports noifstatements in the changed test;npm run source-shape:checkpassednpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable; the focused CI contract is covered by targeted tests andnpm run check:diff.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
.mtsentrypoint and falling back to.ts.base/headvalues rather than a single hard-coded command.