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

Author: quietbits

config/vitest-utils.ts

@@ -1,5 +1,24 @@
 import { dirname, resolve } from "path";
 
+/**
+ * Test-isolation backstops shared by every vitest config, so protection does
+ * not depend on which runner a test file happens to execute under —
+ * `test/unit` runs under both the node and the browser config.
+ *
+ * Vitest restores spies and unstubs globals between tests, so a test that
+ * forgets to undo either one cannot leak into the tests that follow. This
+ * matters most under the browser config, which shares a single iframe — and
+ * therefore a single module registry — across every test file.
+ *
+ * Scope is spies and `vi.stubGlobal` only. Module-level state — a `vi.mock`,
+ * or a mutation of an SDK singleton like `Config` or `SERVER_TIME_MAP` — is
+ * not restored, and under a shared registry it outlives the file that set it.
+ */
+export const mockSafeguards = {
+  restoreMocks: true,
+  unstubGlobals: true,
+} as const;
+
 export function aliasHttpClientToAxiosSource(isAxios: boolean) {
   const fetchClientSource = resolve(__dirname, "../src/http-client/index.js");
   const fetchClientDirectory = resolve(__dirname, "../src/http-client");

config/vitest.config.browser.ts

@@ -1,7 +1,7 @@
 import { defineConfig } from "vitest/config";
 import { playwright } from "@vitest/browser-playwright";
 import packageJson from "../package.json" with { type: "json" };
-import { aliasHttpClientToAxiosSource } from "./vitest-utils";
+import { aliasHttpClientToAxiosSource, mockSafeguards } from "./vitest-utils";
 import { resolve } from "path";
 const isAxios = process.env.TRANSPORT === "axios";
 
@@ -20,6 +20,30 @@ export default defineConfig({
   test: {
     globals: true,
     environment: "jsdom",
+    // Reuse one iframe across all test files instead of creating a fresh one
+    // per file. Every test file imports the SDK source entrypoint, so with
+    // isolation each of the 117 files re-evaluated the ~470-module `src/xdr`
+    // graph from scratch; the browser page grew until Firefox lost it mid-run
+    // ("Browser connection was closed while running tests"), consistently
+    // around file ~60 with every test that had run passing. Sharing the iframe
+    // evaluates that graph once, which keeps memory flat and cuts the run from
+    // ~16s to ~6s.
+    //
+    // The trade-off is that the module registry is shared across files, so
+    // module-level state now outlives the file that created it. A
+    // module-level `vi.mock` leaks into every file that runs after it — avoid
+    // module mocks for that reason: tests that need a stubbed transport
+    // inject a `Server` and spy on `server.httpClient` instead (see
+    // test/unit/contract/client_from.test.ts and test/unit/server/soroban/).
+    // Per-test spies are fine — they are attached to an instance built inside
+    // the test, so they cannot outlive it. The SDK's own module-level state
+    // (`Config`, `SERVER_TIME_MAP`) is shared the same way, and the safeguards
+    // below do not restore it: a test that mutates it must reset it itself.
+    isolate: false,
+    // Backstop for the mock rule above — spies and `vi.stubGlobal` only, not
+    // module state. Shared with every other config so the guarantee does not
+    // depend on which runner executes the file.
+    ...mockSafeguards,
     coverage: {
       provider: "istanbul",
       reporter: ["text", "html", "lcov"],

config/vitest.config.e2e.ts

@@ -1,10 +1,14 @@
 import { defineConfig } from 'vitest/config'
 import { resolve } from 'path'
+import { mockSafeguards } from './vitest-utils'
 
 export default defineConfig({
   test: {
     globals: true,
     environment: 'node',
+    // Shared with the other configs so the cleanup guarantee is uniform; inert
+    // here today, since test/e2e uses no spies or global stubs.
+    ...mockSafeguards,
     coverage: {
       provider: 'v8',
       reporter: ['text', 'html', 'lcov'],

config/vitest.config.ts

@@ -1,7 +1,7 @@
 import { defineConfig } from "vitest/config";
 import { resolve } from "path";
 import packageJson from "../package.json" with { type: "json" };
-import { aliasHttpClientToAxiosSource } from "./vitest-utils";
+import { aliasHttpClientToAxiosSource, mockSafeguards } from "./vitest-utils";
 
 const isAxios = process.env.TRANSPORT === "axios";
 
@@ -24,6 +24,9 @@ export default defineConfig({
   test: {
     globals: true,
     environment: "node",
+    // Shared with the browser config: `test/unit` runs under both, so the
+    // cleanup guarantee must not depend on which runner picks up the file.
+    ...mockSafeguards,
     coverage: {
       provider: "v8",
       reporter: ["text", "html", "lcov"],

test/unit/contract/client_from.test.ts

@@ -1,27 +1,12 @@
-import { describe, it, afterEach, expect, vi } from "vitest";
+import { describe, it, beforeEach, expect, vi } from "vitest";
 import { concatUint8Arrays, stringToUint8Array } from "uint8array-extras";
 import * as StellarSdk from "../../../src/index.js";
 
 import { serverUrl } from "../../constants";
 
-// `Client.from` constructs its own `Server` internally, so we cannot spy on a
-// server instance we create here. Instead we mock the http-client factory that
-// the internal `Server` uses, which lets the real code path
-// (`getContractInstance`, then `getContractWasmByHash` for Wasm contracts) run
-// against controlled JSON-RPC responses.
-const { mockPost } = vi.hoisted(() => ({ mockPost: vi.fn() }));
-
-vi.mock("../../../src/rpc/axios.js", async (importActual) => {
-  const actual =
-    await importActual<typeof import("../../../src/rpc/axios.js")>();
-  return {
-    ...actual,
-    createHttpClient: () => ({ post: mockPost }),
-  };
-});
-
-const { xdr, hash, Contract } = StellarSdk;
+const { xdr, hash, Contract, rpc } = StellarSdk;
 const { Client } = StellarSdk.contract;
+const { Server } = rpc;
 
 const networkPassphrase = "Test SDF Network ; September 2015";
 
@@ -65,8 +50,21 @@ function wasmWithSpec(entries: StellarSdk.xdr.ScSpecEntry[]): Uint8Array {
 }
 
 describe("contract.Client.from", () => {
-  afterEach(() => {
-    vi.clearAllMocks();
+  let server: any;
+  let mockPost: any;
+
+  // `Client.from` accepts a pre-built `Server` via `options.server`, so spying
+  // on that instance's http client lets the real code path
+  // (`getContractInstance`, then `getContractWasmByHash` for wasm contracts)
+  // run against controlled JSON-RPC responses without mocking a module.
+  beforeEach(() => {
+    server = new Server(serverUrl);
+    // The default throws rather than calling through: `vi.spyOn` keeps the real
+    // implementation, so once the queued `mockResolvedValueOnce` responses run
+    // out an unexpected call would otherwise issue a real HTTP request.
+    mockPost = vi.spyOn(server.httpClient, "post").mockImplementation(() => {
+      throw new Error("unexpected RPC call");
+    });
   });
 
   const contractId = "CCN57TGC6EXFCYIQJ4UCD2UDZ4C3AQCHVMK74DGZ3JYCA5HD4BY7FNPC";
@@ -155,10 +153,13 @@ describe("contract.Client.from", () => {
         contractId,
         networkPassphrase,
         rpcUrl: serverUrl,
+        server,
       });
 
       expect(client).toBeInstanceOf(Client);
       expect(client.spec.funcs().length).toBeGreaterThan(0);
+      // The instance lookup, then the wasm fetch.
+      expect(mockPost).toHaveBeenCalledTimes(2);
     });
   });
 
@@ -191,6 +192,7 @@ describe("contract.Client.from", () => {
         contractId,
         networkPassphrase,
         rpcUrl: serverUrl,
+        server,
       });
 
       expect(client).toBeInstanceOf(Client);
@@ -204,6 +206,7 @@ describe("contract.Client.from", () => {
       ]) {
         expect(typeof (client as any)[method]).toBe("function");
       }
+      expect(mockPost).toHaveBeenCalledTimes(1);
     });
   });
 });