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/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), []) }) }) 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/) }) })