test(cli): reduce slow test hotspots - #8715
Merged
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThe PR adds focused Ollama, rebuild, Shields, MCP policy, BuildKit, and OpenClaw runtime test coverage. It replaces several end-to-end fixtures with shared harnesses and removes one deleted test’s timing entry. ChangesSandbox and runtime test coverage
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 5d417b3 in the TypeScript / code-coverage/cliThe overall coverage in commit 5d417b3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
test/openclaw-gemini-inference-compat-runtime.test.ts (1)
28-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReplace compiled-bundle discovery with a versioned runtime adapter.
ROUTING_PROBEscansdistfor hashed chunks, reads source text, and finds exports by function name. It also imports private loader, resolver, and transport paths. Packaging changes can break the test without changing behavior. Use a supported OpenClaw entrypoint for observable assertions. If none exists, isolate the pinned-version imports behind one compatibility adapter.🤖 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/openclaw-gemini-inference-compat-runtime.test.ts` around lines 28 - 46, Replace the dist-directory scan and function-name lookup around refreshPluginRegistry with a versioned runtime adapter that uses OpenClaw’s supported entrypoint for the observable routing assertions. Keep all pinned-version loader, resolver, and transport imports inside that single adapter, and update ROUTING_PROBE to call the adapter without depending on hashed chunks or private paths.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.
Inline comments:
In `@src/lib/actions/sandbox/connect-route-lifecycle.test.ts`:
- Around line 101-104: Update the test using createConnectHarness so the
local-provider health path executes the real probe implementation instead of the
mock’s unconditional { ok: true } result. Configure the harness or add a focused
case that exercises skipOllamaAuthProxySubprobe: true, while preserving
assertions that the local probe receives this option and
probeOllamaAuthProxyHealth is not called.
In `@src/lib/actions/sandbox/rebuild-flow-credential-preflight.test.ts`:
- Around line 570-584: The test should also verify registration order: compare
registerHermesInferenceProviderSpy.mock.invocationCallOrder with
backupSandboxStateSpy.mock.invocationCallOrder and assert the Hermes
registration call occurs first, while preserving the existing call and logging
assertions.
In `@src/lib/inference/local.test.ts`:
- Around line 131-141: Update the findReachableOllamaHost test to capture each
probe command and assert that every command includes --connect-timeout 3 and
--max-time 5, while preserving the existing endpoint order assertions.
In `@test/rebuild-credential-preflight.test.ts`:
- Around line 10-18: Restore a process-boundary test in the credential-preflight
suite that invokes bin/nemoclaw.js through the existing process-execution setup
and verifies a nonzero exit status when preflight fails. Keep the focused
rebuildSandbox test, but add back separate CLI exit coverage for this failure
path using the existing timeout and filesystem helpers.
In `@test/rebuild-shields-window.test.ts`:
- Around line 23-29: Add an afterEach hook in the “rebuild Shields window” test
suite that calls vi.restoreAllMocks(), ensuring the console spies created in
beforeEach are restored after every test while preserving the existing mock
reset behavior.
---
Nitpick comments:
In `@test/openclaw-gemini-inference-compat-runtime.test.ts`:
- Around line 28-46: Replace the dist-directory scan and function-name lookup
around refreshPluginRegistry with a versioned runtime adapter that uses
OpenClaw’s supported entrypoint for the observable routing assertions. Keep all
pinned-version loader, resolver, and transport imports inside that single
adapter, and update ROUTING_PROBE to call the adapter without depending on
hashed chunks or private paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0194361c-6e35-4e0c-a77c-408a80904112
📒 Files selected for processing (19)
ci/cli-test-timing-hints.jsonsrc/lib/actions/sandbox/connect-route-lifecycle.test.tssrc/lib/actions/sandbox/rebuild-flow-credential-preflight.test.tssrc/lib/actions/sandbox/rebuild-flow-shields-window.test.tssrc/lib/actions/sandbox/rebuild-shields-phase.test.tssrc/lib/inference/local.test.tssrc/lib/shields/flow.test.tssrc/lib/shields/mcp-policy-transition.test.tstest/helpers/rebuild-flow-test-harness.tstest/helpers/rebuild-flow-test-support.tstest/openclaw-gemini-inference-compat-runtime.test.tstest/rebuild-credential-preflight.test.tstest/rebuild-shields-auto-unlock.test.tstest/rebuild-shields-window.test.tstest/rebuild-stale-recovery.test.tstest/repro-2201.test.tstest/sandbox-build-context.test.tstest/sandbox-connect-inference/route-swap-repair.test.tstest/support/connect-flow-test-harness.ts
💤 Files with no reviewable changes (2)
- test/rebuild-shields-auto-unlock.test.ts
- ci/cli-test-timing-hints.json
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: None 4 optional E2E recommendations
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>
23 tasks
This was referenced Aug 10, 2026
wscurran
added
chore
This was referenced Aug 11, 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
2 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
Replace redundant full CLI fixtures in the slowest test cases with focused source tests. Retain spawned-CLI checks for interactive cancellation and the LangChain Deep Agents Code HTTP 401 failure path, plus real boundaries for local Ollama sandbox connection, public OpenClaw runtime inspection, and aggregate BuildKit artifact import. This reduces runtime without changing supported behavior or lowering coverage for the affected source modules.
Related Issue
Fixes #8669.
Changes
Performance Evidence
The before values come from the issue's 8-shard baseline run 31348939543. Run 31375057176 recorded the after values in the
cli-vitest-resultsartifact for the commit under review,5d417b39c. Split rows sum the named replacement assertions.openclaw plugins inspect --runtime --json. The runtime check covers the executable, generated configuration, and plugin loader. Keep the direct managed Google Gemini routing and tool-round assertion separate.The issue total fell from 187.924 seconds to 75.484 seconds: 112.440 seconds, or 59.8%, lower. The same 12-shard method measured 175.351 seconds in pre-change run 31367710334. Its
cli-vitest-resultsartifact records the comparison data. The selected tests on the commit under review took 99.867 seconds less, a reduction of 57.0%. Each merged 12-shard artifact records every test duration, so the ranking can be refreshed without reading job logs.Type of Change
Quality Gates
Documentation Writer Review
no-docs-needed5d417b39cand the updated PR body. The commits change tests, test harnesses, and one CI timing hint. They do not change user-facing behavior or documentation source. No blocking wording findings remain.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable8669passed the same set; leak diagnostics passed all assertions. The finalnpx vitest run --project integration test/sandbox-build-context.test.ts --maxWorkers=1run passed 15 tests with one environment-gated skip. The review repairs initially passed 104 focused source tests and 9 integration tests. Aftermainadvanced, the 119 affected focused source tests and all 8 Shields-window integration cases passed again. Biome and the 2,268-file test-conditional scan passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: PR CI run 31375057176 passed all 12 CLI shards and the aggregatecli-testsgate on commit5d417b39c.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com