Skip to content

test(browser): stop the unit suite losing its page mid-run - #1596

Merged
Ryang-21 merged 2 commits into
v17-feature-branchfrom
flaky-test-fix
Aug 6, 2026
Merged

test(browser): stop the unit suite losing its page mid-run#1596
Ryang-21 merged 2 commits into
v17-feature-branchfrom
flaky-test-fix

Conversation

@quietbits

@quietbits quietbits commented Aug 6, 2026

Copy link
Copy Markdown
Contributor
  • Fixed the browser unit suite losing its browser page mid-run — each of the 117 test files re-evaluated the ~470-module src/xdr graph in its own iframe until Firefox lost the page partway through
  • Replaced the vi.mock in client_from.test.ts with a Server injected via options.server — sharing one iframe means a module mock leaks into every file that runs after it
  • Set restoreMocks and unstubGlobals on every vitest config as a backstop, since test/unit runs under both the node and the browser runner
  • Gave the new spy a throwing default, since vi.spyOn falls through to the real transport once queued responses run out where the previous bare vi.fn returned undefined
  • Added toHaveBeenCalledTimes assertions to both Client.from tests, enforcing the SAC case's documented "no second ledger-entries call" claim
  • Cut the browser run to ~6s, since the SDK module graph is now evaluated once per browser instead of 117 times

@github-project-automation github-project-automation Bot moved this to Backlog (Not Ready) in DevX Aug 6, 2026
@quietbits
quietbits requested a balanced review from Copilot August 6, 2026 18:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates browser test execution to reduce memory usage and improves Client.from unit tests by avoiding module-level transport mocks.

Changes:

  • Refactor contract.Client.from tests to inject a real rpc.Server and spy on server.httpClient.post.
  • Add assertions around RPC call behavior in Client.from tests.
  • Disable Vitest browser isolation to reuse a single iframe across test files and avoid repeated module graph evaluation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
test/unit/contract/client_from.test.ts Replaces module mocking with injected Server + httpClient.post spy and adds RPC-call assertions.
config/vitest.config.browser.ts Disables test isolation in browser mode to reduce memory growth and improve runtime.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread config/vitest.config.browser.ts
Comment thread config/vitest.config.browser.ts Outdated
Comment thread test/unit/contract/client_from.test.ts
Comment thread test/unit/contract/client_from.test.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@Ryang-21 Ryang-21 closed this Aug 6, 2026
@github-project-automation github-project-automation Bot moved this from Backlog (Not Ready) to Done in DevX Aug 6, 2026
@Ryang-21 Ryang-21 reopened this Aug 6, 2026
@Ryang-21
Ryang-21 merged commit 34bedce into v17-feature-branch Aug 6, 2026
15 checks passed
@Ryang-21
Ryang-21 deleted the flaky-test-fix branch August 6, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants