Skip to content

test(websearch): stabilize Brave timeout coverage - #1959

Merged
kevincodex1 merged 1 commit into
Twigpine:mainfrom
jatmn:fix/brave-timeout-full-suite-flake
Jul 14, 2026
Merged

kevincodex1 merged 1 commit into
Twigpine:mainfrom
jatmn:fix/brave-timeout-full-suite-flake

Conversation

@jatmn

@jatmn jatmn commented Jul 13, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Acquire the Brave provider test's shared global-mutation lock during module setup instead of its per-test hook.
  • Snapshot and restore process.env and globalThis.fetch only while owning that lock.
  • Keep queued lock time outside Bun's individual five-second test deadline, while preserving the one-second provider-timeout and abort assertions.

Why

PR #1939's Node 22 smoke run failed the target test at 5936.95ms, then passed when that job was retried without a code change. The asynchronous beforeEach lock wait was included in the test's deadline; a roughly five-second queue wait plus the deliberate one-second provider timeout caused the flake.

Validation

  • bun test src/tools/WebSearchTool/providers/brave.test.ts (10 pass)
  • Serialized neighboring WebSearch provider tests (96 pass)
  • Concurrent Brave/provider probes (39 pass; 14 pass; 10 pass)
  • bun run test:full: all Brave tests pass, including the target at ~1.00s. It retains the 24 unrelated failures reproduced unchanged on clean upstream main.
  • git diff --check

Notes

The complete baseline failures are the stale startup-provider override, PowerShell/git commit-governance, and full-plan-instructions cases; they reproduce identically from upstream main. security:pr-scan is not reported because it compares against the stale fork-origin baseline and scans unrelated upstream history.

Summary by CodeRabbit

  • Tests
    • Improved test isolation and cleanup for web search provider tests.
    • Ensured shared test resources are acquired and released consistently across the test suite.
    • Simplified restoration of environment variables and network request behavior between tests.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The Brave provider test file now acquires the shared mutation lock once during module setup and releases it after all tests. Per-test cleanup only restores environment variables and the captured global fetch implementation.

Changes

Brave provider test synchronization

Layer / File(s) Summary
File-level mutation lock lifecycle
src/tools/WebSearchTool/providers/brave.test.ts
Moves lock acquisition to top-level setup, keeps environment and fetch restoration in afterEach, and releases the lock in afterAll.

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

Possibly related PRs

Suggested reviewers: adityachaudhary99, chioarub, dnakhla

🚥 Pre-merge checks | ✅ 6 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Risk Surface Disclosed ❓ Inconclusive No review artifact is present; the PR text notes the timeout/lock change, but I can't verify an explicit risk/blocker callout. Please provide the actual review comment or review output so I can confirm it explicitly states the risk surface and blocker status.
✅ Passed checks (6 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.
No Hidden Policy Change ✅ Passed Only Brave test-harness cleanup changed; no product, trust-model, routing, telemetry/network, or permission-policy logic is introduced.
Title check ✅ Passed The title is concise and accurately reflects the Brave websearch test timeout stabilization change.
Description check ✅ Passed The description covers summary, rationale, validation, and notes, though it lacks the template's Impact section and checklist formatting.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@jatmn
jatmn marked this pull request as ready for review July 13, 2026 22:08
@jatmn jatmn self-assigned this Jul 13, 2026
@jatmn jatmn added the bug Something isn't working label Jul 13, 2026

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@kevincodex1
kevincodex1 merged commit 6b2a1d8 into Twigpine:main Jul 14, 2026
5 checks passed
@jatmn
jatmn deleted the fix/brave-timeout-full-suite-flake branch July 14, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants