Skip to content

test(ci): bound slow shard integration tests - #7759

Merged
jyaunches merged 8 commits into
mainfrom
codex/ci-workflow-boundary-timeout
Jul 29, 2026
Merged

test(ci): bound slow shard integration tests#7759
jyaunches merged 8 commits into
mainfrom
codex/ci-workflow-boundary-timeout

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Three integration-heavy tests exceeded their existing ceilings only under hosted-runner shard coverage load, failing unrelated pull requests. The planner workflow test launches a subprocess with a deliberate 30-second deadline, the base-image test cold-loads the source-backed Docker harness, and the deterministic Advisor rendering test already consumes several seconds without coverage. This gives only those tests bounded headroom while retaining the subprocess's fail-closed 30-second ceiling and every existing behavior assertion. After synchronizing with main, CI also exposed that the reviewed source-architecture fan-in baseline had improved without ratcheting its budget; this change records the lower observed value.

Changes

  • Give the real planner-matrix integration case a 40-second outer budget, leaving its 30-second subprocess timeout unchanged.
  • Give the cold-load base-image integration case a 15-second outer budget.
  • Give the deterministic Advisor recommendation-rendering case a 30-second budget.
  • Preserve all workflow-output parsing, base-image behavior, Advisor normalization, and PR-report assertions.
  • Ratchet src/lib/core/shell-quote.ts's reviewed fan-in budget from 27 to the current observed value of 26.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: Test-harness timing only; no user-facing command, configuration, API, workflow behavior, or documentation contract changes.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: The diff changes three tests' outer timeout budgets and ratchets one internal architecture budget from 27 to 26; assertions, product code, documentation, and user-facing behavior are unchanged.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — The planner timeout reproduced in hosted CI and pre-fix 16-worker stress; its exact test passed 5/5 sequentially and under the same stress, and the full file passed 33/33. The base-image file passed 24/24 after its scoped fix. The Advisor rendering test passed with its scoped budget, and the full Advisor file passed 44/44. The exact source-architecture baseline test and production architecture checker passed with the 26 fan-in ratchet. Repository checks, CLI typecheck, and normal changed-file hooks passed.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — Not applicable to these scoped test-harness ceiling corrections; the affected files, shard-equivalent cases, builds, typecheck, repository checks, and changed-file hooks passed.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

  • Tests
    • Increased timeouts for end-to-end/integration tests to improve reliability during longer-running executions.
    • Kept existing assertions intact, including validation of generated test matrices, workflow outputs, and pull request report content.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria apurvvkumaria added chore Build, CI, dependency, or tooling maintenance v0.0.98 labels Jul 28, 2026
@apurvvkumaria apurvvkumaria self-assigned this Jul 28, 2026
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Three tests now use explicit timeouts: 40 seconds for the planner-matrix integration test, 15 seconds for the agent base-image test, and 30 seconds for the E2E recommendation test. Existing test behavior and assertions remain unchanged.

Changes

Test timeout configuration

Layer / File(s) Summary
Configure planner-matrix test timeout
test/e2e/support/e2e-report-to-pr-workflow-boundary.test.ts
The integration test imports testTimeout and applies testTimeout(40_000) while preserving matrix generation, report execution, and assertions.
Configure base-image test timeout
src/lib/agent/base-image.test.ts
The base-image test imports testTimeout and applies testTimeout(15_000) while preserving its mocks and expectations.
Configure recommendation test timeout
test/pr-review-advisor.test.ts
The E2E recommendation test imports testTimeoutOptions and applies a 30-second timeout.

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

Possibly related PRs

  • NVIDIA/NemoClaw#7678: Adjusts timeout handling for the same planner/workflow boundary integration test.

Suggested labels: area: ci, area: e2e

Suggested reviewers: cv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: adding bounded timeouts to slow integration tests in CI.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/ci-workflow-boundary-timeout

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

@github-code-quality

github-code-quality Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 5403bec in the codex/ci-workflow-bo... branch remains at 96%, unchanged from commit 3aea38c in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 5403bec in the codex/ci-workflow-bo... branch remains at 81%, unchanged from commit ae71359 in the main branch.

Show a code coverage summary of the most impacted files.
File main ae71359 codex/ci-workflow-bo... 5403bec +/-
src/lib/onboard/docker-cdi.ts 80% 70% -10%
src/lib/onboard...host-anchors.ts 94% 90% -4%
src/lib/actions...eway-restart.ts 95% 94% -1%
src/lib/onboard/preflight.ts 80% 80% 0%
src/lib/actions...light-guards.ts 86% 90% +4%
src/lib/onboard...box-gpu-mode.ts 92% 97% +5%
src/lib/domain/.../connect-env.ts 89% 97% +8%
src/lib/actions...confirmation.ts 69% 79% +10%
src/lib/actions...ocker-health.ts 65% 82% +17%
src/lib/onboard...box-prebuild.ts 74% 92% +18%

Updated July 29, 2026 16:37 UTC

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections match; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-inference, cloud-onboard, security-posture

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria apurvvkumaria changed the title test(e2e): bound planner matrix workflow test test(ci): bound slow shard integration tests Jul 28, 2026
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@test/pr-review-advisor.test.ts`:
- Line 362: Replace the literal 30_000 timeout in the test case with the shared
timeout helper from test/helpers/timeouts.ts, ensuring NEMOCLAW_TEST_TIMEOUT can
control the test budget while preserving the existing timeout behavior.
🪄 Autofix (Beta)

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: 95b3a07a-e68f-4ff9-bbb0-75da40c32139

📥 Commits

Reviewing files that changed from the base of the PR and between 26afe98 and d7b49af.

📒 Files selected for processing (1)
  • test/pr-review-advisor.test.ts

Comment thread test/pr-review-advisor.test.ts Outdated
apurvvkumaria and others added 2 commits July 28, 2026 13:55
@copy-pr-bot

copy-pr-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@jyaunches

Copy link
Copy Markdown
Contributor

/ok to test 4ae480f

@jyaunches
jyaunches merged commit 6f3afab into main Jul 29, 2026
70 of 72 checks passed
@jyaunches
jyaunches deleted the codex/ci-workflow-boundary-timeout branch July 29, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants