Deflake Vitest tests (#34570)

* Deflake Vitest MatrixChat test

* Update MatrixChat.test.tsx

* Stabilise further

* Stabilise further

* Stabilise init

apps/web/src/components/structures/MatrixChat.test.tsx

@@ -329,13 +329,16 @@ describe("<MatrixChat />", () => {
         localStorage.clear();
         vi.clearAllTimers();
 
+        // RTL cleanup won't touch roots we render ourselves so clean those up manually
+        await clearAllModals();
+
         // This is a massive hack, but a lot of these tests end up completing while the login flow is still proceeding.
         // So then, we start the next test while stuff is still ongoing from the previous test, which messes up the current test.
         // There is no obvious event we could wait for which indicates that everything has completed,
         // since each test does something different. Instead, we just let real timers and microtasks drain.
         await act(() => sleep(200));
 
-        // RTL cleanup won't touch roots we render ourselves so clean those up manually
+        // Anything the drain kicked off may have opened a dialog again
         await clearAllModals();
     });
 

apps/web/src/test/setupTests.ts

@@ -6,7 +6,8 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import { vi, beforeEach, afterEach } from "vitest";
-import fetchMock, { manageFetchMockGlobally } from "@fetch-mock/vitest";
+import { act } from "react";
+import fetchMock from "@fetch-mock/vitest";
 
 import SdkConfig, { DEFAULTS } from "../SdkConfig";
 import "./setupGlobals.ts";
@@ -18,29 +19,19 @@ declare global {
 
 globalThis.IS_REACT_ACT_ENVIRONMENT = true;
 
-// Ignore benign post-teardown exceptions as they cause flakes
+// Captured before any test can install fake timers, so the drain in `afterEach` below always
-const guardState = globalThis as unknown as { __vitestTestRunning?: boolean; __teardownGuardInstalled?: boolean };
+// runs against a real immediate and cannot hang.
-if (!guardState.__teardownGuardInstalled) {
+const realSetImmediate = globalThis.setImmediate;
-    guardState.__teardownGuardInstalled = true;
-    const isPostTeardownStraggler = (): boolean => !guardState.__vitestTestRunning || typeof window === "undefined";
-    process.on("uncaughtException", (err) => {
-        if (isPostTeardownStraggler()) return;
-        throw err;
-    });
-    process.on("unhandledRejection", (reason) => {
-        if (isPostTeardownStraggler()) return;
-        throw reason;
-    });
-}
 
-manageFetchMockGlobally();
+// Deliberately *not* calling `manageFetchMockGlobally()` as it monkey-patches `vi.restoreAllMocks`,
+// `vi.resetAllMocks` and `vi.unstubAllGlobals` such that they also tear the fetch mock down, putting the
+// environment's real `fetch` back on the global.
+// We re-set the mock before every test below, so the lifecycle integration buys us nothing.
 
 beforeEach(() => {
-    guardState.__vitestTestRunning = true;
-
     vi.stubEnv("TZ", "UTC");
 
-    // set up fetch API mock
+    // set up fetch API mock. Unmatched requests 404 rather than reaching the network.
     fetchMock.hardReset();
     fetchMock.catch(404);
     fetchMock.mockGlobal();
@@ -48,9 +39,12 @@ beforeEach(() => {
     setupLanguageMock();
 });
 
-afterEach(() => {
+afterEach(async () => {
-    guardState.__vitestTestRunning = false;
+    await fetchMock.callHistory.flush();
-    return fetchMock.callHistory.flush();
+
+    await act(async () => {
+        await new Promise((resolve) => realSetImmediate(resolve));
+    });
 });
 
 // uninitialised SdkConfig causes lots of warnings in console, init with defaults

apps/web/src/vector/init.test.ts

@@ -57,6 +57,27 @@ describe("loadApp", () => {
         await waitFor(() => expect(window.matrixChat).toBeInstanceOf(MatrixChat));
     });
 
+    it("should replace the previous app rather than leaving it mounted", async () => {
+        await loadApp({});
+        await waitFor(() => expect(window.matrixChat).toBeInstanceOf(MatrixChat));
+        const first = window.matrixChat;
+
+        // Count only what the second load does. We track the mounted/unmounted delta rather than raw
+        // mount count, as StrictMode's extra mount/unmount cycle cancels out in the difference.
+        const mounted = vi.spyOn(MatrixChat.prototype, "componentDidMount");
+        const unmounted = vi.spyOn(MatrixChat.prototype, "componentWillUnmount");
+        const delta = (): number => mounted.mock.calls.length - unmounted.mock.calls.length;
+
+        setUpMatrixChatDiv();
+        await loadApp({});
+        await waitFor(() => expect(window.matrixChat).not.toBe(first));
+
+        // The new app replaces the old one, so the number of live apps is unchanged. A second root over
+        // the same container would instead leave the first tree mounted against a detached node, with both
+        // copies still driven by the dispatcher and the client peg.
+        await waitFor(() => expect(delta()).toBe(0));
+    });
+
     it("should pass onTokenLoginCompleted which strips searchParams & fragment to MatrixChat", async () => {
         const spy = vi.spyOn(window.history, "replaceState");
 

apps/web/src/vector/init.tsx

@@ -8,7 +8,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */
 
-import { createRoot } from "react-dom/client";
+import { createRoot, type Root } from "react-dom/client";
 import React, { StrictMode } from "react";
 import { logger } from "matrix-js-sdk/src/logger";
 import { ModuleLoader } from "@element-hq/element-web-module-api";
@@ -32,6 +32,25 @@ import { type URLParams } from "./url_utils.ts";
 
 export const rageshakePromise = initRageshake();
 
+let root: Root | undefined;
+let rootContainer: Element | undefined;
+
+/**
+ * Get the React root for the `#matrixchat` container.
+ *
+ * These views replace one another (`showError` may be called after `loadApp`, for instance), so they share a
+ * single root: calling `createRoot` again for the same container leaves the previous tree mounted and running
+ * against a detached DOM node, with both copies still subscribed to the dispatcher and the client peg.
+ */
+function getRoot(): Root {
+    const container = document.getElementById("matrixchat")!;
+    if (root && rootContainer === container) return root;
+    root?.unmount();
+    rootContainer = container;
+    root = createRoot(container);
+    return root;
+}
+
 export function preparePlatform(): void {
     if (window.electron) {
         logger.log("Using Electron platform");
@@ -98,8 +117,7 @@ export async function loadApp(urlParams: URLParams): Promise<void> {
         window.matrixChat = matrixChat;
     }
     const app = await module.loadApp(urlParams, setWindowMatrixChat);
-    const root = createRoot(document.getElementById("matrixchat")!);
+    getRoot().render(app);
-    root.render(app);
 }
 
 export async function showError(title: string, messages?: string[]): Promise<void> {
@@ -107,8 +125,7 @@ export async function showError(title: string, messages?: string[]): Promise<voi
         /* webpackChunkName: "error-view" */
         "../async-components/structures/ErrorView"
     );
-    const root = createRoot(document.getElementById("matrixchat")!);
+    getRoot().render(
-    root.render(
         <StrictMode>
             <ErrorView title={title} messages={messages} />
         </StrictMode>,
@@ -120,8 +137,7 @@ export async function showIncompatibleBrowser(onAccept: () => void): Promise<voi
         /* webpackChunkName: "error-view" */
         "../async-components/structures/ErrorView"
     );
-    const root = createRoot(document.getElementById("matrixchat")!);
+    getRoot().render(
-    root.render(
         <StrictMode>
             <UnsupportedBrowserView onAccept={onAccept} />
         </StrictMode>,