Skip to content

perf(ci): remove repeated test setup - #11337

Merged
cv merged 4 commits into
mainfrom
codex/6237-test-hidden-setup
Sep 9, 2026
Merged

perf(ci): remove repeated test setup#11337
cv merged 4 commits into
mainfrom
codex/6237-test-hidden-setup

Conversation

@cjagwani

@cjagwani cjagwani commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Outcome

Three CI-only paths stop repeating work that another layer already guarantees. Local three-run medians improve as follows:

Path Before After Saving
ARM64 GPU prover test file 6.49s 5.01s 23%
Credential rotation test file 4.20s 1.87s 55%
Plugin typecheck step 2.28s 1.67s 27%

Production behavior is unchanged.

Reason

  • Five ARM64 prover cases already inject the proof result but still invoked the default runtime-provider loader through the source-require bridge.
  • The credential rotation suite reloaded the full onboard module before every test even though it exercises three focused source modules.
  • compile-artifacts already runs the plugin production compiler and build-typecheck depends on that verified output, but the latter compiled production sources again before checking tests.

Related issues

Part of #6237.

Changes

  • Inject the existing Docker provider fixture into the five isolated ARM64 prover cases.
  • Import credential rotation, hashing, and registry modules directly instead of dynamically loading the full onboard entry point before every test.
  • Limit the dependent build/typecheck job to nemoclaw/tsconfig.test.json; preserve plugin-local compiler ownership and add a workflow contract proving production compilation remains an upstream prerequisite.

This is a small recovery of the remaining hidden-setup ideas from closed PR #11098 without its shard-remapping or large test-splitting changes.

Verification

  • npx vitest run --project cli --project integration src/lib/onboard/wsl-docker-desktop-gpu.test.ts test/credentials/credential-rotation.test.ts test/automation/pull-requests/pr-workflow-contract.test.ts --coverage=false — 3 files and 92 tests passed.
  • npm --prefix nemoclaw exec -- tsc --noEmit -p nemoclaw/tsconfig.test.json — passed.
  • Three before/after wall-time runs per path produced the medians in the table above.
  • Normal pre-commit hooks — formatting, lint, repository checks, env-var documentation, gitleaks, source-shape budget, growth guardrails, and commitlint passed.
  • Normal pre-push hooks — CLI TypeScript check passed.
  • GitHub commit verification — commits b0361285adb24bff9c847e59595ad3d500abaa50, 12b357faabc0ffb313a9ddf1b755ece1b5a7b74f, and 08888562873b4a823c80a22ee593e4c7aaafe28f are Verified.
  • Merged current main without rewriting history, including test(ci): refresh current main contracts #11332's refreshed assertions and fix(images): refresh Hermes probe digest #11338's Hermes probe digest repair.
  • The repaired Hermes image-probe contract passes locally: 1 file and 57 tests.
  • No docs change is needed because only test fixtures and internal CI execution change.

Review notes

The timing measurements are local macOS evidence; the fresh hosted CI run validates behavior on the current merge commit. The production plugin project remains typechecked by the exact compiler invocation that creates the verified artifact consumed by this job.


Signed-off-by: Charan Jagwani cjagwani@nvidia.com

Summary by CodeRabbit

  • Tests
    • Improved validation of pull request and main workflow build checks.
    • Added coverage for plugin test typechecking and confirmed outdated package-level checks are not used.
    • Expanded GPU provisioning test scenarios, including ARM64, terminal escaping, failed proofs, and execution-format errors.
    • Simplified credential rotation test setup using direct module imports.
  • Chores
    • Updated continuous integration checks to provide more reliable TypeScript validation.

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@cjagwani cjagwani self-assigned this Sep 9, 2026
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7efd0892-e1f7-4906-81be-95e5819597fc

📥 Commits

Reviewing files that changed from the base of the PR and between 0888856 and 4a72616.

📒 Files selected for processing (1)
  • test/automation/pull-requests/pr-workflow-contract.test.ts

Included review availability: Your plan provides up to 12 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The CI action now typechecks plugin tests with a dedicated TypeScript configuration. Workflow contracts verify build ordering and command usage. GPU tests resolve Docker providers. Credential rotation tests use direct ESM imports.

Changes

CI typecheck contracts

Layer / File(s) Summary
Plugin test typecheck contract
.github/actions/ci-build-typecheck/action.yaml, test/automation/pull-requests/pr-workflow-contract.test.ts, ci/source-shape-test-budget.json
The CI action runs tsc --noEmit with nemoclaw/tsconfig.test.json. Contract tests verify trusted compilation precedes typechecking and reject the package-level typecheck command.

GPU provider test setup

Layer / File(s) Summary
Docker provider resolution in GPU tests
src/lib/onboard/wsl-docker-desktop-gpu.test.ts
GPU prover scenarios resolve a Docker runtime provider for ARM64, terminal escaping, failed-proof, Docker Desktop WSL, and exec-format-error cases.

Credential rotation test imports

Layer / File(s) Summary
Direct credential helper imports
test/credentials/credential-rotation.test.ts
The test replaces dynamic CommonJS loading and per-test module initialization with direct ESM imports.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 4a726

This updates CI typechecking and test setup without changing production behavior. Current evidence shows no remaining merge-blocking risk.

Suggested reviewers: cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: removing repeated setup in CI and tests to improve performance.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/6237-test-hidden-setup

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit 4a72616 in the codex/6237-test-hidd... branch remains at 96%, unchanged from commit 801fb0c in the main branch.

TypeScript / code-coverage/cli

The overall line coverage in commit 4a72616 in the codex/6237-test-hidd... branch remains at 83%, unchanged from commit 801fb0c in the main branch.

Show a line coverage summary of the most impacted files.
File main 801fb0c codex/6237-test-hidd... 4a72616 +/-
src/lib/actions...-diagnostics.ts 81% 67% -14%
src/lib/state/o...config-merge.ts 92% 85% -7%
src/lib/inferen...anaged-state.ts 85% 82% -3%
src/lib/actions...ess-recovery.ts 84% 82% -2%
src/lib/actions...eway-restart.ts 94% 92% -2%
src/lib/actions...ard-recovery.ts 84% 85% +1%
src/lib/actions...ard-recovery.ts 95% 97% +2%
src/lib/actions...e-validation.ts 84% 88% +4%
src/lib/actions...ridge-output.ts 89% 95% +6%
src/lib/onboard...-credentials.ts 50% 63% +13%

Updated September 09, 2026 22:27 UTC

Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions chore Build, CI, dependency, or tooling maintenance labels Sep 9, 2026
@cjagwani
cjagwani requested review from cv and prekshivyas September 9, 2026 22:05
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor finished for commit 4a72616. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

All previous runs

@cv
cv merged commit 08c2011 into main Sep 9, 2026
80 checks passed
@cv
cv deleted the codex/6237-test-hidden-setup branch September 9, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants