From b82db7a3deca3dbc53dafa59930bbb3d20215857 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Mon, 23 Oct 2023 14:33:01 +0200 Subject: [PATCH] frontend: use in-memory history in test environments This removes the flakiness of location-based tests --- frontend/package-lock.json | 9 ++++ frontend/package.json | 1 + frontend/src/components/Layout.test.tsx | 39 ++--------------- .../src/components/NavItem/NavItem.test.tsx | 38 +---------------- frontend/src/routing/atoms.ts | 42 ++++++++++++++++++- frontend/src/routing/index.ts | 2 +- frontend/src/routing/routes.ts | 2 +- frontend/src/test-utils/WithLocation.tsx | 13 ++++-- frontend/tsconfig.json | 2 +- frontend/vite.config.ts | 5 +++ 10 files changed, 73 insertions(+), 80 deletions(-) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2e48a8f30..f4bf5b01b 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -22,6 +22,7 @@ "classnames": "^2.3.2", "date-fns": "^2.30.0", "graphql": "^16.8.1", + "history": "^5.3.0", "i18next": "^23.6.0", "i18next-browser-languagedetector": "^7.1.0", "i18next-http-backend": "^2.2.2", @@ -15683,6 +15684,14 @@ "integrity": "sha512-Rf4YVNYpKjZ6ASAmibcwTNciQ5Co5Ztq6iZPEykHpkoflnD/K5ryE/rHehFsTm4NJj8nKDhbi3eKBWGogmNnkg==", "dev": true }, + "node_modules/history": { + "version": "5.3.0", + "resolved": "https://registry.npmjs.org/history/-/history-5.3.0.tgz", + "integrity": "sha512-ZqaKwjjrAYUYfLG+htGaIIZ4nioX2L70ZUMIFysS3xvBsSG4x/n1V6TXV3N8ZYNuFGlDirFg32T7B6WOUPDYcQ==", + "dependencies": { + "@babel/runtime": "^7.7.6" + } + }, "node_modules/hoist-non-react-statics": { "version": "3.3.2", "resolved": "https://registry.npmjs.org/hoist-non-react-statics/-/hoist-non-react-statics-3.3.2.tgz", diff --git a/frontend/package.json b/frontend/package.json index 5e5abbaee..b0e2160c6 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -30,6 +30,7 @@ "classnames": "^2.3.2", "date-fns": "^2.30.0", "graphql": "^16.8.1", + "history": "^5.3.0", "i18next": "^23.6.0", "i18next-browser-languagedetector": "^7.1.0", "i18next-http-backend": "^2.2.2", diff --git a/frontend/src/components/Layout.test.tsx b/frontend/src/components/Layout.test.tsx index b7f441acb..93a3d26bd 100644 --- a/frontend/src/components/Layout.test.tsx +++ b/frontend/src/components/Layout.test.tsx @@ -15,57 +15,24 @@ // @vitest-environment happy-dom import { render } from "@testing-library/react"; -import { Provider } from "jotai"; -import { useHydrateAtoms } from "jotai/utils"; -import { Suspense } from "react"; import { describe, expect, it, vi, afterAll, beforeEach } from "vitest"; import { currentUserIdAtom, GqlResult } from "../atoms"; -import { appConfigAtom, locationAtom } from "../routing"; +import { WithLocation } from "../test-utils/WithLocation"; import Layout from "./Layout"; -beforeEach(async () => { - // For some reason, the locationAtom gets updated with `about:black` on render, - // so we need to set a "real" location and wait for the next tick - window.location.assign("https://example.com/"); - // Wait the next tick for the location to update - await new Promise((resolve) => setTimeout(resolve, 0)); -}); - -const HydrateLocation: React.FC> = ({ - children, - path, -}) => { - useHydrateAtoms([ - [appConfigAtom, { root: "/", graphqlEndpoint: "/graphql" }], - [locationAtom, { pathname: path }], - ]); - return <>{children}; -}; - -const WithLocation: React.FC> = ({ - children, - path, -}) => { - return ( - - - {children} - - - ); -}; - describe("", () => { beforeEach(() => { vi.spyOn(currentUserIdAtom, "read").mockResolvedValue( "abc123" as unknown as GqlResult, ); }); + afterAll(() => { vi.restoreAllMocks(); }); + it("renders app navigation correctly", async () => { const component = render( diff --git a/frontend/src/components/NavItem/NavItem.test.tsx b/frontend/src/components/NavItem/NavItem.test.tsx index 6f06f41c0..c42e70c96 100644 --- a/frontend/src/components/NavItem/NavItem.test.tsx +++ b/frontend/src/components/NavItem/NavItem.test.tsx @@ -14,47 +14,13 @@ // @vitest-environment happy-dom -import type { IWindow } from "happy-dom"; -import { Provider } from "jotai"; -import { useHydrateAtoms } from "jotai/utils"; import { create } from "react-test-renderer"; -import { beforeEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; -import { appConfigAtom, locationAtom } from "../../routing"; +import { WithLocation } from "../../test-utils/WithLocation"; import NavItem from "./NavItem"; -beforeEach(async () => { - const w = window as unknown as IWindow; - - // For some reason, the locationAtom gets updated with `about:black` on render, - // so we need to set a "real" location and wait for the next tick - w.happyDOM.setURL("https://example.com/"); - await w.happyDOM.whenAsyncComplete(); -}); - -const HydrateLocation: React.FC> = ({ - children, - path, -}) => { - useHydrateAtoms([ - [appConfigAtom, { root: "/", graphqlEndpoint: "/graphql" }], - [locationAtom, { pathname: path }], - ]); - return <>{children}; -}; - -const WithLocation: React.FC> = ({ - children, - path, -}) => { - return ( - - {children} - - ); -}; - describe("NavItem", () => { it("render an active ", () => { const component = create( diff --git a/frontend/src/routing/atoms.ts b/frontend/src/routing/atoms.ts index 83e03499f..4f0dfe348 100644 --- a/frontend/src/routing/atoms.ts +++ b/frontend/src/routing/atoms.ts @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +import { createBrowserHistory, createMemoryHistory } from "history"; import { atom } from "jotai"; import { atomWithLocation } from "jotai-location"; @@ -19,6 +20,11 @@ import appConfig, { AppConfig } from "../config"; import { Location, pathToRoute, Route, routeToPath } from "./routes"; +/* Use memory history for testing */ +export const history = import.meta.vitest + ? createMemoryHistory() + : createBrowserHistory(); + export const appConfigAtom = atom(appConfig); const locationToRoute = (root: string, location: Location): Route => { @@ -30,7 +36,41 @@ const locationToRoute = (root: string, location: Location): Route => { return pathToRoute(path); }; -export const locationAtom = atomWithLocation(); +const getLocation = (): Location => { + return { + pathname: history.location.pathname, + searchParams: new URLSearchParams(history.location.search), + }; +}; + +const applyLocation = ( + location: Location, + options?: { replace?: boolean }, +): void => { + const destination = { + pathname: location.pathname, + search: location.searchParams?.toString(), + }; + + if (options?.replace) { + history.replace(destination); + } else { + history.push(destination); + } +}; + +type Callback = () => void; +type Unsubscribe = () => void; +const subscribe = (callback: Callback): Unsubscribe => + history.listen(() => { + callback(); + }); + +export const locationAtom = atomWithLocation({ + subscribe, + getLocation, + applyLocation, +}); export const routeAtom = atom( (get) => { diff --git a/frontend/src/routing/index.ts b/frontend/src/routing/index.ts index 1e942fc56..fec86c100 100644 --- a/frontend/src/routing/index.ts +++ b/frontend/src/routing/index.ts @@ -17,5 +17,5 @@ export { default as Link } from "./Link"; export type { Route, Location } from "./routes"; export { pathToRoute, routeToPath } from "./routes"; export { getRouteActionRedirection } from "./actions"; -export { routeAtom, locationAtom, appConfigAtom } from "./atoms"; +export { routeAtom, locationAtom, appConfigAtom, history } from "./atoms"; export { useNavigationLink } from "./useNavigationLink"; diff --git a/frontend/src/routing/routes.ts b/frontend/src/routing/routes.ts index 46596dd3d..12e464044 100644 --- a/frontend/src/routing/routes.ts +++ b/frontend/src/routing/routes.ts @@ -13,7 +13,7 @@ // limitations under the License. export type Location = Readonly<{ - pathname?: string; + pathname: string; searchParams?: URLSearchParams; }>; diff --git a/frontend/src/test-utils/WithLocation.tsx b/frontend/src/test-utils/WithLocation.tsx index 71a2f24f5..10f1d11e2 100644 --- a/frontend/src/test-utils/WithLocation.tsx +++ b/frontend/src/test-utils/WithLocation.tsx @@ -12,17 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -// @vitest-environment happy-dom - import { Provider } from "jotai"; import { useHydrateAtoms } from "jotai/utils"; +import { Suspense, useEffect } from "react"; -import { appConfigAtom, locationAtom } from "../routing"; +import { appConfigAtom, history, locationAtom } from "../routing"; const HydrateLocation: React.FC> = ({ children, path, }) => { + useEffect(() => { + history.replace(path); + }, [path]); + useHydrateAtoms([ [appConfigAtom, { root: "/", graphqlEndpoint: "/graphql" }], [locationAtom, { pathname: path }], @@ -47,7 +50,9 @@ export const WithLocation: React.FC< > = ({ children, path }) => { return ( - {children} + + {children} + ); }; diff --git a/frontend/tsconfig.json b/frontend/tsconfig.json index a81f045b5..eecbb01ab 100644 --- a/frontend/tsconfig.json +++ b/frontend/tsconfig.json @@ -7,7 +7,7 @@ "DOM.Iterable", "ESNext" ], - "types": ["vite/client"], + "types": ["vite/client", "vitest/importMeta"], "allowJs": false, "skipLibCheck": true, "esModuleInterop": false, diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index b8c90c3d5..38990534c 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -36,6 +36,7 @@ function i18nHotReload(): PluginOption { }, }; } + export default defineConfig((env) => ({ base: "./", @@ -45,6 +46,10 @@ export default defineConfig((env) => ({ }, }, + define: { + "import.meta.vitest": "undefined", + }, + build: { manifest: true, assetsDir: "",