From 8ee77ce57adca2839f20a5a7ae3c13ede6767073 Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Sun, 30 Aug 2026 00:34:22 +0200 Subject: [PATCH 1/2] test(ipc): count leftover staging folders in a temp root of the test's own The cleanup tests read the machine-wide temp root and counted entries by name prefix, so a staging folder another run was still using read as a leak. Each test now points the temp root at a folder inside its own workspace, where anything left over is its own. The extraction one was also filtering on a prefix the worker stopped using, so it counted zero on both sides and passed even with the cleanup removed. It now fails. --- tests/ipc/extraction.test.ts | 14 +++++++++++--- tests/ipc/innoExtraction.test.ts | 15 ++++++++++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/tests/ipc/extraction.test.ts b/tests/ipc/extraction.test.ts index 0d6489ff..740cc761 100644 --- a/tests/ipc/extraction.test.ts +++ b/tests/ipc/extraction.test.ts @@ -3,7 +3,7 @@ import { execFileSync } from "node:child_process" import { existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" -import { afterEach, beforeEach, describe, it } from "vitest" +import { afterEach, beforeEach, describe, it, vi } from "vitest" import * as tar from "tar" import { path7za } from "7zip-bin" @@ -50,6 +50,7 @@ beforeEach(() => { afterEach(() => { rmSync(workspace, { recursive: true, force: true }) + vi.unstubAllEnvs() }) describe("contentRoot", () => { @@ -199,13 +200,20 @@ describe("runExtraction on a gzipped tar", () => { }) it("leaves no temporary workspace behind", async () => { - const before = readdirSync(tmpdir()).filter((entry) => entry.startsWith("vs-launcher-extract-")).length + // Same reasoning as the Inno extraction's own cleanup test: the machine-wide + // temp root holds other runs' staging folders, so this one gets a root of its + // own and what is left in it at the end belongs to this call. + const temporaryRoot = workspacePath("temp-root") + mkdirSync(temporaryRoot) + vi.stubEnv("TMPDIR", temporaryRoot) + vi.stubEnv("TMP", temporaryRoot) + vi.stubEnv("TEMP", temporaryRoot) writeTree(workspacePath("source"), { vintagestory: { Vintagestory: "elf" } }) const archivePath = await makeTarGz("clean.tar.gz", workspacePath("source")) await runExtraction({ filePath: archivePath, outputPath: workspacePath("target"), deleteArchive: false, sevenZipBin }) - assert.equal(readdirSync(tmpdir()).filter((entry) => entry.startsWith("vs-launcher-extract-")).length, before) + assert.deepEqual(readdirSync(temporaryRoot), []) }) }) diff --git a/tests/ipc/innoExtraction.test.ts b/tests/ipc/innoExtraction.test.ts index 9ee1e3b2..29767def 100644 --- a/tests/ipc/innoExtraction.test.ts +++ b/tests/ipc/innoExtraction.test.ts @@ -3,7 +3,7 @@ import { createHash } from "node:crypto" import { copyFileSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, statSync, symlinkSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" -import { afterEach, beforeEach, describe, it } from "vitest" +import { afterEach, beforeEach, describe, it, vi } from "vitest" import { runInnoExtraction } from "../../src/ipc/workers/innoExtraction" @@ -37,6 +37,7 @@ beforeEach(() => { afterEach(() => { rmSync(workspace, { recursive: true, force: true }) + vi.unstubAllEnvs() }) describe("runInnoExtraction", () => { @@ -117,12 +118,20 @@ describe("runInnoExtraction", () => { }) it("leaves no temporary folder behind, whatever happened", async () => { - const before = readdirSync(tmpdir()).filter((name) => name.startsWith("riftlauncher-inno-")).length + // Staging goes wherever the temp root points, and the machine-wide one is + // shared with everything else running: counting folders by name there counts + // the ones other runs are still using. These two calls get a temp root to + // themselves, so whatever is left in it at the end is theirs. + const temporaryRoot = workspacePath("temp-root") + mkdirSync(temporaryRoot) + vi.stubEnv("TMPDIR", temporaryRoot) + vi.stubEnv("TMP", temporaryRoot) + vi.stubEnv("TEMP", temporaryRoot) await runInnoExtraction({ filePath: installerFrom("valid.bin"), outputPath: workspacePath("target"), deleteInstaller: false }) await runInnoExtraction({ filePath: installerFrom("wrong-digest.bin"), outputPath: workspacePath("other"), deleteInstaller: false }) - assert.equal(readdirSync(tmpdir()).filter((name) => name.startsWith("riftlauncher-inno-")).length, before) + assert.deepEqual(readdirSync(temporaryRoot), []) }) }) From 775a129d21999763c654ec0a446650d1a3fe5087 Mon Sep 17 00:00:00 2001 From: Pixnop <77785313+Pixnop@users.noreply.github.com> Date: Sun, 30 Aug 2026 00:38:39 +0200 Subject: [PATCH 2/2] test(renderer): put the provider-guard tests behind one helper Both tests that check a context hook throws outside its provider now go through expectHookThrowsOutsideProvider, which catches the throw inside the component and asserts nothing reached console.error. The config one used to let the throw escape and silence the window error event it set off, which is the shape that can fail an unrelated file. useNotificationsContext had a guard that could not fire: the context carries a working no-op default, so the hook never sees an absent value. It is gone, and the reason it never threw is written where it was. --- .../src/contexts/NotificationsContext.tsx | 13 ++++--- .../renderer-dom/configContextSlices.test.tsx | 21 ++--------- tests/renderer-dom/helpers/render.tsx | 35 ++++++++++++++++++- tests/renderer-dom/taskManagerFlows.test.tsx | 28 ++------------- 4 files changed, 46 insertions(+), 51 deletions(-) diff --git a/src/renderer/src/contexts/NotificationsContext.tsx b/src/renderer/src/contexts/NotificationsContext.tsx index 2dcb5d55..d35519d9 100644 --- a/src/renderer/src/contexts/NotificationsContext.tsx +++ b/src/renderer/src/contexts/NotificationsContext.tsx @@ -139,12 +139,11 @@ const NotificationsProvider = ({ children }: { children: React.ReactNode }): JSX return {children} } -const useNotificationsContext = (): NotificationsContextType => { - const context = useContext(NotificationsContext) - if (!context) { - throw new Error("useNotificationsContext must be used within an NotificationsProvider") - } - return context -} +/** + * The context ships the no-op `defaultValue` above, so a call from outside a + * provider gets that and never nothing. There is no absent case to guard, which + * is why this one has no throw where the task and config hooks have one. + */ +const useNotificationsContext = (): NotificationsContextType => useContext(NotificationsContext) export { NotificationsProvider, useNotificationsContext } diff --git a/tests/renderer-dom/configContextSlices.test.tsx b/tests/renderer-dom/configContextSlices.test.tsx index c8144e7d..35a62d37 100644 --- a/tests/renderer-dom/configContextSlices.test.tsx +++ b/tests/renderer-dom/configContextSlices.test.tsx @@ -1,11 +1,11 @@ import { describe, expect, it, vi } from "vitest" -import { render, screen } from "@testing-library/react" +import { screen } from "@testing-library/react" import userEvent from "@testing-library/user-event" import { CONFIG_ACTIONS, useAccountList, useConfigDispatch, useCustomIcons, useFavMods, useGameVersions, useInstallations, useSettingsConfig } from "@renderer/features/config/contexts/ConfigContext" import { createMockConfig, installMockWindowApi } from "./helpers/windowApi" -import { renderWithProviders } from "./helpers/render" +import { expectHookThrowsOutsideProvider, renderWithProviders } from "./helpers/render" function anInstallation(overrides: Partial = {}): InstallationType { return { @@ -153,21 +153,6 @@ describe("config slice contexts", () => { }) it("throws by name when a slice hook is used outside the provider", () => { - function Orphan(): JSX.Element { - useInstallations() - return

never rendered

- } - - // React re-reports the throw through console.error and a window error - // event; both are silenced so a passing run stays readable. - const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}) - const swallow = (event: ErrorEvent): void => event.preventDefault() - window.addEventListener("error", swallow) - try { - expect(() => render()).toThrow(/useInstallations must be used within a ConfigProvider/) - } finally { - window.removeEventListener("error", swallow) - consoleError.mockRestore() - } + expectHookThrowsOutsideProvider(useInstallations, /useInstallations must be used within a ConfigProvider/) }) }) diff --git a/tests/renderer-dom/helpers/render.tsx b/tests/renderer-dom/helpers/render.tsx index 1c616923..e6aca097 100644 --- a/tests/renderer-dom/helpers/render.tsx +++ b/tests/renderer-dom/helpers/render.tsx @@ -1,6 +1,7 @@ import type { ReactElement, ReactNode } from "react" -import { render, type RenderOptions, type RenderResult } from "@testing-library/react" +import { render, renderHook, type RenderOptions, type RenderResult } from "@testing-library/react" import { MemoryRouter } from "react-router-dom" +import { expect, onTestFinished, vi } from "vitest" import { NotificationsProvider } from "@renderer/contexts/NotificationsContext" import { ConfigProvider } from "@renderer/features/config/contexts/ConfigContext" @@ -41,3 +42,35 @@ export function renderWithProviders(ui: ReactElement, { route = "/", ...renderOp return render(ui, { wrapper: Wrapper, ...renderOptions }) } + +/** + * Checks that `useHook` throws a message matching `expected` when it is called + * with no provider above it. Every hook guarded that way is tested through here. + * + * The throw is caught inside the component on purpose, and every one of these + * must be written that way. A render throw that escapes reaches React's + * development-only replay, which rethrows it through a synthetic DOM event so + * devtools can see it; jsdom turns that into an uncancelled window "error" + * event, and the jsdom environment re-emits such an event as an uncaught + * exception, which fails the whole run and pins the failure on whichever file + * happened to be running at the time. React also prints "The above error + * occurred in ..." for every render throw it handles itself, so the console + * check at the end is what fails first if an edit ever lets one escape again. + */ +export function expectHookThrowsOutsideProvider(useHook: () => unknown, expected: RegExp): void { + const consoleError = vi.spyOn(console, "error") + onTestFinished(() => consoleError.mockRestore()) + + let thrown: unknown + renderHook(() => { + try { + useHook() + } catch (error) { + thrown = error + } + }) + + expect(thrown).toBeInstanceOf(Error) + expect((thrown as Error).message).toMatch(expected) + expect(consoleError).not.toHaveBeenCalled() +} diff --git a/tests/renderer-dom/taskManagerFlows.test.tsx b/tests/renderer-dom/taskManagerFlows.test.tsx index 8571441a..eac847fb 100644 --- a/tests/renderer-dom/taskManagerFlows.test.tsx +++ b/tests/renderer-dom/taskManagerFlows.test.tsx @@ -1,5 +1,5 @@ import type { ReactElement, ReactNode } from "react" -import { describe, expect, it, onTestFinished, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { act, renderHook, waitFor } from "@testing-library/react" import type { RenderHookResult } from "@testing-library/react" @@ -7,6 +7,7 @@ import { NotificationsProvider, useNotificationsContext } from "@renderer/contex import { ACTIONS, TaskProvider, taskReducer, useTaskContext } from "@renderer/contexts/TaskManagerContext" import type { TaskNotificationsMode, TaskType } from "@renderer/contexts/TaskManagerContext" +import { expectHookThrowsOutsideProvider } from "./helpers/render" import { installMockWindowApi } from "./helpers/windowApi" // Registers the i18n instance useTranslation() reads inside TaskProvider and @@ -79,30 +80,7 @@ describe("taskReducer", () => { describe("useTaskContext", () => { it("throws when used outside a TaskProvider", () => { - // The throw is caught inside the component on purpose. A render throw - // that escapes reaches React's dev-only replay, which rethrows it - // through a synthetic DOM event so devtools can see it; jsdom turns that - // into an uncancelled window "error" event, and Vitest's jsdom - // environment re-emits such an event as an uncaught exception that fails - // the whole run and gets pinned on whichever file happened to be running. - const consoleError = vi.spyOn(console, "error") - onTestFinished(() => consoleError.mockRestore()) - - let thrown: unknown - renderHook(() => { - try { - useTaskContext() - } catch (error) { - thrown = error - } - }) - - expect(thrown).toBeInstanceOf(Error) - expect((thrown as Error).message).toMatch(/must be used within an TaskProvider/) - // React logs "The above error occurred in ..." for every render throw it - // has to handle itself, so this fails right here if the throw is ever let - // out of the component again. - expect(consoleError).not.toHaveBeenCalled() + expectHookThrowsOutsideProvider(useTaskContext, /must be used within an TaskProvider/) }) })