From 127ae519c5fb216db1810fb1b82c64e2f7763598 Mon Sep 17 00:00:00 2001
From: Michael Telatynski <7t3chguy@gmail.com>
Date: Mon, 10 Aug 2026 11:23:56 +0100
Subject: [PATCH] Deflake Vitest tests (#34570)
* Deflake Vitest MatrixChat test
* Update MatrixChat.test.tsx
* Stabilise further
* Stabilise further
* Stabilise init
---
.../components/structures/MatrixChat.test.tsx | 5 ++-
apps/web/src/test/setupTests.ts | 38 ++++++++-----------
apps/web/src/vector/init.test.ts | 21 ++++++++++
apps/web/src/vector/init.tsx | 30 +++++++++++----
4 files changed, 64 insertions(+), 30 deletions(-)
diff --git a/apps/web/src/components/structures/MatrixChat.test.tsx b/apps/web/src/components/structures/MatrixChat.test.tsx
index d5eb98a4c7..8b3e7ef3aa 100644
--- a/apps/web/src/components/structures/MatrixChat.test.tsx
+++ b/apps/web/src/components/structures/MatrixChat.test.tsx
@@ -329,13 +329,16 @@ describe("", () => {
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();
});
diff --git a/apps/web/src/test/setupTests.ts b/apps/web/src/test/setupTests.ts
index 13f8f64cb0..03f24c0185 100644
--- a/apps/web/src/test/setupTests.ts
+++ b/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
-const guardState = globalThis as unknown as { __vitestTestRunning?: boolean; __teardownGuardInstalled?: boolean };
-if (!guardState.__teardownGuardInstalled) {
- 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;
- });
-}
+// Captured before any test can install fake timers, so the drain in `afterEach` below always
+// runs against a real immediate and cannot hang.
+const realSetImmediate = globalThis.setImmediate;
-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(() => {
- guardState.__vitestTestRunning = false;
- return fetchMock.callHistory.flush();
+afterEach(async () => {
+ await fetchMock.callHistory.flush();
+
+ await act(async () => {
+ await new Promise((resolve) => realSetImmediate(resolve));
+ });
});
// uninitialised SdkConfig causes lots of warnings in console, init with defaults
diff --git a/apps/web/src/vector/init.test.ts b/apps/web/src/vector/init.test.ts
index 70a9e8f67d..37d0582be8 100644
--- a/apps/web/src/vector/init.test.ts
+++ b/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");
diff --git a/apps/web/src/vector/init.tsx b/apps/web/src/vector/init.tsx
index 90ebb2dde6..208f29d0bf 100644
--- a/apps/web/src/vector/init.tsx
+++ b/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 {
window.matrixChat = matrixChat;
}
const app = await module.loadApp(urlParams, setWindowMatrixChat);
- const root = createRoot(document.getElementById("matrixchat")!);
- root.render(app);
+ getRoot().render(app);
}
export async function showError(title: string, messages?: string[]): Promise {
@@ -107,8 +125,7 @@ export async function showError(title: string, messages?: string[]): Promise
,
@@ -120,8 +137,7 @@ export async function showIncompatibleBrowser(onAccept: () => void): Promise
,