Skip to content

Improve Bun test timing and runner diagnostics (Fixes #2904) - #3110

Merged
acoliver merged 3 commits into
mainfrom
issue2904
Aug 7, 2026
Merged

Improve Bun test timing and runner diagnostics (Fixes #2904)#3110
acoliver merged 3 commits into
mainfrom
issue2904

Conversation

@acoliver

@acoliver acoliver commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

TLDR

Makes the Bun-migrated auth and core tests prove their stated behavior instead of relying on disabled timing, broad timer drains, or undiscovered spec files. It also makes the auth test runner report child signal termination accurately in JUnit and console diagnostics.

Dive Deeper

  • Converts the touched suites to bun:test and renames the two directly affected .spec.ts files so the existing package runners discover them.
  • Replaces zero-delay OAuth retry tests with deterministic fake-timer evidence for exponential backoff, maximum-delay capping, explicit retry-after, jitter bounds, and retry exhaustion.
  • Replaces broad folder-trust timer draining with exact 29,999 ms and 30,000 ms boundary assertions, including abort-state evidence.
  • Directly awaits provider settings getters, narrows the MCP partial-mock import type, removes redundant assertion scopes, and consolidates the repeated ripgrep-unavailable mock.
  • Captures child exit signals in the auth runner and shares one tested failure-reason formatter between JUnit and console reporting. A real child process self-terminating with SIGTERM proves end-to-end propagation on POSIX platforms.
  • Records accepted, rejected, deferred, and out-of-scope issue findings in the issue plan. No production OAuth retry behavior, dependencies, workflows, or broad runner discovery changed.

Local review completed with DeepThinker and Open Code Review. All valid in-scope findings were remediated. OCR's exact-timeout assertion suggestion was rejected because the configured 60-second boundary is intentional behavioral evidence; the OCR partial result was caused by one unchanged test file exhausting its review tool-round budget, not a code diagnostic.

Reviewer Test Plan

  1. From packages/auth, run bun run-bun-tests.ts and confirm all 35 discovered files pass, including OAuth timing and real SIGTERM propagation coverage.

  2. From packages/core, run:

    bun test src/config/config.folderTrustMcpWiring.test.ts src/config/config.d.test.ts src/integration-tests/provider-settings-integration.test.ts src/recording/sessionManagement.test.ts test/utils/ripgrepPathResolver.test.ts
    

    Confirm all 110 tests pass.

  3. Run npm run typecheck, npm run build, npm run format, and the canonical lint command. The complete root test chain was also attempted locally, but the external command runner terminated it by signal 15 before completion; package-level affected suites passed and CI is the definitive aggregate test gate.

  4. Run the smoke check:

    bun scripts/start.ts --profile-load stepfun-37 "write me a haiku and nothing else"
    

Testing Matrix

🍏 🪟 🐧
npm run
npx
Docker
Podman - -
Seatbelt - -

Linked issues / bugs

Fixes #2904

Summary by CodeRabbit

  • Improvements

    • Improved test failure reporting with clearer details for timeouts, terminated processes, and exit codes.
    • Enhanced generated test reports with more meaningful failure reasons.
    • Improved OAuth retry handling and verification for delays, caps, jitter, and retry exhaustion.
    • Strengthened timeout and cancellation behavior across configuration and integration workflows.
  • Testing

    • Expanded coverage for process termination, failure reporting, retries, timeout handling, and fallback scenarios.
    • Standardized the test suite on Bun’s testing framework for more consistent execution.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review 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: Pro Plus

Run ID: 3a4aac28-c5bc-45b8-90a7-4060aaeaab54

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff146 and 5c1b438.

⛔ Files ignored due to path filters (1)
  • project-plans/issue2904/plan.md is excluded by !project-plans/**
📒 Files selected for processing (2)
  • packages/auth/src/__tests__/run-bun-tests.behavior.test.ts
  • packages/auth/test-fixtures/self-sigterm.fixture.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/auth/src/tests/run-bun-tests.behavior.test.ts

📝 Walkthrough

Walkthrough

The auth test runner records signal termination and formats timeout, signal, and exit failures for CLI and JUnit output. OAuth retry tests cover timing behavior. Core tests migrate utilities to Bun and refine timer, mocking, and assertion patterns.

Changes

Auth test runner and validation

Layer / File(s) Summary
Auth runner failure reporting
packages/auth/run-bun-tests.ts
Exports test-runner APIs, records terminating signals, and formats failure reasons for CLI and JUnit output.
Auth runner validation
packages/auth/src/__tests__/run-bun-tests.behavior.test.ts, packages/auth/test-fixtures/self-sigterm.fixture.ts
Tests failure formatting, JUnit output, signal propagation, and SIGTERM handling.
OAuth retry timing coverage
packages/auth/src/__tests__/oauth-errors.test.ts
Migrates tests to Bun and verifies exponential delays, caps, explicit delays, jitter bounds, and retry exhaustion.

Core test updates

Layer / File(s) Summary
Core Bun test updates
packages/core/src/config/*.test.ts, packages/core/src/integration-tests/*, packages/core/src/recording/sessionManagement.test.ts, packages/core/test/utils/ripgrepPathResolver.test.ts
Migrates test utilities to Bun, improves timeout assertions, strengthens mock typing, simplifies assertions, and centralizes unavailable-ripgrep mocking.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check ❓ Inconclusive The summary confirms most #2904 findings, but it does not verify consistent numRuns settings or all OAuth maintainability findings. Provide diff evidence or update the affected tests to confirm that all remaining #2904 findings are addressed.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the two main changes: improved Bun test timing and improved runner diagnostics.
Description check ✅ Passed The description includes all template sections, explains the changes, provides reviewer steps, reports testing status, and links issue #2904.
Out of Scope Changes check ✅ Passed All reported changes support #2904 and the stated Bun test and runner-diagnostics objectives; no unrelated changes are reported.
📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #2904

✨ 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 issue2904

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

@github-actions github-actions Bot added the maintainer:e2e:ok Trusted contributor; maintainer-approved E2E run label Aug 6, 2026
Comment thread packages/auth/src/__tests__/run-bun-tests.behavior.test.ts Outdated
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

OpenCodeReview — PR #3110

  • Reviewed head SHA: 5c1b4387075c1543fff751511eaf5210ea482497
  • Merge base: 05d50c1e89562a2a041e91f3916910f10ed0e3b3
  • Range: incremental from 48ff146d295ab739c9efac1cabd066f9ca01fb84
  • Range fallback: none
  • Scope: selected 3 file(s), +23/-20; cumulative 10 file(s), +536/-210
  • Tokens: 41178 total (35723 input, 5455 output, 21504 cache)
  • OCR version: open-code-review v1.8.4 (e78474478) linux/amd64 built at: 2026-08-01T03:27:37Z https://github.com/alibaba/open-code-review
  • Phase: review
  • Exit code: 0
  • Run: https://github.com/vybestack/llxprt-code/actions/runs/31130382537
  • No findings.
  • Artifacts: ocr-review-output contains raw JSON, stdout, stderr, preview, phase, and exit-code diagnostics.
  • WARNING: Changed-file coverage 0/2 preview files covered is below the 90% threshold.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Before this change, the auth workspace’s Bun test execution path lacked a dedicated runner that could safely bound per-file execution time, capture child-process outcomes consistently, and emit structured failure output for CI. A hung or slow test file could stall the whole suite with little diagnostic context, and failures were reported only through raw exit codes or console noise. After this PR, the workspace uses a dedicated Bun test runner that discovers test files under src, runs them in isolated child processes with bounded parallelism, enforces a per-file timeout by terminating runaway tests, and writes a JUnit report plus standardized console failure messages. The migration also moves related vitest suites to bun:test, adds behavioral coverage for OAuth retry timing and runner diagnostics, and introduces a self-SIGTERM fixture so signal-propagation behavior is explicitly validated.

Release Notes

New Features

  • Adds a dedicated Bun test runner for the auth workspace that discovers test files under src and executes them in isolated child processes with bounded parallelism and a per-file timeout.
  • Introduces JUnit report generation from runner results, improving CI failure reporting and downstream test-result consumption.
  • Adds a self-SIGTERM test fixture to validate process-signal propagation behavior in Bun test scenarios.

Bug Fixes

  • Improves Bun test runner diagnostics by capturing child-process exit outcomes and reporting standardized timeout versus nonzero-exit failures instead of raw, inconsistent console output.

Tests

  • Migrates vitest-based suites to bun:test in the auth workspace.
  • Adds behavioral and integration coverage for OAuth retry timing and runner diagnostics.
  • Adds runner-level behavioral coverage validating timeout handling, failure messaging, and signal-related behavior.

Documentation

  • Documents the issue implementation plan with behavioral requirements, TDD sequence, and scope boundaries for Bun test timing and runner diagnostics.

Changes

Layer File(s) Summary
core packages/auth/run-bun-tests.ts Improves Bun test runner diagnostics by capturing process signals, adding standardized failure messaging, and exporting JUnit generation for better failure reporting.
tests packages/auth/src/tests/oauth-errors.spec.ts, packages/auth/src/tests/oauth-errors.test.ts, packages/auth/src/tests/run-bun-tests.behavior.test.ts, packages/auth/test-fixtures/self-sigterm.fixture.ts, packages/core/test/utils/ripgrepPathResolver.test.ts, packages/core/src/recording/sessionManagement.test.ts, packages/core/src/integration-tests/provider-settings-integration.spec.ts, packages/core/src/integration-tests/provider-settings-integration.test.ts, packages/core/src/config/config.folderTrustMcpWiring.test.ts, packages/core/src/config/config.d.test.ts Migrates vitest suites to bun:test, adds new behavioral and integration coverage for OAuth retry timing and runner diagnostics, and introduces a self-SIGTERM fixture to validate signal propagation.
docs project-plans/issue2904/plan.md Documents the issue 2904 implementation plan with behavioral requirements, TDD sequence, and scope boundaries for Bun test timing and runner diagnostics.

Sequence Diagram

sequenceDiagram
  participant Runner as run-bun-tests runner
  participant FS as filesystem
  participant Child as bun test child process
  participant Report as JUnit report

  Runner->>FS: discoverTestFiles(WORKSPACE_ROOT)
  FS-->>Runner: sorted test file paths
  Runner->>Runner: batch files by CONCURRENCY
  loop for each batch
    Runner->>Child: spawn process.execPath test --preload bun-preload.ts <file>
    Runner->>Runner: start PER_FILE_TIMEOUT_MS timer
    alt child exits before timeout
      Child-->>Runner: exit code
      Runner->>Runner: clear timeout and resolve TestResult
    else timeout expires first
      Runner->>Child: kill SIGKILL
      Runner-->>Runner: resolve TestResult with timedOut=true
    end
  end
  Runner->>Runner: aggregate passed/failed results
  Runner->>Report: writeFileSync(JUNIT_PATH, generateJUnit(...))
  Runner->>Runner: process.exit(1) if failures else process.exit(0)
Loading

Magnitude

🎯 1 (S)
536 additions, 210 deletions, 10 changed files across 2 packages, 0 acceptance criteria

Related

Pre-merge Checks

Check Status Note
Title Clear and descriptive; states the improvement area (Bun test timing and runner diagnostics) and references the fix (#2904).
Description Includes all required template sections: TLDR, Dive Deeper, Reviewer Test Plan, Testing Matrix, and Linked issues / bugs.
Linked Issues Addresses the pre-existing test quality findings from issue #2904: converts oauth-errors to bun:test with deterministic timing evidence, replaces broad timer drains with exact 29,999/30,000 ms boundaries, renames provider-settings-integration for runner discovery, narrows MCP mock types, extracts ripgrep mock helper, removes unnecessary scoping, and adds signal-aware JUnit/console failure reporting. One low-priority maintainability item (inconsistent numRuns in sessionManagement.test.ts) was not explicitly addressed.
Out of Scope No production code, dependencies, or workflow changes. The inconsistent numRuns across property-based tests in sessionManagement.test.ts (issue finding #7) was not remediated; test logic was migrated but the inconsistency remains. OCR's dismissed false-positive for toolDeclaration.test.ts was correctly left untouched.

Walkthrough generated by LLxprt PR Review. Planner issue: #2256

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Summary

Package Lines Statements Functions Branches
CLI N/A% N/A% N/A% N/A%
Core N/A% N/A% N/A% N/A%
CLI Package - Full Text Report
CLI full-text-summary.txt not found at: coverage_cli/packages/cli/coverage/full-text-summary.txt
Core Package - Full Text Report
Core full-text-summary.txt not found at: coverage_core/packages/core/coverage/full-text-summary.txt

For detailed HTML reports, please see the 'coverage-reports-24.x-ubuntu-latest' artifact from the main CI run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer:e2e:ok Trusted contributor; maintainer-approved E2E run

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow-up: pre-existing test quality findings from Bun migration OCR review (PR #2858)

1 participant