From 732e1997ddb7e1cbeef9834791428ccc4147802d Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Fri, 23 Oct 2026 23:38:56 +0000 Subject: [PATCH 1/6] test(hub): isolate Vitest state and worker databases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/hub-ci.yml | 18 ++- CONTRIBUTING.md | 2 + docs/testing-and-review.md | 2 +- hub/.devcontainer/compose.yaml | 1 + hub/.env.example | 3 + hub/AGENTS.md | 2 + .../prototype-canvas/stories.test.tsx | 7 +- .../prototype-preview-bridge/controls.test.ts | 15 ++- .../prototype-preview-bridge/index.test.ts | 20 ++-- .../copyable-text-input/index.test.tsx | 4 +- .../prototype-browser-bar/index.test.tsx | 4 +- .../stories.test.tsx | 4 +- .../repository-selector/index.test.tsx | 7 +- .../repository-user-menu/index.test.tsx | 7 +- .../scrolling-screenshot/index.test.tsx | 6 +- .../index.test.tsx | 3 +- .../index.test.ts | 4 +- .../minimized-review-toolbar/index.test.tsx | 4 +- .../index.test.tsx | 7 +- .../prototype-workspace-header/index.test.tsx | 3 +- .../review-comment-control/index.test.tsx | 3 +- .../repository-navigation/index.test.tsx | 4 +- .../index.test.tsx | 3 +- .../review-comment-reactions/index.test.tsx | 3 +- .../review-relative-time/index.test.tsx | 4 +- .../review-thread-conversation/index.test.tsx | 6 +- .../review-thread-message/index.test.tsx | 3 +- .../index.test.tsx | 7 +- .../use-canvas-prototype-state/index.test.tsx | 7 +- .../use-canvas-share-links/index.test.tsx | 7 +- .../hooks/use-emoji-burst/index.test.tsx | 7 +- .../hooks/use-hub-theme/index.test.tsx | 4 +- .../use-notification-inbox/index.test.tsx | 4 +- .../hooks/use-prototype-review/index.test.tsx | 8 +- .../hooks/use-review-comments/index.test.ts | 6 +- .../ascii-gradient/index.test.tsx | 4 +- .../pages/home-index-page/index.test.tsx | 7 +- .../pages/new-prototype-page/index.test.tsx | 7 +- .../pages/prototypes-show-page/index.test.tsx | 11 +- .../pages/replica-browse-page/index.test.tsx | 4 +- .../pages/security-audit-page/index.test.tsx | 3 +- .../storybook/inbox-prototype/index.test.tsx | 7 +- hub/app/javascript/test/setup.test.ts | 34 ++++++ hub/app/javascript/test/setup.ts | 13 +- .../index.test.ts | 13 +- .../prototype-canvas-request/index.test.ts | 4 +- hub/docs/coding-conventions.md | 2 + hub/docs/database.md | 2 + hub/docs/development.md | 2 + hub/docs/testing-and-operations.md | 29 ++++- hub/package-lock.json | 2 + hub/package.json | 5 +- hub/server/integrations/prisma/index.test.ts | 7 +- .../integrations/prisma/isolation.db.test.ts | 111 ++++++++++++++++++ .../prisma/parallel-isolation.db.test.ts | 40 +++++++ .../get-worker-database/index.test.ts | 67 +++++++++++ .../database/get-worker-database/index.ts | 24 ++++ hub/test/database/global-setup/index.ts | 88 ++++++++++++++ hub/test/database/global-setup/types.ts | 11 ++ .../database/reset-database/index.test.ts | 26 ++++ hub/test/database/reset-database/index.ts | 40 +++++++ hub/test/database/setup.ts | 12 ++ hub/test/setup.test.ts | 31 +++++ hub/test/setup.ts | 6 + hub/tsconfig.json | 1 + hub/vitest.config.ts | 21 +++- scripts/check-ci-workflows/index.ts | 1 + 67 files changed, 646 insertions(+), 188 deletions(-) create mode 100644 hub/app/javascript/test/setup.test.ts create mode 100644 hub/server/integrations/prisma/isolation.db.test.ts create mode 100644 hub/server/integrations/prisma/parallel-isolation.db.test.ts create mode 100644 hub/test/database/get-worker-database/index.test.ts create mode 100644 hub/test/database/get-worker-database/index.ts create mode 100644 hub/test/database/global-setup/index.ts create mode 100644 hub/test/database/global-setup/types.ts create mode 100644 hub/test/database/reset-database/index.test.ts create mode 100644 hub/test/database/reset-database/index.ts create mode 100644 hub/test/database/setup.ts create mode 100644 hub/test/setup.test.ts create mode 100644 hub/test/setup.ts diff --git a/.github/workflows/hub-ci.yml b/.github/workflows/hub-ci.yml index c39f8b81..471795cc 100644 --- a/.github/workflows/hub-ci.yml +++ b/.github/workflows/hub-ci.yml @@ -25,7 +25,21 @@ jobs: fail-fast: false matrix: node: [24, 26] - command: [lint, typecheck, test, build, storybook:build, "db:validate"] + command: [lint, typecheck, test, build, storybook:build, "db:validate", "test:database"] + services: + postgres: + image: ${{ matrix.command == 'test:database' && 'postgres:18.6-bookworm' || '' }} + env: + POSTGRES_USER: replica + POSTGRES_PASSWORD: replica + POSTGRES_DB: postgres + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -h 127.0.0.1 -U replica -d postgres" + --health-interval 5s + --health-timeout 5s + --health-retries 10 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 @@ -35,6 +49,8 @@ jobs: cache-dependency-path: hub/package-lock.json - run: npm ci --engine-strict - run: npm run ${{ matrix.command }} + env: + TEST_DATABASE_URL: ${{ matrix.command == 'test:database' && 'postgresql://replica:replica@127.0.0.1:5432/postgres' || '' }} integration: name: Controls and extension tests${{ matrix.node == 26 && ' (Node 26)' || '' }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b4ef4de3..e6785eba 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -107,6 +107,8 @@ npm run build npm run storybook:build ``` +Run `npm run test:database` with the [Hub development database](hub/docs/development.md) for server cases that save records. It prepares separate databases per Vitest worker; ordinary `npm test` remains database-free. + Useful focused commands include: | Goal | Command | diff --git a/docs/testing-and-review.md b/docs/testing-and-review.md index 365784a5..ee382016 100644 --- a/docs/testing-and-review.md +++ b/docs/testing-and-review.md @@ -58,7 +58,7 @@ The [shared selection workflow](../.github/workflows/ci-changes.yml) calls [the | Root contributor documents, shared and Hub guides, skills, and documentation assets | Neither | | Workflow changes, selector changes, and unclassified paths | Both | -The `Frontend tests` and `Replica action tests` result jobs always run, so skipping an entire workflow cannot leave these checks pending. They accept skipped work only when the selector succeeded and said it was unnecessary. Hub's result covers lint, types, Vitest, the frontend/API build, Storybook, and Controls/extension checks on every supported Node major. A failed selector, invalid output, or failed selected check makes the result fail. The new Hub has no production-image, database, or release jobs; branch protection must not require removed jobs. +The `Frontend tests` and `Replica action tests` result jobs always run, so skipping an entire workflow cannot leave these checks pending. They accept skipped work only when the selector succeeded and said it was unnecessary. Hub's result covers lint, types, Vitest unit and worker-isolated database tests, the frontend/API build, Storybook, and Controls/extension checks on every supported Node major. A failed selector, invalid output, or failed selected check makes the result fail. The new Hub has no production-image or release jobs; branch protection must not require removed jobs. When adding a shared dependency or a new project, update the selector and its behavioral tests together. Unknown paths deliberately run both suites until their dependencies are known. Install tooling with `npm ci --prefix scripts --ignore-scripts --engine-strict`, then run `node --test scripts/select-ci-suites.test.mjs`, `npm run typecheck --prefix scripts`, `npm test --prefix scripts`, and `node scripts/check-ci-workflows/index.ts` from the repository root. Keep the TypeScript checks in function directories with colocated Vitest tests. The workflow checker reads the result-check scripts and tests them with success, failure, cancellation, and missing outputs. These checks run before CI selects suites. Check changed workflows with `actionlint` when available. Actual job skipping and check reporting still need verification on GitHub after publishing. diff --git a/hub/.devcontainer/compose.yaml b/hub/.devcontainer/compose.yaml index 6c310f84..f95cc194 100644 --- a/hub/.devcontainer/compose.yaml +++ b/hub/.devcontainer/compose.yaml @@ -10,6 +10,7 @@ services: - ../..:/workspaces/replica:cached environment: DATABASE_URL: postgresql://replica:replica@db:5432/replica + TEST_DATABASE_URL: postgresql://replica:replica@db:5432/postgres depends_on: db: condition: service_healthy diff --git a/hub/.env.example b/hub/.env.example index 959f3868..aaa77d99 100644 --- a/hub/.env.example +++ b/hub/.env.example @@ -3,3 +3,6 @@ PORT=3001 # Local development only. The devcontainer supplies its own database hostname. DATABASE_URL=postgresql://replica:replica@127.0.0.1:5432/replica + +# Only the database test project requires this URL and CREATEDB permission. +TEST_DATABASE_URL=postgresql://replica:replica@127.0.0.1:5432/postgres diff --git a/hub/AGENTS.md b/hub/AGENTS.md index 457239ab..bef48a11 100644 --- a/hub/AGENTS.md +++ b/hub/AGENTS.md @@ -11,6 +11,8 @@ Hub is being rebuilt in TypeScript. The app starts locally, but its product feat Run Node commands from `hub/`, locally or inside the devcontainer. PostgreSQL and Prisma are configured without product models. Agree on the design before adding models, sign-in, deployment, or product API routes. +`npm test` is database-free. Use `npm run test:database` for `.db.test.ts` server cases, with the worker-specific databases and shared reset lifecycle described in [testing](docs/testing-and-operations.md). + Keep the top-level `HubThemeProvider` around Hub and Storybook content. It synchronizes the resolved light/dark mode to `` so portals inherit Brand and `--hub-*` tokens. Do not add a fixed light theme to an overlay. Preserve the shared canvas and Controls source paths under `app/javascript/`; the prototype extension and build action consume them independently of the Hub API. diff --git a/hub/app/javascript/application/prototype-canvas/stories.test.tsx b/hub/app/javascript/application/prototype-canvas/stories.test.tsx index 7240d3c7..9503829e 100644 --- a/hub/app/javascript/application/prototype-canvas/stories.test.tsx +++ b/hub/app/javascript/application/prototype-canvas/stories.test.tsx @@ -1,14 +1,9 @@ import { fireEvent, render, screen, waitFor, within } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { HubThemeProvider } from "@hooks/use-hub-theme" import story from "./stories" -afterEach(() => { - vi.unstubAllGlobals() - vi.restoreAllMocks() -}) - describe("PublicationDemo()", () => { it("shows the updated comments in the complete canvas sidebar without contacting Hub", async () => { const externalFetch = vi.fn(() => Promise.reject(new Error("Unexpected external request."))) diff --git a/hub/app/javascript/application/prototype-preview-bridge/controls.test.ts b/hub/app/javascript/application/prototype-preview-bridge/controls.test.ts index 38116ce6..37350d8b 100644 --- a/hub/app/javascript/application/prototype-preview-bridge/controls.test.ts +++ b/hub/app/javascript/application/prototype-preview-bridge/controls.test.ts @@ -1,5 +1,5 @@ // @vitest-environment jsdom -import { afterEach, expect, it, vi } from "vitest" +import { beforeEach, expect, it, onTestFinished, vi } from "vitest" import { getControlsRuntime } from "@utils/prototype-controls/controls.mjs" import { installControlsBridge } from "./controls" @@ -23,13 +23,12 @@ const declaration = { ], } as const -afterEach(() => { - dispose?.() - dispose = undefined - delete globals[runtimeKey] - vi.restoreAllMocks() - vi.unstubAllGlobals() - window.history.replaceState(null, "", "/") +beforeEach(() => { + onTestFinished(() => { + dispose?.() + dispose = undefined + delete globals[runtimeKey] + }) }) function message(data: object, messageOrigin = origin, source: MessageEventSource = window.parent) { diff --git a/hub/app/javascript/application/prototype-preview-bridge/index.test.ts b/hub/app/javascript/application/prototype-preview-bridge/index.test.ts index 96727b91..15a5ecbe 100644 --- a/hub/app/javascript/application/prototype-preview-bridge/index.test.ts +++ b/hub/app/javascript/application/prototype-preview-bridge/index.test.ts @@ -1,11 +1,17 @@ // @vitest-environment jsdom -import { afterEach, beforeEach, expect, it, vi } from "vitest" +import { beforeEach, expect, it, onTestFinished, vi } from "vitest" import { REVIEW_ANCHOR_SCHEMA } from "@utils/review-anchor/schemas" import { installBridge } from "." -beforeEach(() => installBridge("http://127.0.0.1:1234")) +beforeEach(() => { + onTestFinished(() => { + const bridgeWindow = window as Window & { [installed]?: () => void } + bridgeWindow[installed]?.() + }) + installBridge("http://127.0.0.1:1234") +}) const TEXT_ENCODER = new TextEncoder() const installed = Symbol.for("replica.canvas.bridge.v1") @@ -18,16 +24,6 @@ function boundedText(maximumBytes: number, character: string, remainingBytes: nu } } -afterEach(() => { - const bridgeWindow = window as Window & { [installed]?: () => void } - bridgeWindow[installed]?.() - window.getSelection()?.removeAllRanges() - document.body.innerHTML = "" - document.body.style.position = "" - vi.restoreAllMocks() - vi.unstubAllGlobals() -}) - it.each(["static", "fixed", "sticky"])( "updates a %s-ancestor pin on scroll instead of waiting for polling", async (position) => { diff --git a/hub/app/javascript/components/copyable-text-input/index.test.tsx b/hub/app/javascript/components/copyable-text-input/index.test.tsx index c8c95ab3..4922ce99 100644 --- a/hub/app/javascript/components/copyable-text-input/index.test.tsx +++ b/hub/app/javascript/components/copyable-text-input/index.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { CopyableTextInput } from "." import styles from "./styles.css?inline" @@ -12,8 +12,6 @@ const props = { copyErrorLabel: "Copy failed", } -afterEach(() => vi.unstubAllGlobals()) - describe("CopyableTextInput()", () => { it("fades long URLs into an opaque themed button without changing compact controls", () => { const { rerender } = render() diff --git a/hub/app/javascript/components/prototype-browser-bar/index.test.tsx b/hub/app/javascript/components/prototype-browser-bar/index.test.tsx index ed137e2a..ab77dd59 100644 --- a/hub/app/javascript/components/prototype-browser-bar/index.test.tsx +++ b/hub/app/javascript/components/prototype-browser-bar/index.test.tsx @@ -1,11 +1,9 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react" -import { afterEach, expect, it, vi } from "vitest" +import { expect, it, vi } from "vitest" import { BrowserBar } from "." import browserStyles from "./styles.css?inline" -afterEach(() => vi.unstubAllGlobals()) - const address = "http://localhost:5173/#/cli/cli/tree/main" function renderBar(onNavigate = vi.fn(), appearance: "light" | "dark" = "light") { diff --git a/hub/app/javascript/components/prototype-controls-fields/stories.test.tsx b/hub/app/javascript/components/prototype-controls-fields/stories.test.tsx index 4e6310ad..df7b8e6c 100644 --- a/hub/app/javascript/components/prototype-controls-fields/stories.test.tsx +++ b/hub/app/javascript/components/prototype-controls-fields/stories.test.tsx @@ -1,6 +1,6 @@ import { ThemeProvider } from "@primer/react-brand" import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import panelStyles from "../prototype-controls-panel/styles.css?inline" import meta, { @@ -13,8 +13,6 @@ import meta, { ScopeDividersPanel, } from "./stories" -afterEach(() => vi.useRealTimers()) - describe("Controls examples", () => { it("toggles an ungrouped first field without changing group folds or scope separators", async () => { const { container } = render( diff --git a/hub/app/javascript/components/repository-selector/index.test.tsx b/hub/app/javascript/components/repository-selector/index.test.tsx index 57303871..c46dc01a 100644 --- a/hub/app/javascript/components/repository-selector/index.test.tsx +++ b/hub/app/javascript/components/repository-selector/index.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render, screen, waitFor } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { RepositorySelector } from "." @@ -25,11 +25,6 @@ describe("RepositorySelector()", () => { ) }) - afterEach(() => { - vi.restoreAllMocks() - vi.unstubAllGlobals() - }) - it("shows the selected repository and keeps its menu links", async () => { render() diff --git a/hub/app/javascript/components/repository-user-menu/index.test.tsx b/hub/app/javascript/components/repository-user-menu/index.test.tsx index dd03e081..5450dede 100644 --- a/hub/app/javascript/components/repository-user-menu/index.test.tsx +++ b/hub/app/javascript/components/repository-user-menu/index.test.tsx @@ -1,7 +1,7 @@ import type { PropsWithChildren } from "react" import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { HubThemeProvider } from "@hooks/use-hub-theme" @@ -34,11 +34,6 @@ describe("RepositoryUserMenu()", () => { ) }) - afterEach(() => { - vi.restoreAllMocks() - vi.unstubAllGlobals() - }) - it("shows the account identity and opens the sign-out menu when expanded", () => { render(, { wrapper: ThemeWrapper }) diff --git a/hub/app/javascript/components/scrolling-screenshot/index.test.tsx b/hub/app/javascript/components/scrolling-screenshot/index.test.tsx index b8ec44c2..e9e81d21 100644 --- a/hub/app/javascript/components/scrolling-screenshot/index.test.tsx +++ b/hub/app/javascript/components/scrolling-screenshot/index.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { HubThemeProvider } from "@hooks/use-hub-theme" @@ -25,10 +25,6 @@ describe("ScrollingScreenshot()", () => { vi.spyOn(HTMLImageElement.prototype, "naturalWidth", "get").mockReturnValue(1440) }) - afterEach(() => { - vi.unstubAllGlobals() - }) - it("calculates the full scroll distance from the loaded image height", () => { vi.spyOn(HTMLImageElement.prototype, "naturalHeight", "get").mockReturnValue(3600) diff --git a/hub/app/javascript/features/hub-administration-workspace/index.test.tsx b/hub/app/javascript/features/hub-administration-workspace/index.test.tsx index 3de13ede..25bce1db 100644 --- a/hub/app/javascript/features/hub-administration-workspace/index.test.tsx +++ b/hub/app/javascript/features/hub-administration-workspace/index.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { hubAdministrationWorkspaceData } from "@/storybook/fixtures/hub-administration" @@ -16,7 +16,6 @@ describe("HubAdministrationWorkspace()", () => { }, ) }) - afterEach(() => vi.unstubAllGlobals()) it.each([false, true])( "uses repository navigation and lower Hub links without scope headings, collapsed %s", diff --git a/hub/app/javascript/features/install-replica-preview-bridge/index.test.ts b/hub/app/javascript/features/install-replica-preview-bridge/index.test.ts index 3ea23c81..5878809c 100644 --- a/hub/app/javascript/features/install-replica-preview-bridge/index.test.ts +++ b/hub/app/javascript/features/install-replica-preview-bridge/index.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, onTestFinished, vi } from "vitest" import { PREVIEW_BRIDGE_MESSAGE_SCHEMA } from "@utils/preview-bridge/schemas" @@ -8,7 +8,7 @@ const harness = createPreviewBridgeTestHarness() const { anchor, areaAnchor, bridgeWindow, dispatchMessage, message, postedMessages, renderFrames } = harness -afterEach(harness.close) +beforeEach(() => onTestFinished(harness.close)) describe("installReplicaPreviewBridge()", () => { it.each(["static", "fixed", "sticky"])( diff --git a/hub/app/javascript/features/minimized-review-toolbar/index.test.tsx b/hub/app/javascript/features/minimized-review-toolbar/index.test.tsx index 8cf63a7d..f3b32621 100644 --- a/hub/app/javascript/features/minimized-review-toolbar/index.test.tsx +++ b/hub/app/javascript/features/minimized-review-toolbar/index.test.tsx @@ -7,7 +7,7 @@ import { waitFor, within, } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { reviewCommentsCopy } from "@utils/review-comment/en" @@ -116,8 +116,6 @@ beforeEach(() => { }) }) -afterEach(() => vi.unstubAllGlobals()) - describe("MinimizedReviewToolbar()", () => { it("keeps the same draggable handle and maximize action without any review controls for Browse", () => { toolbarWidth = 80 diff --git a/hub/app/javascript/features/prototype-canvas-share-links/index.test.tsx b/hub/app/javascript/features/prototype-canvas-share-links/index.test.tsx index 3f8b729d..80fd765e 100644 --- a/hub/app/javascript/features/prototype-canvas-share-links/index.test.tsx +++ b/hub/app/javascript/features/prototype-canvas-share-links/index.test.tsx @@ -1,18 +1,13 @@ import { act, fireEvent, render, screen, waitFor } from "@testing-library/react" import { ThemeProvider } from "@primer/react-brand" import { createRef } from "react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { importedPrototype } from "@utils/resolve-prototype-publication/fixtures" import { ShareLinksPage } from "." import { shareCopy } from "@hooks/use-canvas-share-links/en" import type { SharingState } from "@hooks/use-canvas-share-links/types" -afterEach(() => { - vi.restoreAllMocks() - vi.unstubAllGlobals() -}) - const state: SharingState = { open: true, context: { diff --git a/hub/app/javascript/features/prototype-workspace-header/index.test.tsx b/hub/app/javascript/features/prototype-workspace-header/index.test.tsx index dfc33429..360a8ca0 100644 --- a/hub/app/javascript/features/prototype-workspace-header/index.test.tsx +++ b/hub/app/javascript/features/prototype-workspace-header/index.test.tsx @@ -1,5 +1,5 @@ import { createEvent, fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { HubThemeProvider } from "@hooks/use-hub-theme" @@ -16,7 +16,6 @@ beforeEach(() => { }, ) }) -afterEach(() => vi.unstubAllGlobals()) describe("PrototypeWorkspaceHeader()", () => { it("keeps author identity in the normal prototype header", () => { diff --git a/hub/app/javascript/features/prototype-workspace-header/review-comment-control/index.test.tsx b/hub/app/javascript/features/prototype-workspace-header/review-comment-control/index.test.tsx index 888ac9c0..3d953f8e 100644 --- a/hub/app/javascript/features/prototype-workspace-header/review-comment-control/index.test.tsx +++ b/hub/app/javascript/features/prototype-workspace-header/review-comment-control/index.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { reviewCommentsCopy } from "@utils/review-comment/en" @@ -21,7 +21,6 @@ beforeEach(() => { }, ) }) -afterEach(() => vi.unstubAllGlobals()) describe("ReviewCommentControl()", () => { it("aligns the comment icon as a Brand leading visual and describes the menu trigger", () => { diff --git a/hub/app/javascript/features/repository-navigation/index.test.tsx b/hub/app/javascript/features/repository-navigation/index.test.tsx index e2cb22bf..ce7894c7 100644 --- a/hub/app/javascript/features/repository-navigation/index.test.tsx +++ b/hub/app/javascript/features/repository-navigation/index.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { hubAdministrationWorkspaceData } from "@/storybook/fixtures/hub-administration" import { prototypeWorkspaceData } from "@/storybook/fixtures/prototype-workspace" @@ -51,8 +51,6 @@ describe("RepositoryNavigation()", () => { ) }) - afterEach(() => vi.unstubAllGlobals()) - it.each([false, true])( "groups one audit with Hub links above collapse and account, collapsed %s", (collapsed) => { diff --git a/hub/app/javascript/features/review-comment-filter-controls/index.test.tsx b/hub/app/javascript/features/review-comment-filter-controls/index.test.tsx index da5dea33..59e1e4d2 100644 --- a/hub/app/javascript/features/review-comment-filter-controls/index.test.tsx +++ b/hub/app/javascript/features/review-comment-filter-controls/index.test.tsx @@ -1,5 +1,5 @@ import { act, createEvent, fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { defaultReviewFilters } from "@/storybook/fixtures/review/comments" import { reviewCommentsCopy } from "@utils/review-comment/en" @@ -16,7 +16,6 @@ beforeEach(() => { }, ) }) -afterEach(() => vi.unstubAllGlobals()) describe("ReviewCommentFilterControls()", () => { it("shows the leading search icon in the default search field", () => { diff --git a/hub/app/javascript/features/review-comment-reactions/index.test.tsx b/hub/app/javascript/features/review-comment-reactions/index.test.tsx index b778c1e2..851491e0 100644 --- a/hub/app/javascript/features/review-comment-reactions/index.test.tsx +++ b/hub/app/javascript/features/review-comment-reactions/index.test.tsx @@ -1,5 +1,5 @@ import { act, fireEvent, render, screen, waitFor } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import type { ReviewReply } from "@utils/review-comment/types" @@ -47,7 +47,6 @@ beforeEach(() => { }, ) }) -afterEach(() => vi.unstubAllGlobals()) describe("ReviewCommentReactions()", () => { it("shows an add immediately without replacing the picker and rolls back a rejected request", async () => { diff --git a/hub/app/javascript/features/review-relative-time/index.test.tsx b/hub/app/javascript/features/review-relative-time/index.test.tsx index 310f4344..3ecd0bdc 100644 --- a/hub/app/javascript/features/review-relative-time/index.test.tsx +++ b/hub/app/javascript/features/review-relative-time/index.test.tsx @@ -1,11 +1,9 @@ import { act, render, screen } from "@testing-library/react" import { renderToString } from "react-dom/server" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { ReviewRelativeTime } from "." -afterEach(() => vi.useRealTimers()) - describe("ReviewRelativeTime()", () => { it("can omit the native title while keeping the full accessible date", () => { render() diff --git a/hub/app/javascript/features/review-thread-conversation/index.test.tsx b/hub/app/javascript/features/review-thread-conversation/index.test.tsx index d85faf75..0028db85 100644 --- a/hub/app/javascript/features/review-thread-conversation/index.test.tsx +++ b/hub/app/javascript/features/review-thread-conversation/index.test.tsx @@ -1,5 +1,5 @@ import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import type { ReviewCommentsCopy, ReviewThread } from "@utils/review-comment/types" import { ReviewCommentMarkers } from "@features/review-comment-markers" @@ -17,10 +17,6 @@ beforeEach(() => { }, ) }) -afterEach(() => { - vi.restoreAllMocks() - vi.unstubAllGlobals() -}) const COPY: ReviewCommentsCopy = { heading: "Comments", diff --git a/hub/app/javascript/features/review-thread-message/index.test.tsx b/hub/app/javascript/features/review-thread-message/index.test.tsx index da0eef0a..d85f2a9f 100644 --- a/hub/app/javascript/features/review-thread-message/index.test.tsx +++ b/hub/app/javascript/features/review-thread-message/index.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { octocatRootComment } from "@/storybook/fixtures/review/comments" import { reviewCommentsCopy } from "@utils/review-comment/en" @@ -48,7 +48,6 @@ beforeEach(() => { }, ) }) -afterEach(() => vi.unstubAllGlobals()) describe("ReviewThreadMessage()", () => { it("keeps the author, compact timestamp, and edited status on one byline", () => { diff --git a/hub/app/javascript/hooks/use-canvas-commenting-recovery/index.test.tsx b/hub/app/javascript/hooks/use-canvas-commenting-recovery/index.test.tsx index c82cfb23..70c484fa 100644 --- a/hub/app/javascript/hooks/use-canvas-commenting-recovery/index.test.tsx +++ b/hub/app/javascript/hooks/use-canvas-commenting-recovery/index.test.tsx @@ -1,12 +1,7 @@ import { act, renderHook } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { useCommentingRecovery } from "." -afterEach(() => { - vi.useRealTimers() - vi.unstubAllGlobals() -}) - describe("useCommentingRecovery()", () => { const initial = { identity: "hub/repo/branch", diff --git a/hub/app/javascript/hooks/use-canvas-prototype-state/index.test.tsx b/hub/app/javascript/hooks/use-canvas-prototype-state/index.test.tsx index 4b4fbd0e..d617bdda 100644 --- a/hub/app/javascript/hooks/use-canvas-prototype-state/index.test.tsx +++ b/hub/app/javascript/hooks/use-canvas-prototype-state/index.test.tsx @@ -1,5 +1,5 @@ import { act, renderHook } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { usePrototypeState } from "." @@ -12,11 +12,6 @@ beforeEach(() => { vi.stubGlobal("fetch", fetchMock) }) -afterEach(() => { - vi.useRealTimers() - vi.unstubAllGlobals() -}) - describe("usePrototypeState()", () => { it("discards an older response that arrives after a newer checkout", async () => { let completeOld: ((value: unknown) => void) | undefined diff --git a/hub/app/javascript/hooks/use-canvas-share-links/index.test.tsx b/hub/app/javascript/hooks/use-canvas-share-links/index.test.tsx index 18c1c81c..06d6f9a3 100644 --- a/hub/app/javascript/hooks/use-canvas-share-links/index.test.tsx +++ b/hub/app/javascript/hooks/use-canvas-share-links/index.test.tsx @@ -1,6 +1,6 @@ import { act, renderHook, waitFor } from "@testing-library/react" import { useLayoutEffect } from "react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { importedPrototype, @@ -33,11 +33,6 @@ const link = { createdBy: { login: "octocat", name: null }, } -afterEach(() => { - vi.unstubAllGlobals() - vi.restoreAllMocks() -}) - type HookProps = { hubStatus: HubStatus token?: string diff --git a/hub/app/javascript/hooks/use-emoji-burst/index.test.tsx b/hub/app/javascript/hooks/use-emoji-burst/index.test.tsx index 02831ddb..1054ec51 100644 --- a/hub/app/javascript/hooks/use-emoji-burst/index.test.tsx +++ b/hub/app/javascript/hooks/use-emoji-burst/index.test.tsx @@ -1,13 +1,8 @@ import { act, render } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { useEmojiBurst } from "." -afterEach(() => { - vi.restoreAllMocks() - vi.unstubAllGlobals() -}) - describe("useEmojiBurst()", () => { let burst: ReturnType["burst"] diff --git a/hub/app/javascript/hooks/use-hub-theme/index.test.tsx b/hub/app/javascript/hooks/use-hub-theme/index.test.tsx index 0e6f522d..d105011b 100644 --- a/hub/app/javascript/hooks/use-hub-theme/index.test.tsx +++ b/hub/app/javascript/hooks/use-hub-theme/index.test.tsx @@ -1,7 +1,7 @@ import { act } from "react" import { createPortal } from "react-dom" import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { HubThemeProvider, useHubTheme } from "." @@ -19,8 +19,6 @@ function ThemeProbe() { } describe("useHubTheme()", () => { - afterEach(() => vi.restoreAllMocks()) - it("follows the system until a reviewer selects a color mode", () => { const listeners = new Set<(event: MediaQueryListEvent) => void>() const preference = Object.assign(window.matchMedia("(prefers-color-scheme: dark)"), { diff --git a/hub/app/javascript/hooks/use-notification-inbox/index.test.tsx b/hub/app/javascript/hooks/use-notification-inbox/index.test.tsx index f0f58c5c..1fe8bdbd 100644 --- a/hub/app/javascript/hooks/use-notification-inbox/index.test.tsx +++ b/hub/app/javascript/hooks/use-notification-inbox/index.test.tsx @@ -1,5 +1,5 @@ import { act, renderHook, waitFor } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import type { InboxNotification } from "@utils/notification/types" @@ -21,8 +21,6 @@ const notification: InboxNotification = { } const page = { notifications: [notification], nextBefore: null, unreadCount: 12 } -afterEach(() => vi.unstubAllGlobals()) - describe("useNotificationInbox()", () => { it("keeps cached unread notifications visible on reopen and adds new arrivals first without duplicates", async () => { let finish!: (response: Response) => void diff --git a/hub/app/javascript/hooks/use-prototype-review/index.test.tsx b/hub/app/javascript/hooks/use-prototype-review/index.test.tsx index f833450d..239cb1d2 100644 --- a/hub/app/javascript/hooks/use-prototype-review/index.test.tsx +++ b/hub/app/javascript/hooks/use-prototype-review/index.test.tsx @@ -1,5 +1,5 @@ import { act, renderHook, waitFor } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { resolvedAnchor } from "@/storybook/fixtures/review/anchors" import { octocatRootComment } from "@/storybook/fixtures/review/comments" @@ -22,12 +22,6 @@ const createdComment = { anchor: elementAnchor, } -afterEach(() => { - document.querySelectorAll("iframe").forEach((frame) => frame.remove()) - vi.restoreAllMocks() - vi.unstubAllGlobals() -}) - async function openReview() { const fetchMock = vi .fn() diff --git a/hub/app/javascript/hooks/use-review-comments/index.test.ts b/hub/app/javascript/hooks/use-review-comments/index.test.ts index 0012c240..2c4428fd 100644 --- a/hub/app/javascript/hooks/use-review-comments/index.test.ts +++ b/hub/app/javascript/hooks/use-review-comments/index.test.ts @@ -1,5 +1,5 @@ import { act, renderHook, waitFor } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import type { ReviewComment, ReviewRootComment } from "@utils/review-comment/types" @@ -54,10 +54,6 @@ function deferredResponse() { return { promise, resolve } } -afterEach(() => { - vi.unstubAllGlobals() -}) - describe("useReviewComments()", () => { it("marks a thread unread through its read endpoint and keeps the returned viewer state", async () => { const unread = { ...COMMENT_1, viewerLastSeenActivityVersion: null } diff --git a/hub/app/javascript/pages/home-index-page/ascii-gradient/index.test.tsx b/hub/app/javascript/pages/home-index-page/ascii-gradient/index.test.tsx index 0215a454..f57d1622 100644 --- a/hub/app/javascript/pages/home-index-page/ascii-gradient/index.test.tsx +++ b/hub/app/javascript/pages/home-index-page/ascii-gradient/index.test.tsx @@ -1,5 +1,5 @@ import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { AsciiGradient } from "." @@ -105,8 +105,6 @@ describe("AsciiGradient()", () => { vi.spyOn(window, "cancelAnimationFrame").mockImplementation(cancelFrame) }) - afterEach(() => vi.restoreAllMocks()) - function tick(elapsed = 16) { time += elapsed const callbacks = [...frames.values()] diff --git a/hub/app/javascript/pages/home-index-page/index.test.tsx b/hub/app/javascript/pages/home-index-page/index.test.tsx index fea4d763..d28c21d3 100644 --- a/hub/app/javascript/pages/home-index-page/index.test.tsx +++ b/hub/app/javascript/pages/home-index-page/index.test.tsx @@ -1,5 +1,5 @@ import { act, fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, it, vi } from "vitest" +import { beforeEach, describe, it, vi } from "vitest" import { HomeIndexPage } from "." import type { HomeIndexPageData } from "./types" @@ -31,11 +31,6 @@ describe("HomeIndexPage()", () => { vi.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue(null) }) - afterEach(() => { - vi.useRealTimers() - vi.restoreAllMocks() - }) - it("rotates the eyebrow every six seconds without changing the heading", async () => { vi.useFakeTimers({ toFake: ["setInterval", "clearInterval"] }) renderHome() diff --git a/hub/app/javascript/pages/new-prototype-page/index.test.tsx b/hub/app/javascript/pages/new-prototype-page/index.test.tsx index 79c5bdb1..1d2f7786 100644 --- a/hub/app/javascript/pages/new-prototype-page/index.test.tsx +++ b/hub/app/javascript/pages/new-prototype-page/index.test.tsx @@ -1,5 +1,5 @@ import { act, fireEvent, render, screen } from "@testing-library/react" -import { afterEach, describe, it, vi } from "vitest" +import { describe, it, vi } from "vitest" import { NewPrototypePage } from "." import type { NewPrototypePageData } from "./types" @@ -74,11 +74,6 @@ function submitBrief(brief: string) { fireEvent.click(screen.getByRole("button", { name: PAGE_DATA.copy.submitLabel })) } -afterEach(() => { - vi.useRealTimers() - vi.restoreAllMocks() -}) - describe("NewPrototypePage()", () => { it("copies a local brief and saved details without launching Copilot", async () => { const writeText = vi.fn().mockResolvedValue(undefined) diff --git a/hub/app/javascript/pages/prototypes-show-page/index.test.tsx b/hub/app/javascript/pages/prototypes-show-page/index.test.tsx index 03e05f79..3f410e12 100644 --- a/hub/app/javascript/pages/prototypes-show-page/index.test.tsx +++ b/hub/app/javascript/pages/prototypes-show-page/index.test.tsx @@ -1,6 +1,6 @@ import { useRef, useState } from "react" import { act, fireEvent, render, screen, within } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { HubThemeProvider, useHubTheme } from "@hooks/use-hub-theme" import { usePrototypeReview } from "@hooks/use-prototype-review" @@ -536,6 +536,8 @@ describe("PrototypesShowPage()", () => { }) beforeEach(() => { + BRIDGE.selectedAnchor = null + currentReviewComments = REVIEW_COMMENTS window.history.replaceState({}, "", "/repositories/2001/prototypes/1") BRIDGE.route = { pathname: "/", search: "", hash: "" } vi.stubGlobal( @@ -548,13 +550,6 @@ describe("PrototypesShowPage()", () => { ) }) - afterEach(() => { - vi.useRealTimers() - vi.unstubAllGlobals() - BRIDGE.selectedAnchor = null - currentReviewComments = REVIEW_COMMENTS - }) - it("opens over the review page, dismisses, and reopens from the title", () => { render( diff --git a/hub/app/javascript/pages/replica-browse-page/index.test.tsx b/hub/app/javascript/pages/replica-browse-page/index.test.tsx index 25ce4f6e..9e84233f 100644 --- a/hub/app/javascript/pages/replica-browse-page/index.test.tsx +++ b/hub/app/javascript/pages/replica-browse-page/index.test.tsx @@ -1,6 +1,6 @@ import { createRef } from "react" import { act, fireEvent, render, screen, within } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { HubThemeProvider } from "@hooks/use-hub-theme" import { localReplicaBrowseData, replicaBrowseData } from "@/storybook/fixtures/replica-browse" @@ -21,8 +21,6 @@ function renderPage(overrides: Partial = {}) { ) } -afterEach(() => vi.unstubAllGlobals()) - describe("ReplicaBrowsePage()", () => { it("matches the browse payload without prototype-only metadata", () => { expect(replicaBrowseData).not.toHaveProperty("author") diff --git a/hub/app/javascript/pages/security-audit-page/index.test.tsx b/hub/app/javascript/pages/security-audit-page/index.test.tsx index 769ab811..dd1616cd 100644 --- a/hub/app/javascript/pages/security-audit-page/index.test.tsx +++ b/hub/app/javascript/pages/security-audit-page/index.test.tsx @@ -1,6 +1,6 @@ import { ThemeProvider } from "@primer/react-brand" import { fireEvent, render, screen } from "@testing-library/react" -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, vi } from "vitest" import { hubAdministrationWorkspaceData } from "@/storybook/fixtures/hub-administration" @@ -46,7 +46,6 @@ describe("SecurityAuditPage()", () => { }, ) }) - afterEach(() => vi.unstubAllGlobals()) it("links only server-authorized scopes without carrying a pagination cursor across scopes", () => { render( diff --git a/hub/app/javascript/storybook/inbox-prototype/index.test.tsx b/hub/app/javascript/storybook/inbox-prototype/index.test.tsx index 18832c86..27dbb19e 100644 --- a/hub/app/javascript/storybook/inbox-prototype/index.test.tsx +++ b/hub/app/javascript/storybook/inbox-prototype/index.test.tsx @@ -1,13 +1,8 @@ import { act, fireEvent, render, screen, within } from "@testing-library/react" -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { InboxPrototype } from "." -afterEach(() => { - vi.unstubAllGlobals() - vi.useRealTimers() -}) - describe("InboxPrototype()", () => { it.each([false, true])( "places the hover panel 8 pixels from the actual Inbox button with collapsed=%s", diff --git a/hub/app/javascript/test/setup.test.ts b/hub/app/javascript/test/setup.test.ts new file mode 100644 index 00000000..19523191 --- /dev/null +++ b/hub/app/javascript/test/setup.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest" + +const initialLocation = window.location.href +const initialHistoryState: unknown = window.history.state + +describe("frontend test setup", () => { + it.each(["first case", "second case"])( + "gives the %s clean storage, navigation, selection and document body", + () => { + expect(window.localStorage.length).toBe(0) + expect(window.sessionStorage.length).toBe(0) + expect(window.location.href).toBe(initialLocation) + expect(window.history.state).toEqual(initialHistoryState) + expect(window.getSelection()?.rangeCount).toBe(0) + expect(document.body.childNodes.length).toBe(0) + expect(document.body.hasAttribute("style")).toBe(false) + + window.localStorage.setItem("replica-setup-test", "contaminated") + window.sessionStorage.setItem("replica-setup-test", "contaminated") + window.history.replaceState( + { replicaSetupTest: "contaminated" }, + "", + "/setup-contamination?dirty=1#selection", + ) + const leftover = document.createElement("p") + leftover.textContent = "Selected content from another case" + document.body.append(leftover) + document.body.style.overflow = "hidden" + const range = document.createRange() + range.selectNodeContents(leftover) + window.getSelection()?.addRange(range) + }, + ) +}) diff --git a/hub/app/javascript/test/setup.ts b/hub/app/javascript/test/setup.ts index 9c17ae47..b3d45af4 100644 --- a/hub/app/javascript/test/setup.ts +++ b/hub/app/javascript/test/setup.ts @@ -1,5 +1,6 @@ import "@testing-library/jest-dom/vitest" -import { afterEach, vi } from "vitest" +import { cleanup } from "@testing-library/react" +import { beforeEach } from "vitest" globalThis.IS_REACT_ACT_ENVIRONMENT = true @@ -69,6 +70,12 @@ Object.defineProperty(HTMLDialogElement.prototype, "close", { }, }) -afterEach(() => { - vi.useRealTimers() +beforeEach(() => { + cleanup() + window.localStorage.clear() + window.sessionStorage.clear() + window.history.replaceState(null, "", "/") + window.getSelection()?.removeAllRanges() + document.body.replaceChildren() + document.body.removeAttribute("style") }) diff --git a/hub/app/javascript/utils/install-prototype-controls-url/index.test.ts b/hub/app/javascript/utils/install-prototype-controls-url/index.test.ts index cc79a854..34c57a2e 100644 --- a/hub/app/javascript/utils/install-prototype-controls-url/index.test.ts +++ b/hub/app/javascript/utils/install-prototype-controls-url/index.test.ts @@ -1,16 +1,15 @@ // @vitest-environment jsdom -import { afterEach, describe, expect, it, vi } from "vitest" +import { beforeEach, describe, expect, it, onTestFinished, vi } from "vitest" import { installControlsUrl } from "." let dispose: (() => void) | undefined -afterEach(() => { - dispose?.() - dispose = undefined - vi.restoreAllMocks() - vi.unstubAllGlobals() - window.history.replaceState(null, "", "/") +beforeEach(() => { + onTestFinished(() => { + dispose?.() + dispose = undefined + }) }) describe("installControlsUrl()", () => { diff --git a/hub/app/javascript/utils/prototype-canvas-request/index.test.ts b/hub/app/javascript/utils/prototype-canvas-request/index.test.ts index aea3738f..8a7252f9 100644 --- a/hub/app/javascript/utils/prototype-canvas-request/index.test.ts +++ b/hub/app/javascript/utils/prototype-canvas-request/index.test.ts @@ -1,9 +1,7 @@ -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import { request } from "." -afterEach(() => vi.unstubAllGlobals()) - describe("request()", () => { it("sends the canvas token and returns a successful response", async () => { const fetchMock = vi.fn().mockResolvedValue({ diff --git a/hub/docs/coding-conventions.md b/hub/docs/coding-conventions.md index be0536ad..8ae64a40 100644 --- a/hub/docs/coding-conventions.md +++ b/hub/docs/coding-conventions.md @@ -32,3 +32,5 @@ Preserve the existing `app/javascript/{application,pages,features,components,hoo ## Tests Keep tests beside their TypeScript code. Use `describe("functionName()")` and plain-English cases. Keep fixtures and mutable state inside each case; use the global Vitest setup and configuration for shared environment cleanup and automatic mock resets. + +Follow the [test lifecycle](testing-and-operations.md#clean-test-cases): arrange each case's records, act, and assert. Database-enabled server tests use `.db.test.ts` and the dedicated database project. Do not add local cleanup hooks for state already reset globally. diff --git a/hub/docs/database.md b/hub/docs/database.md index db66c912..c8050618 100644 --- a/hub/docs/database.md +++ b/hub/docs/database.md @@ -54,3 +54,5 @@ npm run db:generate `npm ci` also generates the client. Generated TypeScript lives under the ignored `server/generated/prisma/` directory and compiles with the API. Schema validation and generation do not require a running database or `DATABASE_URL`; migration and database commands require both. Generate again after changing a schema. Use `db:migrate` only with a local development database. It needs permission to create a shadow database to calculate migrations. `npm run db:deploy` applies committed migrations without creating new ones; it is not a production deployment setup. `npm run db:studio` opens Prisma's database editor and should be used only with an intended development database. + +The [database test project](testing-and-operations.md#parallel-database-tests) uses separate databases for every worker and test run. It applies the current Prisma schema directly, clears records before each case, and does not use the development database for test data. diff --git a/hub/docs/development.md b/hub/docs/development.md index 4673c877..8e690189 100644 --- a/hub/docs/development.md +++ b/hub/docs/development.md @@ -48,6 +48,8 @@ If port 5432 is already in use, stop the conflicting local service or explicitly Follow the [database guide](database.md) for schema conventions, migrations, and the shared client. +`npm test` needs no database. To run database-enabled server tests, start PostgreSQL and run `npm run test:database`. The separate `TEST_DATABASE_URL` in `.env.example` provides a connection with permission to create disposable test databases. See [parallel database tests](testing-and-operations.md#parallel-database-tests) for isolation and cleanup. + ## Build the app ```sh diff --git a/hub/docs/testing-and-operations.md b/hub/docs/testing-and-operations.md index f8e900d3..f0798048 100644 --- a/hub/docs/testing-and-operations.md +++ b/hub/docs/testing-and-operations.md @@ -2,22 +2,45 @@ The new Hub has a PostgreSQL and Prisma foundation but no product models, deployment setup, sign-in, or product APIs yet. These checks cover the new app setup and the frontend code we kept. They do not prove that the old Hub's features work. -Run commands directly from `hub/` after `npm ci`: +Run commands directly from `hub/` after `npm ci`. The database command also needs PostgreSQL and `TEST_DATABASE_URL`, as described in [parallel database tests](#parallel-database-tests): ```sh npm run lint npm run db:validate npm run typecheck npm test +npm run test:database npm run build npm run storybook:build ``` -Vitest uses a Node project for API and pure utility tests and a jsdom project for frontend behavior. A focused API check is `npm test -- --project=node server/create-app/index.test.ts`. HTTP tests listen on ephemeral loopback ports and close their servers afterward. +`npm test` selects the Node project for API and pure utility tests and the jsdom project for frontend behavior. Neither requires PostgreSQL. A focused API check is `npm test -- server/create-app/index.test.ts`. HTTP tests listen on ephemeral loopback ports and close their servers afterward. The API health check proves only that Express responds. Routes that have not been built must return 404 rather than pretending to work. -Installation generates Prisma Client without a database connection. Client lifecycle tests cover reuse across development reloads, production global behavior, and missing configuration. Database queries and migrations need the PostgreSQL service described in [Development](development.md); the ordinary Vitest suite does not connect to it. +Installation generates Prisma Client without a database connection. Client lifecycle tests cover reuse across development reloads, production global behavior, and missing configuration. + +## Clean test cases + +[The shared setup](../test/setup.ts) and Vitest configuration reset mocks, spies, stubbed globals, stubbed environment variables, fake timers, and the module cache before every case. The browser setup also clears storage, navigation state, selections, and leftover document content. React Testing Library unmounts rendered components after cases. + +Arrange each case's inputs and records inside that case, act through its public interface, and assert the result. Do not depend on records, mocks, or effects left by another case. Use `beforeEach` only for repeated setup, not to hide a case's important inputs. Avoid local `afterEach` and `afterAll` for state already reset globally. + +Global resets cannot dispose arbitrary servers, files, sockets, or event listeners. Release resources owned by a case with `try/finally` or Vitest's `onTestFinished`, registered when creating the resource. Module-cache resets affect subsequent imports, not references already imported by a test file; reset any deliberately shared fixture state before each case. + +## Parallel database tests + +Vitest runs files in parallel with its [default forked-process pool](https://vitest.dev/config/pool). The configuration makes this explicit and keeps cases within a worker sequential, including cases marked concurrent, because resetting a shared worker database while another case runs would erase its records. `--maxWorkers` controls file-level parallelism. + +Only `npm run test:database` selects the database-enabled server project. Name those tests `server/**/*.db.test.ts`; they are excluded from the ordinary Node project. Use the PostgreSQL service in [Development](development.md) and set `TEST_DATABASE_URL` from `.env.example`. The devcontainer supplies its internal service URL. The database user needs permission to create and drop databases; use a local development service, not a production connection. + +Before workers start, global setup creates a run-unique template database and applies the current Prisma schema with `prisma db push`. It clones that template for every worker slot. Setup uses Vitest's [documented `VITEST_POOL_ID`](https://vitest.dev/guide/migration/jest#envs), which stays within the configured worker limit, rather than the ever-increasing worker instance ID. Each worker receives its own `DATABASE_URL` before application modules load. + +Before every case, the database setup truncates application tables across user schemas and restarts their identities. It preserves Prisma's migration records. Tests create their own records in an arrange, act, assert pattern; there is no shared seed data. At the end of the run, global teardown drops only the databases it created, even if tests left connections open. Unique run prefixes also isolate simultaneous Vitest runs. Cleanup rejects database names outside the test namespace and verifies the actual connected database before truncating. + +The test helpers under `test/` are test infrastructure, not application repositories, integrations, utilities, or domains. Runtime code must not import them. These helpers use direct PostgreSQL access to prepare databases before Prisma Client and application code load. + +Hub CI runs this project against PostgreSQL on every supported Node major. Only that matrix command starts the database service; browser, pure unit, formatting, and build checks do not require it. ## Shared code diff --git a/hub/package-lock.json b/hub/package-lock.json index bdbd6a4b..e7a1d8e7 100644 --- a/hub/package-lock.json +++ b/hub/package-lock.json @@ -30,6 +30,7 @@ "@types/express": "5.0.3", "@types/jsdom": "30.0.0", "@types/node": "26.6.4", + "@types/pg": "8.23.1", "@types/react": "19.3.0", "@types/react-dom": "19.3.0", "@typescript/native": "npm:typescript@7.0.2", @@ -42,6 +43,7 @@ "globals": "17.13.0", "jiti": "2.7.0", "jsdom": "30.1.3", + "pg": "8.23.1", "prettier": "3.9.10", "prisma": "7.10.1", "storybook": "10.6.2", diff --git a/hub/package.json b/hub/package.json index b10f5916..57920de7 100644 --- a/hub/package.json +++ b/hub/package.json @@ -33,7 +33,8 @@ "lint": "eslint . ../.github/extensions/replica-prototype ../packages/learning && prettier --check . '../eslint.config.mjs' '../.github/extensions/replica-prototype/**/*.{mjs,mts,ts}' '../.github/extensions/replica-prototype/*.json' '../packages/learning/**/*.{mjs,mts,json}' --config .prettierrc.json", "storybook": "STORYBOOK_DISABLE_TELEMETRY=1 storybook dev --host 0.0.0.0 --port 6006 --initial-path /story/hub-design-overview--taxonomy", "storybook:build": "STORYBOOK_DISABLE_TELEMETRY=1 storybook build --output-dir storybook-static", - "test": "vitest run", + "test": "vitest run --project=node --project=browser", + "test:database": "vitest run --project=database", "typecheck": "tsc --noEmit && tsc --project ../.github/extensions/replica-prototype/tsconfig.json" }, "dependencies": { @@ -58,6 +59,7 @@ "@types/jsdom": "30.0.0", "@types/express": "5.0.3", "@types/node": "26.6.4", + "@types/pg": "8.23.1", "@types/react": "19.3.0", "@types/react-dom": "19.3.0", "@typescript/native": "npm:typescript@7.0.2", @@ -70,6 +72,7 @@ "globals": "17.13.0", "jiti": "2.7.0", "jsdom": "30.1.3", + "pg": "8.23.1", "prettier": "3.9.10", "prisma": "7.10.1", "storybook": "10.6.2", diff --git a/hub/server/integrations/prisma/index.test.ts b/hub/server/integrations/prisma/index.test.ts index 464560f9..b2d20a9b 100644 --- a/hub/server/integrations/prisma/index.test.ts +++ b/hub/server/integrations/prisma/index.test.ts @@ -1,14 +1,9 @@ -import { afterEach, describe, expect, it, vi } from "vitest" +import { describe, expect, it, vi } from "vitest" import type { PrismaClient } from "../../generated/prisma/client" vi.mock("dotenv/config", () => ({})) -afterEach(() => { - vi.unstubAllEnvs() - vi.resetModules() -}) - describe("prisma", () => { it("reuses the development client after a module reload", async () => { vi.stubEnv("NODE_ENV", "development") diff --git a/hub/server/integrations/prisma/isolation.db.test.ts b/hub/server/integrations/prisma/isolation.db.test.ts new file mode 100644 index 00000000..1de2546c --- /dev/null +++ b/hub/server/integrations/prisma/isolation.db.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, inject, it } from "vitest" + +import { resetDatabase } from "../../../test/database/reset-database" + +import { prisma } from "./index" + +describe("Prisma database isolation", () => { + it.each(["first case", "second case"])( + "starts the %s empty in its assigned worker database", + async () => { + try { + const database = inject("testDatabase") + const [connection] = await prisma.$queryRaw<{ name: string }[]>` + SELECT current_database() AS name + ` + expect(connection?.name).toBe(`${database.prefix}_${process.env.VITEST_POOL_ID}`) + + await prisma.$executeRaw` + CREATE TABLE IF NOT EXISTS public.test_worker_isolation ( + id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + unique_key text NOT NULL UNIQUE + ) + ` + const [before] = await prisma.$queryRaw<{ count: bigint }[]>` + SELECT count(*) FROM public.test_worker_isolation + ` + expect(before?.count).toBe(0n) + + await prisma.$executeRaw` + INSERT INTO public.test_worker_isolation (unique_key) VALUES ('shared-key') + ` + + const records = await prisma.$queryRaw<{ id: number; unique_key: string }[]>` + SELECT id, unique_key FROM public.test_worker_isolation + ` + expect(records).toEqual([{ id: 1, unique_key: "shared-key" }]) + } finally { + await prisma.$disconnect() + } + }, + ) + + it("resets custom-schema tables, foreign keys and identities without deleting migrations", async () => { + try { + await prisma.$executeRaw`CREATE SCHEMA IF NOT EXISTS test_isolation` + await prisma.$executeRaw` + CREATE TABLE IF NOT EXISTS public.test_reset_parents ( + id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY + ) + ` + await prisma.$executeRaw` + CREATE TABLE IF NOT EXISTS test_isolation.children ( + id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + parent_id integer NOT NULL REFERENCES public.test_reset_parents(id) + ) + ` + await prisma.$executeRaw` + CREATE TABLE IF NOT EXISTS public._prisma_migrations ( + id varchar(36) PRIMARY KEY, + checksum varchar(64) NOT NULL, + finished_at timestamptz, + migration_name varchar(255) NOT NULL, + logs text, + rolled_back_at timestamptz, + started_at timestamptz NOT NULL DEFAULT now(), + applied_steps_count integer NOT NULL DEFAULT 0 + ) + ` + await prisma.$executeRaw` + INSERT INTO public._prisma_migrations (id, checksum, migration_name) + VALUES ('test-reset-migration', 'test-checksum', 'test-only-reset-marker') + ` + const [parent] = await prisma.$queryRaw<{ id: number }[]>` + INSERT INTO public.test_reset_parents DEFAULT VALUES RETURNING id + ` + await prisma.$executeRaw` + INSERT INTO test_isolation.children (parent_id) VALUES (${parent!.id}) + ` + + await resetDatabase(process.env.DATABASE_URL!) + + const [counts] = await prisma.$queryRaw<{ parents: bigint; children: bigint }[]>` + SELECT + (SELECT count(*) FROM public.test_reset_parents) AS parents, + (SELECT count(*) FROM test_isolation.children) AS children + ` + expect(counts).toEqual({ parents: 0n, children: 0n }) + const migrations = await prisma.$queryRaw<{ migration_name: string }[]>` + SELECT migration_name FROM public._prisma_migrations WHERE id = 'test-reset-migration' + ` + expect(migrations).toEqual([{ migration_name: "test-only-reset-marker" }]) + + const parents = await prisma.$queryRaw<{ id: number }[]>` + INSERT INTO public.test_reset_parents DEFAULT VALUES RETURNING id + ` + const children = await prisma.$queryRaw<{ id: number }[]>` + INSERT INTO test_isolation.children (parent_id) VALUES (${parents[0]!.id}) RETURNING id + ` + expect(parents).toEqual([{ id: 1 }]) + expect(children).toEqual([{ id: 1 }]) + } finally { + try { + await prisma.$executeRaw` + DELETE FROM public._prisma_migrations WHERE id = 'test-reset-migration' + ` + } finally { + await prisma.$disconnect() + } + } + }) +}) diff --git a/hub/server/integrations/prisma/parallel-isolation.db.test.ts b/hub/server/integrations/prisma/parallel-isolation.db.test.ts new file mode 100644 index 00000000..7d75f999 --- /dev/null +++ b/hub/server/integrations/prisma/parallel-isolation.db.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, inject, it } from "vitest" + +import { prisma } from "./index" + +describe("Prisma parallel database isolation", () => { + it.each(["first case", "second case"])( + "starts the %s empty without sharing another file's records", + async () => { + try { + const database = inject("testDatabase") + const [connection] = await prisma.$queryRaw<{ name: string }[]>` + SELECT current_database() AS name + ` + expect(connection?.name).toBe(`${database.prefix}_${process.env.VITEST_POOL_ID}`) + + await prisma.$executeRaw` + CREATE TABLE IF NOT EXISTS public.test_worker_isolation ( + id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, + unique_key text NOT NULL UNIQUE + ) + ` + const [before] = await prisma.$queryRaw<{ count: bigint }[]>` + SELECT count(*) FROM public.test_worker_isolation + ` + expect(before?.count).toBe(0n) + + await prisma.$executeRaw` + INSERT INTO public.test_worker_isolation (unique_key) VALUES ('shared-key') + ` + + const records = await prisma.$queryRaw<{ id: number; unique_key: string }[]>` + SELECT id, unique_key FROM public.test_worker_isolation + ` + expect(records).toEqual([{ id: 1, unique_key: "shared-key" }]) + } finally { + await prisma.$disconnect() + } + }, + ) +}) diff --git a/hub/test/database/get-worker-database/index.test.ts b/hub/test/database/get-worker-database/index.test.ts new file mode 100644 index 00000000..74b95696 --- /dev/null +++ b/hub/test/database/get-worker-database/index.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest" + +import { getWorkerDatabase } from "." + +describe("getWorkerDatabase()", () => { + it.each(["postgres:", "postgresql:"])( + "changes only the database path for a %s connection", + (protocol) => { + const adminUrl = `${protocol}//test_user:example@localhost:5433/hub_development?sslmode=require&application_name=hub-tests&options=-c%20statement_timeout%3D5000` + const database = { adminUrl, prefix: "replica_test_0123456789abcdef", workers: 4 } + + const result = new URL(getWorkerDatabase(database, "4")) + const expected = new URL(adminUrl) + expected.pathname = "/replica_test_0123456789abcdef_4" + + expect(result.href).toBe(expected.href) + }, + ) + + it("gives the first worker its own database rather than the admin database", () => { + const database = { + adminUrl: "postgresql://localhost/hub_development", + prefix: "replica_test_fedcba9876543210", + workers: 1, + } + + expect(new URL(getWorkerDatabase(database, "1")).pathname).toBe( + "/replica_test_fedcba9876543210_1", + ) + }) + + it.each([undefined, "", "0", "-1", "1.5", "1e0", "01", " 1", "1 ", "2", "NaN"])( + "rejects invalid worker ID %s instead of selecting another database", + (poolId) => { + const database = { + adminUrl: "postgresql://localhost/hub_development", + prefix: "replica_test_0123456789abcdef", + workers: 1, + } + + expect(() => getWorkerDatabase(database, poolId)).toThrow() + }, + ) + + it.each([ + "hub_development", + "replica_test_0123456789abcde", + "replica_test_0123456789abcdef0", + "replica_test_0123456789abcdeg", + "replica_test_0123456789ABCDEF", + "replica_test_0123456789abcdef_1", + "replica_test_0123456789abcdef/other", + ])("rejects unsafe database prefix %s", (prefix) => { + const database = { adminUrl: "postgresql://localhost/hub_development", prefix, workers: 1 } + + expect(() => getWorkerDatabase(database, "1")).toThrow() + }) + + it.each(["not a URL", "https://localhost/hub_development", "file:///hub_development"])( + "rejects a non-PostgreSQL admin URL %s", + (adminUrl) => { + const database = { adminUrl, prefix: "replica_test_0123456789abcdef", workers: 1 } + + expect(() => getWorkerDatabase(database, "1")).toThrow() + }, + ) +}) diff --git a/hub/test/database/get-worker-database/index.ts b/hub/test/database/get-worker-database/index.ts new file mode 100644 index 00000000..b27ef899 --- /dev/null +++ b/hub/test/database/get-worker-database/index.ts @@ -0,0 +1,24 @@ +import type { TestDatabaseContext } from "../global-setup/types.ts" + +export function getWorkerDatabase(database: TestDatabaseContext, poolId: string | undefined) { + if ( + !/^replica_test_[0-9a-f]{16}$/.test(database.prefix) || + !Number.isSafeInteger(database.workers) || + database.workers < 1 + ) { + throw new Error("Test database configuration requires a unique run prefix and worker count.") + } + if (!poolId || !/^[1-9][0-9]*$/.test(poolId) || Number(poolId) > database.workers) { + throw new Error("VITEST_POOL_ID must identify a configured test database worker.") + } + + if (!URL.canParse(database.adminUrl)) { + throw new Error("TEST_DATABASE_URL must be a valid PostgreSQL connection URL.") + } + const url = new URL(database.adminUrl) + if (url.protocol !== "postgresql:" && url.protocol !== "postgres:") { + throw new Error("TEST_DATABASE_URL must use PostgreSQL.") + } + url.pathname = `${database.prefix}_${poolId}` + return url.toString() +} diff --git a/hub/test/database/global-setup/index.ts b/hub/test/database/global-setup/index.ts new file mode 100644 index 00000000..cf60e93d --- /dev/null +++ b/hub/test/database/global-setup/index.ts @@ -0,0 +1,88 @@ +import "dotenv/config" +import { execFile } from "node:child_process" +import { randomBytes } from "node:crypto" +import { join } from "node:path" +import { fileURLToPath } from "node:url" +import { promisify } from "node:util" +import pg from "pg" +import type { TestProject } from "vitest/node" + +import { getWorkerDatabase } from "../get-worker-database/index.ts" + +export default async function setup(project: TestProject) { + const adminUrl = process.env.TEST_DATABASE_URL + if (!adminUrl) { + throw new Error("Set TEST_DATABASE_URL to a PostgreSQL connection with CREATEDB permission.") + } + + const database = { + adminUrl, + prefix: `replica_test_${randomBytes(8).toString("hex")}`, + workers: project.config.maxWorkers ?? project.vitest.config.maxWorkers, + } + getWorkerDatabase(database, "1") + + const admin = new pg.Client({ connectionString: adminUrl, connectionTimeoutMillis: 10_000 }) + await admin.connect() + const created: string[] = [] + + async function teardown() { + const errors: unknown[] = [] + for (const name of created.toReversed()) { + try { + await admin.query(`DROP DATABASE ${pg.escapeIdentifier(name)} WITH (FORCE)`) + } catch (error) { + errors.push(error) + } + } + try { + await admin.end() + } catch (error) { + errors.push(error) + } + if (errors.length) throw new AggregateError(errors, "Could not remove test databases.") + } + + try { + const template = `${database.prefix}_template` + await admin.query(`CREATE DATABASE ${pg.escapeIdentifier(template)}`) + created.push(template) + const templateUrl = new URL(adminUrl) + templateUrl.pathname = template + const hubRoot = fileURLToPath(new URL("../../../", import.meta.url)) + await promisify(execFile)( + process.execPath, + [ + join(hubRoot, "node_modules/prisma/build/index.js"), + "db", + "push", + "--config", + join(hubRoot, "prisma7.config.ts"), + ], + { + cwd: hubRoot, + env: { ...process.env, DATABASE_URL: templateUrl.toString() }, + timeout: 60_000, + }, + ) + + for (let worker = 1; worker <= database.workers; worker++) { + const name = `${database.prefix}_${worker}` + await admin.query( + `CREATE DATABASE ${pg.escapeIdentifier(name)} TEMPLATE ${pg.escapeIdentifier(template)}`, + ) + created.push(name) + } + project.provide("testDatabase", database) + return teardown + } catch (error) { + try { + await teardown() + } catch (cleanupError) { + throw new AggregateError([error, cleanupError], "Test database setup and cleanup failed.", { + cause: cleanupError, + }) + } + throw error + } +} diff --git a/hub/test/database/global-setup/types.ts b/hub/test/database/global-setup/types.ts new file mode 100644 index 00000000..5a804b61 --- /dev/null +++ b/hub/test/database/global-setup/types.ts @@ -0,0 +1,11 @@ +export interface TestDatabaseContext { + adminUrl: string + prefix: string + workers: number +} + +declare module "vitest" { + interface ProvidedContext { + testDatabase: TestDatabaseContext + } +} diff --git a/hub/test/database/reset-database/index.test.ts b/hub/test/database/reset-database/index.test.ts new file mode 100644 index 00000000..1abe04db --- /dev/null +++ b/hub/test/database/reset-database/index.test.ts @@ -0,0 +1,26 @@ +import { Socket } from "node:net" +import { describe, expect, it, vi } from "vitest" + +import { resetDatabase } from "." + +describe("resetDatabase()", () => { + it.each([ + "hub_development", + "replica_test_0123456789abcdef", + "replica_test_0123456789abcdef_0", + "replica_test_0123456789abcdef_01", + "replica_test_0123456789abcdef_-1", + "replica_test_0123456789abcdef_1_other", + "replica_test_0123456789abcdeg_1", + "replica_test_0123456789ABCDEF_1", + "replica_test_0123456789abcde_1", + "replica_test_0123456789abcdef0_1", + ])("refuses to reset %s before opening a database connection", async (databaseName) => { + const connect = vi.spyOn(Socket.prototype, "connect").mockImplementation(() => { + throw new Error("The unsafe database must not receive a network connection.") + }) + + await expect(resetDatabase(`postgresql://127.0.0.1:1/${databaseName}`)).rejects.toThrow() + expect(connect).not.toHaveBeenCalled() + }) +}) diff --git a/hub/test/database/reset-database/index.ts b/hub/test/database/reset-database/index.ts new file mode 100644 index 00000000..dfc51cf6 --- /dev/null +++ b/hub/test/database/reset-database/index.ts @@ -0,0 +1,40 @@ +import pg from "pg" + +export async function resetDatabase(databaseUrl: string) { + if (!URL.canParse(databaseUrl)) { + throw new Error("Test database cleanup requires a valid PostgreSQL connection URL.") + } + const url = new URL(databaseUrl) + const name = decodeURIComponent(url.pathname.slice(1)) + if ( + (url.protocol !== "postgresql:" && url.protocol !== "postgres:") || + !/^replica_test_[0-9a-f]{16}_[1-9][0-9]*$/.test(name) + ) { + throw new Error("Refusing to clear a database outside this test worker's namespace.") + } + + const client = new pg.Client({ + connectionString: databaseUrl, + connectionTimeoutMillis: 10_000, + statement_timeout: 10_000, + }) + await client.connect() + try { + const current = await client.query<{ name: string }>("SELECT current_database() AS name") + if (current.rows[0]?.name !== name) { + throw new Error("Connected database does not match the expected test worker database.") + } + const tables = await client.query<{ schemaname: string; tablename: string }>( + "SELECT schemaname, tablename FROM pg_tables WHERE schemaname <> 'information_schema' AND schemaname !~ '^pg_' AND tablename <> '_prisma_migrations'", + ) + if (tables.rows.length) { + const names = tables.rows.map( + ({ schemaname, tablename }) => + `${pg.escapeIdentifier(schemaname)}.${pg.escapeIdentifier(tablename)}`, + ) + await client.query(`TRUNCATE ${names.join(", ")} RESTART IDENTITY CASCADE`) + } + } finally { + await client.end() + } +} diff --git a/hub/test/database/setup.ts b/hub/test/database/setup.ts new file mode 100644 index 00000000..af4d8015 --- /dev/null +++ b/hub/test/database/setup.ts @@ -0,0 +1,12 @@ +import { beforeEach, inject } from "vitest" + +import { getWorkerDatabase } from "./get-worker-database/index.ts" +import { resetDatabase } from "./reset-database/index.ts" + +const databaseUrl = getWorkerDatabase(inject("testDatabase"), process.env.VITEST_POOL_ID) +process.env.DATABASE_URL = databaseUrl + +beforeEach(async () => { + process.env.DATABASE_URL = databaseUrl + await resetDatabase(databaseUrl) +}) diff --git a/hub/test/setup.test.ts b/hub/test/setup.test.ts new file mode 100644 index 00000000..1bc14fe4 --- /dev/null +++ b/hub/test/setup.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it, vi } from "vitest" + +const environmentKey = "REPLICA_SHARED_SETUP_TEST" +const globalKey = "__replicaSharedSetupTest" +const initialEnvironment = process.env[environmentKey] +const initialGlobal: unknown = Reflect.get(globalThis, globalKey) +const mock = vi.fn((value: string) => value) + +describe("shared test setup", () => { + it.each(["first case", "second case"])( + "gives the %s clean timers, environment, globals, mocks and modules", + async () => { + expect(vi.isFakeTimers()).toBe(false) + expect(process.env[environmentKey]).toBe(initialEnvironment) + expect(Reflect.get(globalThis, globalKey)).toBe(initialGlobal) + expect(mock).not.toHaveBeenCalled() + expect(mock("fresh implementation")).toBe("fresh implementation") + + const { getWorkerDatabase } = await import("./database/get-worker-database") + expect(Reflect.has(getWorkerDatabase, globalKey)).toBe(false) + + vi.useFakeTimers() + setTimeout(() => mock("pending timer"), 1_000) + vi.stubEnv(environmentKey, "contaminated") + vi.stubGlobal(globalKey, "contaminated") + mock.mockReturnValue("contaminated") + mock("changed implementation") + Reflect.set(getWorkerDatabase, globalKey, "contaminated") + }, + ) +}) diff --git a/hub/test/setup.ts b/hub/test/setup.ts new file mode 100644 index 00000000..6873d7c3 --- /dev/null +++ b/hub/test/setup.ts @@ -0,0 +1,6 @@ +import { beforeEach, vi } from "vitest" + +beforeEach(() => { + vi.useRealTimers() + vi.resetModules() +}) diff --git a/hub/tsconfig.json b/hub/tsconfig.json index 08eb99cc..516108e7 100644 --- a/hub/tsconfig.json +++ b/hub/tsconfig.json @@ -28,6 +28,7 @@ "include": [ "app/javascript", "server", + "test", ".storybook/**/*.ts", ".storybook/**/*.tsx", "eslint.config.ts", diff --git a/hub/vitest.config.ts b/hub/vitest.config.ts index 4951ecbe..463a323a 100644 --- a/hub/vitest.config.ts +++ b/hub/vitest.config.ts @@ -1,3 +1,4 @@ +import { availableParallelism } from "node:os" import react from "@vitejs/plugin-react" import { configDefaults, defineConfig } from "vitest/config" @@ -33,9 +34,15 @@ export default defineConfig({ clearMocks: true, css: true, globals: true, + pool: "forks", + fileParallelism: true, + maxWorkers: Math.max(1, availableParallelism() - 1), + maxConcurrency: 1, mockReset: true, restoreMocks: true, unstubGlobals: true, + unstubEnvs: true, + setupFiles: ["./test/setup.ts"], server: { deps: { inline: [/@primer\//] } }, projects: [ { @@ -43,7 +50,8 @@ export default defineConfig({ test: { name: "node", environment: "node", - include: [...nodeTests, "server/**/*.test.ts"], + include: [...nodeTests, "server/**/*.test.ts", "test/**/*.test.ts"], + exclude: [...configDefaults.exclude, "**/*.db.test.ts"], }, }, { @@ -56,6 +64,17 @@ export default defineConfig({ setupFiles: ["./app/javascript/test/setup.ts"], }, }, + { + extends: true, + test: { + name: "database", + environment: "node", + include: ["server/**/*.db.test.ts"], + globalSetup: ["./test/database/global-setup/index.ts"], + setupFiles: ["./test/database/setup.ts"], + hookTimeout: 30_000, + }, + }, ], }, }) diff --git a/scripts/check-ci-workflows/index.ts b/scripts/check-ci-workflows/index.ts index 40e7fdf9..f42c032e 100644 --- a/scripts/check-ci-workflows/index.ts +++ b/scripts/check-ci-workflows/index.ts @@ -131,6 +131,7 @@ export function checkWorkflows(root: string): number { "build", "storybook:build", "db:validate", + "test:database", ]) assert(hub.check.steps?.some((step) => step.run === "npm run ${{ matrix.command }}")) for (const command of [ From 8e1a8759acf7668f92a08901fbf9b5da670c4447 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:16:43 +0000 Subject: [PATCH 2/6] ci: run independent checks without suite selection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/actions-ci.yml | 24 --- .github/workflows/ci-changes.yml | 40 ---- .github/workflows/hub-ci.yml | 30 +-- .github/workflows/learning-ci.yml | 6 - .github/workflows/repository-ci.yml | 27 +++ .github/workflows/starter-ci.yml | 14 -- CONTRIBUTING.md | 1 - docs/testing-and-review.md | 24 +-- scripts/check-ci-workflows/index.test.ts | 29 ++- scripts/check-ci-workflows/index.ts | 130 +++-------- scripts/check-ci-workflows/types.ts | 4 + scripts/select-ci-suites.mjs | 89 -------- scripts/select-ci-suites.test.mjs | 262 ----------------------- 13 files changed, 90 insertions(+), 590 deletions(-) delete mode 100644 .github/workflows/ci-changes.yml create mode 100644 .github/workflows/repository-ci.yml delete mode 100644 scripts/select-ci-suites.mjs delete mode 100644 scripts/select-ci-suites.test.mjs diff --git a/.github/workflows/actions-ci.yml b/.github/workflows/actions-ci.yml index 251423cd..0d762e65 100644 --- a/.github/workflows/actions-ci.yml +++ b/.github/workflows/actions-ci.yml @@ -9,13 +9,8 @@ permissions: contents: read jobs: - changes: - uses: ./.github/workflows/ci-changes.yml - test: name: Build action suite${{ matrix.node == 26 && ' (Node 26)' || '' }} - needs: changes - if: ${{ needs.changes.outputs.build_action == 'true' }} runs-on: ubuntu-latest strategy: fail-fast: false @@ -39,22 +34,3 @@ jobs: run: cp actions/build/test/fixture/package.json actions/build/test/fixture/package-lock.json actions/build/test/fixture/fixture-build.mjs . - uses: ./actions/build if: ${{ matrix.node == 24 }} - - result: - name: Replica action tests - needs: [changes, test] - if: ${{ always() }} - runs-on: ubuntu-latest - steps: - - name: Require the selected build-action suite to pass - env: - SELECTION_RESULT: ${{ needs.changes.result }} - SELECTED: ${{ needs.changes.outputs.build_action }} - TEST_RESULT: ${{ needs.test.result }} - run: | - test "$SELECTION_RESULT" = success - case "$SELECTED" in - true) test "$TEST_RESULT" = success ;; - false) test "$TEST_RESULT" = skipped ;; - *) echo "Invalid build-action suite selection" >&2; exit 1 ;; - esac diff --git a/.github/workflows/ci-changes.yml b/.github/workflows/ci-changes.yml deleted file mode 100644 index f486f2e9..00000000 --- a/.github/workflows/ci-changes.yml +++ /dev/null @@ -1,40 +0,0 @@ -name: CI selection - -on: - workflow_call: - outputs: - hub: - description: Run the complete Hub suite - value: ${{ jobs.changes.outputs.hub }} - build_action: - description: Run the build-action suite - value: ${{ jobs.changes.outputs.build_action }} - -permissions: - contents: read - -jobs: - changes: - name: Select affected suites - runs-on: ubuntu-latest - outputs: - hub: ${{ steps.select.outputs.hub }} - build_action: ${{ steps.select.outputs.build_action }} - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - fetch-depth: 0 - - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 - with: - node-version: 24 - - run: npm ci --prefix scripts --ignore-scripts --engine-strict - - run: node --test scripts/select-ci-suites.test.mjs - - run: npm run typecheck --prefix scripts - - run: npm test --prefix scripts - - run: node scripts/check-ci-workflows/index.ts - - id: select - run: node scripts/select-ci-suites.mjs - env: - CI_EVENT_NAME: ${{ github.event_name }} - CI_BASE_SHA: ${{ github.event.pull_request.base.sha }} - CI_HEAD_SHA: ${{ github.event.pull_request.head.sha }} diff --git a/.github/workflows/hub-ci.yml b/.github/workflows/hub-ci.yml index 471795cc..ab189513 100644 --- a/.github/workflows/hub-ci.yml +++ b/.github/workflows/hub-ci.yml @@ -13,13 +13,8 @@ defaults: working-directory: hub jobs: - changes: - uses: ./.github/workflows/ci-changes.yml - check: - name: Hub ${{ matrix.command }}${{ matrix.node == 26 && ' (Node 26)' || '' }} - needs: changes - if: ${{ needs.changes.outputs.hub == 'true' }} + name: Hub ${{ matrix.command == 'test' && 'unit tests' || matrix.command == 'test:database' && 'database tests' || matrix.command }}${{ matrix.node == 26 && ' (Node 26)' || '' }} runs-on: ubuntu-latest strategy: fail-fast: false @@ -54,8 +49,6 @@ jobs: integration: name: Controls and extension tests${{ matrix.node == 26 && ' (Node 26)' || '' }} - needs: changes - if: ${{ needs.changes.outputs.hub == 'true' }} runs-on: ubuntu-latest strategy: fail-fast: false @@ -83,24 +76,3 @@ jobs: - run: npm run controls:assets:check - run: npm run controls:schema:check - run: npm run extension:assets:check - - result: - name: Frontend tests - needs: [changes, check, integration] - if: ${{ always() }} - runs-on: ubuntu-latest - steps: - - name: Require the selected Hub suite to pass - working-directory: . - env: - SELECTION_RESULT: ${{ needs.changes.result }} - SELECTED: ${{ needs.changes.outputs.hub }} - CHECK_RESULT: ${{ needs.check.result }} - INTEGRATION_RESULT: ${{ needs.integration.result }} - run: | - test "$SELECTION_RESULT" = success - case "$SELECTED" in - true) test "$CHECK_RESULT" = success && test "$INTEGRATION_RESULT" = success ;; - false) test "$CHECK_RESULT" = skipped && test "$INTEGRATION_RESULT" = skipped ;; - *) echo "Invalid Hub suite selection" >&2; exit 1 ;; - esac diff --git a/.github/workflows/learning-ci.yml b/.github/workflows/learning-ci.yml index bddec7b4..cc03a376 100644 --- a/.github/workflows/learning-ci.yml +++ b/.github/workflows/learning-ci.yml @@ -2,14 +2,8 @@ name: Learning CI on: pull_request: - paths: - - ".github/workflows/learning-ci.yml" - - "packages/learning/**" push: branches: [main] - paths: - - ".github/workflows/learning-ci.yml" - - "packages/learning/**" permissions: contents: read diff --git a/.github/workflows/repository-ci.yml b/.github/workflows/repository-ci.yml new file mode 100644 index 00000000..5fac1894 --- /dev/null +++ b/.github/workflows/repository-ci.yml @@ -0,0 +1,27 @@ +name: Repository CI + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +jobs: + check: + name: Repository ${{ matrix.command }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + command: [typecheck, test, "check:workflows"] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: 24 + cache: npm + cache-dependency-path: scripts/package-lock.json + - run: npm ci --prefix scripts --ignore-scripts --engine-strict + - run: npm run ${{ matrix.command }} --prefix scripts diff --git a/.github/workflows/starter-ci.yml b/.github/workflows/starter-ci.yml index 4f1b35f5..7218e5c1 100644 --- a/.github/workflows/starter-ci.yml +++ b/.github/workflows/starter-ci.yml @@ -2,22 +2,8 @@ name: Starter CI on: pull_request: - paths: - - ".github/workflows/starter-ci.yml" - - "starter/**" - - "scripts/create-prototype-repo.sh" - - "scripts/create-prototype-repo.test.mjs" - - "scripts/setup-replica-plugin.test.mjs" - - "scripts/install-starter.sh" push: branches: [main] - paths: - - ".github/workflows/starter-ci.yml" - - "starter/**" - - "scripts/create-prototype-repo.sh" - - "scripts/create-prototype-repo.test.mjs" - - "scripts/setup-replica-plugin.test.mjs" - - "scripts/install-starter.sh" permissions: contents: read diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e6785eba..2b592adc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -126,7 +126,6 @@ Repository workflow and dependency checks have their own small npm project: npm ci --prefix scripts --ignore-scripts --engine-strict npm run typecheck --prefix scripts npm test --prefix scripts -node --test scripts/select-ci-suites.test.mjs node scripts/check-ci-workflows/index.ts node scripts/check-dependency-age/index.ts ``` diff --git a/docs/testing-and-review.md b/docs/testing-and-review.md index ee382016..623ea589 100644 --- a/docs/testing-and-review.md +++ b/docs/testing-and-review.md @@ -40,31 +40,19 @@ Project commands define the check. GitHub Actions should prepare the required en When a command gains a dependency or build prerequisite, update its local setup and affected workflow in the same change. A green local suite does not prove that CI installed the same dependencies, so inspect both paths. -### Select affected CI suites +### Run independent CI checks -Hub and build-action workflows start on every pull request, but run their expensive jobs only when the change can affect that suite. Pushes to `main` run both complete suites. Local project commands still run their complete checks; selection changes when CI calls them, not what they do. Dependency-age checks remain unconditional, and Starter and Learning CI retain their own path filters. +Hub, build-action, Starter, Learning, and repository-tooling workflows run on every pull request and push to `main`. Do not add changed-path filters, suite selectors, or aggregate result gates. Checks run independently, and branch protection should require their individual check names. -The [shared selection workflow](../.github/workflows/ci-changes.yml) calls [the selector](../scripts/select-ci-suites.mjs). For pull requests, it compares the merge base with the PR head using Git rather than the size-limited changed-files API. Deletions and both sides of renames count as changes. +Hub reports lint, types, unit tests, database tests, the frontend/API build, Storybook, and Controls/extension checks separately. Only the database test command starts PostgreSQL. The new Hub has no production-image or release jobs. -| Changed area | Selected suites | -| --- | --- | -| Hub code and configuration | Hub | -| Shared Controls runtime, schema, fixtures, and capture adapter under `hub/app/javascript/utils/prototype-controls/` | Hub and build action | -| `actions/build/` | Hub and build action; Controls and extension integration use its pinned browser | -| Copilot extension and plugin catalog | Hub | -| `packages/learning/` | Hub; its shared lint and retained integration consume the package | -| `starter/` | Hub; retained Storybook fixtures and Controls checks consume starter files. Starter CI still runs separately | -| Existing installer scripts | Neither; Starter CI covers them | -| Root contributor documents, shared and Hub guides, skills, and documentation assets | Neither | -| Workflow changes, selector changes, and unclassified paths | Both | +When replacing the old aggregate gates, maintainers must replace `Frontend tests` and `Replica action tests` in any branch rules with the individual checks they require. Workflow changes do not update repository rules automatically. -The `Frontend tests` and `Replica action tests` result jobs always run, so skipping an entire workflow cannot leave these checks pending. They accept skipped work only when the selector succeeded and said it was unnecessary. Hub's result covers lint, types, Vitest unit and worker-isolated database tests, the frontend/API build, Storybook, and Controls/extension checks on every supported Node major. A failed selector, invalid output, or failed selected check makes the result fail. The new Hub has no production-image or release jobs; branch protection must not require removed jobs. - -When adding a shared dependency or a new project, update the selector and its behavioral tests together. Unknown paths deliberately run both suites until their dependencies are known. Install tooling with `npm ci --prefix scripts --ignore-scripts --engine-strict`, then run `node --test scripts/select-ci-suites.test.mjs`, `npm run typecheck --prefix scripts`, `npm test --prefix scripts`, and `node scripts/check-ci-workflows/index.ts` from the repository root. Keep the TypeScript checks in function directories with colocated Vitest tests. The workflow checker reads the result-check scripts and tests them with success, failure, cancellation, and missing outputs. These checks run before CI selects suites. Check changed workflows with `actionlint` when available. Actual job skipping and check reporting still need verification on GitHub after publishing. +The [repository workflow](../.github/workflows/repository-ci.yml) runs tooling type checks, tests, and workflow checks independently. Locally, install tooling with `npm ci --prefix scripts --ignore-scripts --engine-strict`, then run `npm run typecheck --prefix scripts`, `npm test --prefix scripts`, and `node scripts/check-ci-workflows/index.ts`. The workflow checker rejects aggregate gates, conditional jobs, path filters, and missing required commands. Keep TypeScript checks in function directories with colocated Vitest tests. Check changed workflows with `actionlint` when available. Actual check reporting still needs verification on GitHub after publishing. Node-dependent jobs in Hub, Starter, build-action, and Learning CI use a Node 24 and 26 matrix rather than a separate compatibility workflow. Each check adds a `(Node 26)` suffix on Node 26. A failure does not cancel the other version. npm installation enforces dependency engines, and `scripts/check-ci-workflows/index.ts` checks the supported ranges, matching lockfile metadata, and matrix wiring. These checks track the supported majors, not their exact minimum patch releases. -CI selection and dependency-age tooling run on Node 24. The build-action suite tests its JavaScript and browser capture on both Node versions, but runs the composite-action smoke test only on Node 24: the composite action pins that runtime itself and uploads a single named artifact. +Repository and dependency-age tooling run on Node 24. The build-action suite tests its JavaScript and browser capture on both Node versions, but runs the composite-action smoke test only on Node 24: the composite action pins that runtime itself and uploads a single named artifact. ## Update dependencies diff --git a/scripts/check-ci-workflows/index.test.ts b/scripts/check-ci-workflows/index.test.ts index dc03e062..070d3d45 100644 --- a/scripts/check-ci-workflows/index.test.ts +++ b/scripts/check-ci-workflows/index.test.ts @@ -29,28 +29,39 @@ function fixture() { } describe("checkWorkflows()", () => { - it("rejects failed, cancelled, skipped, and missing selected results in the committed workflows", () => { - expect(checkWorkflows(root)).toBe(600) + it("accepts the committed independent checks on both supported Node versions", () => { + expect(() => checkWorkflows(root)).not.toThrow() }) - it("rejects a result check that silently accepts failed jobs", () => { + it("rejects an aggregate result gate", () => { const directory = fixture() const path = join(directory, ".github/workflows/hub-ci.yml") const workflow = parse(readFileSync(path, "utf8")) as Workflow - assert(workflow.jobs.result.steps) - workflow.jobs.result.steps[0].run = "exit 0" + workflow.jobs.result = { + needs: ["check", "integration"], + steps: [{ run: "exit 0" }], + } writeFileSync(path, stringify(workflow)) expect(() => checkWorkflows(directory)).toThrow(/hub-ci/) }) - it("rejects a result check that reads the wrong suite choice", () => { + it.each(["if", "needs"] as const)("rejects job-level %s that could skip checks", (key) => { const directory = fixture() const path = join(directory, ".github/workflows/actions-ci.yml") const workflow = parse(readFileSync(path, "utf8")) as Workflow - assert(workflow.jobs.result.steps?.[0].env) - workflow.jobs.result.steps[0].env.SELECTED = "${{ needs.changes.outputs.hub }}" + workflow.jobs.test[key] = key === "if" ? "${{ false }}" : "changes" + writeFileSync(path, stringify(workflow)) + expect(() => checkWorkflows(directory)).toThrow(/actions-ci\/test/) + }) + + it.each(["pull_request", "push"] as const)("rejects %s path filters", (event) => { + const directory = fixture() + const path = join(directory, ".github/workflows/starter-ci.yml") + const workflow = parse(readFileSync(path, "utf8")) as Workflow + assert(workflow.on) + workflow.on[event] = { ...workflow.on[event], paths: ["starter/**"] } writeFileSync(path, stringify(workflow)) - expect(() => checkWorkflows(directory)).toThrow(/build_action/) + expect(() => checkWorkflows(directory)).toThrow(/starter-ci/) }) it("rejects missing supported Node versions and required commands", () => { diff --git a/scripts/check-ci-workflows/index.ts b/scripts/check-ci-workflows/index.ts index f42c032e..32d6f0f7 100644 --- a/scripts/check-ci-workflows/index.ts +++ b/scripts/check-ci-workflows/index.ts @@ -1,5 +1,4 @@ import assert from "node:assert/strict" -import { spawnSync, type SpawnSyncReturns } from "node:child_process" import { readFileSync } from "node:fs" import { resolve } from "node:path" import { pathToFileURL } from "node:url" @@ -13,87 +12,28 @@ const expression = (value: unknown) => .replace(/^\$\{\{\s*|\s*\}\}$/g, "") .trim() const suffix = "${{ matrix.node == 26 && ' (Node 26)' || '' }}" -const states = ["success", "failure", "cancelled", "skipped", ""] -export function checkWorkflows(root: string): number { +export function checkWorkflows(root: string): void { const workflow = (name: string): Workflow => parse(readFileSync(resolve(root, `.github/workflows/${name}.yml`), "utf8")) as Workflow - let cases = 0 - for (const [name, suite, results] of [ - ["hub-ci", "hub", ["check", "integration"]], - ["actions-ci", "build_action", ["test"]], + for (const [name, ids] of [ + ["hub-ci", ["check", "integration"]], + ["actions-ci", ["test"]], + ["starter-ci", ["test"]], + ["learning-ci", ["test"]], + ["repository-ci", ["check"]], ] as const) { - const jobs = workflow(name).jobs + const { jobs, on: triggers } = workflow(name) + assert(triggers, `${name}: missing workflow triggers`) + assert("pull_request" in triggers, `${name}: run on every pull request`) + assert.equal(triggers.pull_request, null, `${name}: pull requests must not filter changes`) + assert.deepEqual(triggers.push, { branches: ["main"] }, `${name}: run on every main push`) + assert.deepEqual(Object.keys(jobs).sort(), [...ids].sort(), `${name}: individual checks only`) for (const [id, job] of Object.entries(jobs)) { - if (id === "changes") continue - assert( - Array.isArray(job.needs) ? job.needs.includes("changes") : job.needs === "changes", - `${name}/${id}: missing changes dependency`, - ) - if (id !== "result") { - assert.equal( - expression(job.if), - `needs.changes.outputs.${suite} == 'true'`, - `${name}/${id}: require suite selection`, - ) - continue - } - assert.equal(job.name, suite === "hub" ? "Frontend tests" : "Replica action tests") - assert.equal(expression(job.if), "always()", `${name}: result check must always run`) - assert(Array.isArray(job.needs), `${name}: result dependencies must be a list`) - assert.deepEqual([...job.needs].sort(), ["changes", ...results].sort()) - assert(job.steps, `${name}: missing result steps`) - assert.equal(job.steps.length, 1, `${name}: expected one unconditional result step`) - const guard = job.steps[0] - assert(!guard.if, `${name}: result step must not be conditional`) - assert(guard.env && guard.run, `${name}: result step needs inputs and a command`) - const expectedEnv: Record = { - SELECTION_RESULT: "needs.changes.result", - SELECTED: `needs.changes.outputs.${suite}`, - } - for (const result of results) - expectedEnv[`${result.toUpperCase()}_RESULT`] = `needs.${result}.result` - assert.deepEqual(Object.keys(guard.env).sort(), Object.keys(expectedEnv).sort()) - for (const [key, value] of Object.entries(expectedEnv)) - assert.equal(expression(guard.env[key]), value) - let combinations: Record[] = [{}] - for (const result of results) { - combinations = combinations.flatMap((values) => - states.map((state) => ({ ...values, [`${result.toUpperCase()}_RESULT`]: state })), - ) - } - for (const selection of states) { - for (const selected of ["true", "false", "invalid", ""]) { - for (const values of combinations) { - const env = { SELECTION_RESULT: selection, SELECTED: selected, ...values } - const expected = - selection === "success" && - ["true", "false"].includes(selected) && - Object.values(values).every( - (state) => state === (selected === "true" ? "success" : "skipped"), - ) - const run: SpawnSyncReturns = spawnSync( - "bash", - ["--noprofile", "--norc", "-e", "-c", guard.run], - { - cwd: root, - env: { ...process.env, BASH_ENV: "", ...env }, - encoding: "utf8", - timeout: 5000, - }, - ) - assert.equal( - run.status === 0, - expected, - `${name} ${JSON.stringify(env)}: ${run.error ?? run.stdout + run.stderr}`, - ) - cases++ - } - } - } + assert(!job.if, `${name}/${id}: checks must run unconditionally`) + assert(!job.needs, `${name}/${id}: checks must run independently`) + assert(!job.uses && job.steps?.length, `${name}/${id}: run checks, not an aggregate gate`) } - assert(jobs.result, `${name}: missing result check`) - assert.equal(jobs.changes.uses, "./.github/workflows/ci-changes.yml") } for (const project of ["hub", "starter", "actions/build", "packages/learning", "scripts"]) { const manifest = JSON.parse(readFileSync(resolve(root, project, "package.json"), "utf8")) @@ -154,34 +94,28 @@ export function checkWorkflows(root: string): number { assert(step, "Composite smoke fixture and action must run") assert.equal(expression(step.if), "matrix.node == 24", "Upload the composite fixture only once") } - const selectionSteps = workflow("ci-changes").jobs.changes.steps - assert(selectionSteps, "Missing selection steps") - for (const command of [ - "npm ci --prefix scripts --ignore-scripts --engine-strict", - "node --test scripts/select-ci-suites.test.mjs", - "npm run typecheck --prefix scripts", - "npm test --prefix scripts", - "node scripts/check-ci-workflows/index.ts", - ]) { - assert( - selectionSteps.some((step) => step.run === command), - `Selection must run ${command}`, - ) - } + const repository = workflow("repository-ci").jobs.check + assert.deepEqual(repository.strategy?.matrix.command, ["typecheck", "test", "check:workflows"]) + assert.equal(repository.strategy?.["fail-fast"], false) + assert( + repository.steps?.some( + (step) => step.run === "npm ci --prefix scripts --ignore-scripts --engine-strict", + ), + "Repository checks must install tooling", + ) + assert( + repository.steps?.some((step) => step.run === "npm run ${{ matrix.command }} --prefix scripts"), + "Repository checks must run each tooling command", + ) const ageSteps = workflow("dependency-age").jobs.audit.steps assert( ageSteps?.some((step) => step.run === "node scripts/check-dependency-age/index.ts"), "Dependency age must run unconditionally", ) - assert( - !workflow("dependency-age").jobs.audit.if, - "Dependency age must not depend on suite selection", - ) - return cases + assert(!workflow("dependency-age").jobs.audit.if, "Dependency age must run unconditionally") } if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1])).href) { - console.log( - `CI result checks and supported Node versions passed (${checkWorkflows(resolve(import.meta.dirname, "../.."))} shell cases).`, - ) + checkWorkflows(resolve(import.meta.dirname, "../..")) + console.log("Independent CI checks and supported Node versions passed.") } diff --git a/scripts/check-ci-workflows/types.ts b/scripts/check-ci-workflows/types.ts index 02677d49..a03cdca7 100644 --- a/scripts/check-ci-workflows/types.ts +++ b/scripts/check-ci-workflows/types.ts @@ -20,5 +20,9 @@ interface Job { } export interface Workflow { + on?: { + pull_request?: null | Record + push?: Record + } jobs: Record } diff --git a/scripts/select-ci-suites.mjs b/scripts/select-ci-suites.mjs deleted file mode 100644 index 4614393c..00000000 --- a/scripts/select-ci-suites.mjs +++ /dev/null @@ -1,89 +0,0 @@ -import { execFileSync } from "node:child_process" -import { appendFileSync } from "node:fs" -import { pathToFileURL } from "node:url" - -const documentation = new Set([ - "AGENTS.md", - "CODE_OF_CONDUCT.md", - "CONTRIBUTING.md", - "LICENSE", - "NOTICE.md", - "README.md", - "SECURITY.md", - "SUPPORT.md", - ".github/pull_request_template.md", -]) - -const starterScripts = new Set([ - "scripts/create-prototype-repo.sh", - "scripts/create-prototype-repo.test.mjs", - "scripts/install-starter.sh", - "scripts/setup-replica-plugin.test.mjs", -]) - -export function selectCiSuites(paths) { - const suites = { hub: false, build_action: false } - for (const path of paths) { - if (path.startsWith("hub/app/javascript/utils/prototype-controls/")) { - suites.hub = true - suites.build_action = true - } else if (path.startsWith("actions/build/")) { - suites.hub = true - suites.build_action = true - } else if ( - documentation.has(path) || - path.startsWith("docs/") || - path.startsWith("hub/docs/") || - path === "hub/AGENTS.md" || - path.startsWith(".github/skills/") || - path.startsWith(".github/assets/") || - path.startsWith(".github/ISSUE_TEMPLATE/") - ) { - continue - } else if ( - path.startsWith("hub/") || - path.startsWith(".github/extensions/replica-prototype/") || - path.startsWith(".github/plugin/") || - path.startsWith("packages/learning/") || - path.startsWith("starter/") - ) { - suites.hub = true - } else if (starterScripts.has(path)) { - continue - } else { - suites.hub = true - suites.build_action = true - } - } - return suites -} - -export function changedCiSuites(event, base, head) { - if (event === "push") return { hub: true, build_action: true } - if (event !== "pull_request") throw new Error(`Unsupported CI event: ${event}`) - if (![base, head].every((sha) => typeof sha === "string" && /^[a-f0-9]{40}$/.test(sha))) { - throw new Error("CI selection requires full base and head commit SHAs.") - } - const paths = execFileSync( - "git", - ["diff", "--name-only", "--no-renames", "-z", `${base}...${head}`, "--"], - { encoding: "utf8", maxBuffer: 32 * 1024 * 1024 }, - ) - .split("\0") - .filter(Boolean) - return selectCiSuites(paths) -} - -if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - const suites = changedCiSuites( - process.env.CI_EVENT_NAME, - process.env.CI_BASE_SHA, - process.env.CI_HEAD_SHA, - ) - const output = Object.entries(suites) - .map(([suite, selected]) => `${suite}=${selected}\n`) - .join("") - if (!process.env.GITHUB_OUTPUT) throw new Error("GITHUB_OUTPUT is required for CI selection.") - appendFileSync(process.env.GITHUB_OUTPUT, output) - process.stdout.write(output) -} diff --git a/scripts/select-ci-suites.test.mjs b/scripts/select-ci-suites.test.mjs deleted file mode 100644 index d70e5fee..00000000 --- a/scripts/select-ci-suites.test.mjs +++ /dev/null @@ -1,262 +0,0 @@ -import assert from "node:assert/strict" -import { spawnSync } from "node:child_process" -import { - mkdirSync, - mkdtempSync, - readFileSync, - rmSync, - unlinkSync, - writeFileSync, -} from "node:fs" -import { dirname, join } from "node:path" -import { after, test } from "node:test" -import { fileURLToPath, pathToFileURL } from "node:url" -import { selectCiSuites } from "./select-ci-suites.mjs" - -const root = dirname(dirname(fileURLToPath(import.meta.url))) -const script = join(root, "scripts/select-ci-suites.mjs") -const selectorUrl = pathToFileURL(script).href -const fixtureRoot = mkdtempSync(join(root, ".select-ci-suites-test-")) -after(() => rmSync(fixtureRoot, { recursive: true, force: true })) - -function git(repo, ...args) { - const result = spawnSync("git", args, { cwd: repo, encoding: "utf8" }) - assert.ifError(result.error) - assert.equal(result.status, 0, result.stderr) - return result.stdout.trim() -} - -function createRepo(name) { - const repo = join(fixtureRoot, name) - mkdirSync(repo) - git(repo, "init", "--quiet") - git(repo, "config", "user.name", "CI suite test") - git(repo, "config", "user.email", "ci-suite-test@example.invalid") - git(repo, "config", "commit.gpgsign", "false") - writeFileSync(join(repo, "README.md"), "fixture\n") - git(repo, "add", "--all") - git(repo, "commit", "--quiet", "-m", "initial") - return repo -} - -function writeFile(repo, path, content = "fixture\n") { - const file = join(repo, path) - mkdirSync(dirname(file), { recursive: true }) - writeFileSync(file, content) -} - -function commit(repo, message) { - git(repo, "add", "--all") - git(repo, "commit", "--quiet", "-m", message) - return git(repo, "rev-parse", "HEAD") -} - -function runCli(repo, env) { - return spawnSync(process.execPath, [script], { - cwd: repo, - encoding: "utf8", - env: { ...process.env, ...env }, - timeout: 10_000, - }) -} - -function runChangedCiSuites(repo, args) { - const source = ` - import { changedCiSuites } from ${JSON.stringify(selectorUrl)} - const args = JSON.parse(process.env.SELECTOR_ARGS) - try { - process.stdout.write(JSON.stringify(changedCiSuites(...args))) - } catch (error) { - console.error(error.message) - process.exitCode = 1 - } - ` - const result = spawnSync(process.execPath, ["--input-type=module", "-e", source], { - cwd: repo, - encoding: "utf8", - env: { ...process.env, SELECTOR_ARGS: JSON.stringify(args) }, - timeout: 10_000, - }) - assert.ifError(result.error) - return result -} - -function selectChangedFiles(repo, ...args) { - const result = runChangedCiSuites(repo, args) - assert.equal(result.status, 0, result.stderr) - return JSON.parse(result.stdout) -} - -test("path selection keeps documentation and installer scripts out while covering Hub consumers", () => { - const excluded = [ - "README.md", - "docs/guide.md", - "hub/docs/guide.md", - "hub/AGENTS.md", - ".github/skills/write-tests/SKILL.md", - ".github/assets/logo.svg", - ".github/ISSUE_TEMPLATE/bug.yml", - "scripts/create-prototype-repo.sh", - "scripts/create-prototype-repo.test.mjs", - "scripts/install-starter.sh", - "scripts/setup-replica-plugin.test.mjs", - ] - for (const path of excluded) { - assert.deepEqual(selectCiSuites([path]), { hub: false, build_action: false }, path) - } - - for (const path of [ - "hub/app/models/prototype.rb", - ".github/extensions/replica-prototype/extension.mjs", - ".github/plugin/README.md", - "packages/learning/src/index.mjs", - "starter/.agents/skills/prototype-controls/SKILL.md", - "starter/src/app/router.tsx", - "starter/package-lock.json", - "starter/.prototype", - "starter/.agents/skills/setup-skills/SKILL.md", - "starter/src/App.jsx", - ]) { - assert.deepEqual(selectCiSuites([path]), { hub: true, build_action: false }, path) - } - for (const path of [ - "hub/app/javascript/utils/prototype-controls/controls.ts", - "actions/build/src/index.mjs", - ]) { - assert.deepEqual(selectCiSuites([path]), { hub: true, build_action: true }, path) - } -}) - -test("mixed changes union selections and unknown or CI configuration paths select both suites", () => { - assert.deepEqual( - selectCiSuites(["docs/guide.md", "hub/app/models/prototype.rb", "starter/src/App.jsx"]), - { hub: true, build_action: false }, - ) - for (const path of [ - ".github/workflows/hub-ci.yml", - ".github/workflows/ci-changes.yml", - "scripts/select-ci-suites.mjs", - ".github/extensions/new-extension/extension.yml", - "new-root-file.txt", - ]) { - assert.deepEqual(selectCiSuites([path]), { hub: true, build_action: true }, path) - } -}) - -test("pushes always select both suites while pull requests diff from their merge base", () => { - const repo = createRepo("merge-base") - assert.deepEqual(selectChangedFiles(repo, "push"), { hub: true, build_action: true }) - const common = git(repo, "rev-parse", "HEAD") - git(repo, "switch", "--quiet", "-c", "base") - writeFile(repo, "hub/base-only.rb") - const base = commit(repo, "change base") - git(repo, "switch", "--quiet", "-c", "head", common) - writeFile(repo, "docs/release\nhub/not-a-path.rb") - writeFile(repo, "docs/guide.md") - const head = commit(repo, "change pull request") - - assert.deepEqual(selectChangedFiles(repo, "pull_request", base, head), { - hub: false, - build_action: false, - }) -}) - -test("pull-request diffs count deleted paths and both sides of cross-boundary renames", () => { - const deletionRepo = createRepo("deletion") - writeFile(deletionRepo, "actions/build/removed.mjs") - const withFile = commit(deletionRepo, "add build file") - unlinkSync(join(deletionRepo, "actions/build/removed.mjs")) - const deletionHead = commit(deletionRepo, "delete build file") - assert.deepEqual(selectChangedFiles(deletionRepo, "pull_request", withFile, deletionHead), { - hub: true, - build_action: true, - }) - - const oldPathRepo = createRepo("rename-old-path") - writeFile(oldPathRepo, "actions/build/renamed.mjs") - const oldPathWithFile = commit(oldPathRepo, "add action file") - mkdirSync(join(oldPathRepo, "docs"), { recursive: true }) - git(oldPathRepo, "mv", "actions/build/renamed.mjs", "docs/renamed.mjs") - const oldPathHead = commit(oldPathRepo, "move action file to docs") - assert.deepEqual(selectChangedFiles(oldPathRepo, "pull_request", oldPathWithFile, oldPathHead), { - hub: true, - build_action: true, - }) - - const newPathRepo = createRepo("rename-new-path") - writeFile(newPathRepo, "docs/renamed.mjs") - const newPathWithFile = commit(newPathRepo, "add documentation file") - mkdirSync(join(newPathRepo, "hub/app"), { recursive: true }) - git(newPathRepo, "mv", "docs/renamed.mjs", "hub/app/renamed.mjs") - const newPathHead = commit(newPathRepo, "move docs to Hub") - assert.deepEqual(selectChangedFiles(newPathRepo, "pull_request", newPathWithFile, newPathHead), { - hub: true, - build_action: false, - }) -}) - -test("invalid event data and unavailable git history fail explicitly", () => { - const repo = createRepo("invalid-data") - const unsupported = runChangedCiSuites(repo, ["schedule"]) - assert.notEqual(unsupported.status, 0) - assert.match(unsupported.stderr, /Unsupported CI event/) - const validSha = "b".repeat(40) - for (const [base, head] of [ - [undefined, validSha], - [validSha, undefined], - ["abc", validSha], - [validSha, "abc"], - ["A".repeat(40), validSha], - [validSha, "A".repeat(40)], - ]) { - const result = runChangedCiSuites(repo, ["pull_request", base, head]) - assert.notEqual(result.status, 0) - assert.match(result.stderr, /full base and head commit SHAs/) - } - - const missingHistory = runChangedCiSuites(repo, [ - "pull_request", - "a".repeat(40), - "b".repeat(40), - ]) - assert.notEqual(missingHistory.status, 0) - assert.match(missingHistory.stderr, /fatal|git/i) -}) - -test("the CLI writes suite booleans and fails without an output file", () => { - const repo = createRepo("cli") - const base = git(repo, "rev-parse", "HEAD") - writeFile(repo, "hub/app/models/prototype.rb") - const head = commit(repo, "change Hub") - const output = join(repo, "github-output") - const env = { - CI_EVENT_NAME: "pull_request", - CI_BASE_SHA: base, - CI_HEAD_SHA: head, - GITHUB_OUTPUT: output, - } - - const result = runCli(repo, env) - assert.ifError(result.error) - assert.equal(result.status, 0, result.stderr) - assert.deepEqual(result.stdout.trim().split("\n").sort(), [ - "build_action=false", - "hub=true", - ]) - assert.deepEqual(readFileSync(output, "utf8").trim().split("\n").sort(), [ - "build_action=false", - "hub=true", - ]) - - const missingOutputEnv = { ...env, GITHUB_OUTPUT: undefined } - const missingOutput = runCli(repo, missingOutputEnv) - assert.ifError(missingOutput.error) - assert.notEqual(missingOutput.status, 0) - assert.equal(missingOutput.stdout, "") - assert.match(missingOutput.stderr, /GITHUB_OUTPUT is required/) - assert.equal(missingOutput.signal, null) - assert.deepEqual(readFileSync(output, "utf8").trim().split("\n").sort(), [ - "build_action=false", - "hub=true", - ]) -}) From 7f9505697733e100002de5f09d2cf349f29c11d5 Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:05:27 +0000 Subject: [PATCH 3/6] docs: clarify database and test setup guidance Rename the maintenance workflow and checks to Scripts CI. Co-authored-by: Nishiki Liu Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../{repository-ci.yml => scripts-ci.yml} | 4 +-- AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- docs/testing-and-review.md | 12 +++---- hub/README.md | 4 ++- hub/docs/architecture.md | 4 ++- hub/docs/database.md | 21 ++++++++--- hub/docs/development.md | 4 +-- hub/docs/testing-and-operations.md | 35 ++++++++++++------- scripts/check-ci-workflows/index.ts | 16 ++++----- 10 files changed, 65 insertions(+), 39 deletions(-) rename .github/workflows/{repository-ci.yml => scripts-ci.yml} (91%) diff --git a/.github/workflows/repository-ci.yml b/.github/workflows/scripts-ci.yml similarity index 91% rename from .github/workflows/repository-ci.yml rename to .github/workflows/scripts-ci.yml index 5fac1894..234cbe62 100644 --- a/.github/workflows/repository-ci.yml +++ b/.github/workflows/scripts-ci.yml @@ -1,4 +1,4 @@ -name: Repository CI +name: Scripts CI on: pull_request: @@ -10,7 +10,7 @@ permissions: jobs: check: - name: Repository ${{ matrix.command }} + name: Scripts ${{ matrix.command }} runs-on: ubuntu-latest strategy: fail-fast: false diff --git a/AGENTS.md b/AGENTS.md index 123a80a8..8d6083e3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ Follow the [commit and PR conventions](CONTRIBUTING.md#commits-and-pull-requests ## Project boundaries -- `hub/` starts a TypeScript Express API and React + Vite frontend. Hosting and review features are not implemented. Database, sign-in, and deployment choices are not decided. Shared canvas and Controls source stays under `hub/app/javascript/`. Read `hub/AGENTS.md` and [Hub architecture](hub/docs/architecture.md) before changing it. +- `hub/` starts a TypeScript Express API and React + Vite frontend, with PostgreSQL and Prisma for database development. Hosting and review features are not implemented. Sign-in and deployment choices are not decided. Shared canvas and Controls source stays under `hub/app/javascript/`. Read `hub/AGENTS.md` and [Hub architecture](hub/docs/architecture.md) before changing it. - `starter/` is the single source for the Vite + React app, `.agents/skills/`, and `.agents/context-templates/`. Product- and design-system-specific guidance belongs in consuming repositories. - `actions/build/` owns the artifact contract between prototype repositories and Hub. - `packages/learning/` owns the learning contracts, the memory provider and consolidator interfaces, the reference providers and consolidators, the conformance suite, and the `replica-learning` command. It is private repository source with no release yet. Read its `README.md` and [ADR 0001](docs/adr/0001-learning-extension-architecture.md) before changing it. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2b592adc..6760d912 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -107,7 +107,7 @@ npm run build npm run storybook:build ``` -Run `npm run test:database` with the [Hub development database](hub/docs/development.md) for server cases that save records. It prepares separate databases per Vitest worker; ordinary `npm test` remains database-free. +Run `npm run test:database` for server cases that save records. Follow the [database test setup](hub/docs/testing-and-operations.md#parallel-database-tests) to prepare isolated databases. `npm test` does not require PostgreSQL. Useful focused commands include: diff --git a/docs/testing-and-review.md b/docs/testing-and-review.md index 623ea589..4ffdcbb3 100644 --- a/docs/testing-and-review.md +++ b/docs/testing-and-review.md @@ -42,17 +42,17 @@ When a command gains a dependency or build prerequisite, update its local setup ### Run independent CI checks -Hub, build-action, Starter, Learning, and repository-tooling workflows run on every pull request and push to `main`. Do not add changed-path filters, suite selectors, or aggregate result gates. Checks run independently, and branch protection should require their individual check names. +All CI workflows run on every pull request and push to `main`. Do not skip checks based on changed paths or add jobs that only combine other jobs' results. Configure branch protection to require the individual checks. Hub reports lint, types, unit tests, database tests, the frontend/API build, Storybook, and Controls/extension checks separately. Only the database test command starts PostgreSQL. The new Hub has no production-image or release jobs. -When replacing the old aggregate gates, maintainers must replace `Frontend tests` and `Replica action tests` in any branch rules with the individual checks they require. Workflow changes do not update repository rules automatically. +Maintainers must update branch rules when required checks change. Editing a workflow does not update those rules. -The [repository workflow](../.github/workflows/repository-ci.yml) runs tooling type checks, tests, and workflow checks independently. Locally, install tooling with `npm ci --prefix scripts --ignore-scripts --engine-strict`, then run `npm run typecheck --prefix scripts`, `npm test --prefix scripts`, and `node scripts/check-ci-workflows/index.ts`. The workflow checker rejects aggregate gates, conditional jobs, path filters, and missing required commands. Keep TypeScript checks in function directories with colocated Vitest tests. Check changed workflows with `actionlint` when available. Actual check reporting still needs verification on GitHub after publishing. +[Scripts CI](../.github/workflows/scripts-ci.yml) runs type checks, tests, and workflow checks for the maintenance scripts under `scripts/`. Use the [contributor commands](../CONTRIBUTING.md#verify-the-change) to run them locally. The workflow checker rejects jobs that combine results, conditional jobs, path filters, and missing required commands. Keep TypeScript checks in function directories with colocated Vitest tests. Check changed workflows with `actionlint` when available, then inspect their check results on GitHub after publishing. -Node-dependent jobs in Hub, Starter, build-action, and Learning CI use a Node 24 and 26 matrix rather than a separate compatibility workflow. Each check adds a `(Node 26)` suffix on Node 26. A failure does not cancel the other version. npm installation enforces dependency engines, and `scripts/check-ci-workflows/index.ts` checks the supported ranges, matching lockfile metadata, and matrix wiring. These checks track the supported majors, not their exact minimum patch releases. +Hub, Starter, build-action, and Learning CI test both Node.js majors supported by each project's `engines.node`. The workflow matrices define the versions and distinguish their check names. A failure on one version does not cancel checks on the other. npm installation enforces dependency engines. The workflow checker verifies that the manifests, lockfiles, and matrices agree. -Repository and dependency-age tooling run on Node 24. The build-action suite tests its JavaScript and browser capture on both Node versions, but runs the composite-action smoke test only on Node 24: the composite action pins that runtime itself and uploads a single named artifact. +Scripts CI and dependency-age checks use the Node.js version specified in their workflows. The build-action suite tests JavaScript and browser capture on both supported majors. It runs the composite-action smoke test only on the action's pinned runtime to avoid duplicate artifact uploads. ## Update dependencies @@ -62,7 +62,7 @@ From the repository root, `node scripts/check-dependency-age/index.ts` verifies CI does not run `npm audit`: a newly published security fix could otherwise make the audit fail while the age check rejects the fixed version. Dependabot checks dependencies for known vulnerabilities. Organization-managed CodeQL scans application code on GitHub. -When a dependency is too new, select an older compatible version or wait until the seven-day window ends. See [Hub testing and operations](../hub/docs/testing-and-operations.md#update-dependencies) for Hub-specific tooling. +When a dependency is too new, select an older compatible version or wait until the seven-day window ends. See [Hub testing and operations](../hub/docs/testing-and-operations.md) for Hub-specific tooling. ## Copilot code review diff --git a/hub/README.md b/hub/README.md index 29c1169a..fba50194 100644 --- a/hub/README.md +++ b/hub/README.md @@ -2,7 +2,9 @@ Hub is being rebuilt on `build/hub/new` using TypeScript, Express, React, Vite, and Vitest. This branch lets contributors start the new app locally. It is not ready to replace the existing Hub. -The API has one route: `GET /api/health`. The frontend displays the existing Replica logo and supports light and dark themes. Existing components, features, and pages are kept in Storybook, but they are not connected to the new API. PostgreSQL, Prisma, and a slim-based devcontainer provide a development foundation without product models. The old backend, sign-in, saved data, live updates, background jobs, and deployment setup have been removed without replacements. +The API has one route: `GET /api/health`. The frontend displays the existing Replica logo and supports light and dark themes. Existing components, features, and pages are kept in Storybook, but they are not connected to the new API. + +Contributors can use PostgreSQL and Prisma locally or in the devcontainer. The database has no product models yet. Sign-in, saved data, live updates, background jobs, and deployment are not implemented. Use [Development](docs/development.md) to start the API and frontend. Review [Architecture](docs/architecture.md), [code architecture conventions](docs/backend-architecture.md), and [coding conventions](docs/coding-conventions.md) before adding behavior. diff --git a/hub/docs/architecture.md b/hub/docs/architecture.md index 981e8abd..de76ecfe 100644 --- a/hub/docs/architecture.md +++ b/hub/docs/architecture.md @@ -6,7 +6,9 @@ Hub contributors are rebuilding the app on `build/hub/new` before merging it int The team has chosen TypeScript, Express for the API, React and Vite for the frontend, and Vitest for tests. The old backend and the frontend code that connected to it have been removed rather than rewritten. -The API responds to a health check, and the frontend displays the logo with light and dark themes. Storybook keeps the existing components and pages available for reuse. PostgreSQL, Prisma, and a devcontainer provide the database development foundation without product models. Sign-in, deployment, background jobs, and product APIs have not been built. This app cannot yet find repositories, import Builds, host previews, save product data, or support reviews. +The API responds to a health check, and the frontend displays the logo with light and dark themes. Storybook keeps the existing components and pages available for reuse. + +Contributors can use PostgreSQL and Prisma locally or in the devcontainer. The database has no product models yet. Sign-in, deployment, background jobs, and product APIs have not been built. This app cannot yet find repositories, import Builds, host previews, save product data, or support reviews. ## Backend diff --git a/hub/docs/database.md b/hub/docs/database.md index c8050618..335a095a 100644 --- a/hub/docs/database.md +++ b/hub/docs/database.md @@ -1,16 +1,27 @@ # Hub database -Hub uses PostgreSQL and Prisma. The development image is pinned in [Compose](../.devcontainer/compose.yaml); Prisma packages are pinned together in [package.json](../package.json) and follow the [seven-day dependency policy](../../docs/testing-and-review.md#update-dependencies). Product models and migrations have not been added. +Hub uses PostgreSQL and Prisma. Product models and migrations have not been added. + +[Compose](../.devcontainer/compose.yaml) pins the development database image. [package.json](../package.json) pins the Prisma packages to the same version. Dependency updates must follow the [seven-day dependency policy](../../docs/testing-and-review.md#update-dependencies). Choose the newest stable PostgreSQL major available from both Azure Database for PostgreSQL and Amazon RDS or Aurora. Managed services can offer different patch releases; the local image tracks a stable patch of that shared major. Check provider availability again when changing the major, rather than assuming that an upstream release is available everywhere. ## Schema conventions -[prisma7.config.ts](../prisma7.config.ts) points Prisma at the entire `prisma/` directory. Keep the generator and datasource in `prisma/schema.prisma`, one model per kebab-case `.prisma` file, and one enum per kebab-case `.enum.prisma` file. Store migrations in `prisma/migrations/` and commit them with their schema changes. +[prisma7.config.ts](../prisma7.config.ts) loads the entire `prisma/` directory. Organize schema files as follows: + +- Keep the generator and datasource in `prisma/schema.prisma`. +- Put each model in a kebab-case `.prisma` file and each enum in a kebab-case `.enum.prisma` file. +- Store migrations in `prisma/migrations/` and commit them with their schema changes. + +Use singular PascalCase model and enum names. Follow these field and table conventions: -Use singular PascalCase model and enum names. Map every model to a plural `snake_case` table with `@@map`. Use camelCase Prisma fields and `@map` for database columns whose names differ, including foreign keys. Give every table a UUID `id` primary key and `created_at` and `updated_at` timestamp columns. Use PostgreSQL's native UUID type for primary and foreign keys. +- Map every model to a plural `snake_case` table with `@@map`. +- Use camelCase Prisma fields. Map database columns to `snake_case` with `@map` when their names differ, including foreign keys. +- Give every table a UUID `id` primary key and `created_at` and `updated_at` timestamp columns. +- Use PostgreSQL's native UUID type for primary and foreign keys. -Order each model's sections as ID columns, relations, attribute columns, and timestamp columns, with the table mapping last. Keep the section comments when the section is present. For example, a future self-referencing folder model would follow this structure: +Order each model's sections as ID columns, relations, attribute columns, and timestamp columns. Put the table mapping last. Keep comments for sections that are present. This example shows the required structure; it does not define a product model: ```prisma model Folder { @@ -37,7 +48,7 @@ model Folder { ## Shared client -Import `prisma` from `server/integrations/prisma/index.ts` rather than constructing clients in each caller. It uses the PostgreSQL driver adapter and Prisma's [documented global cache pattern](https://www.prisma.io/docs/orm/v7/more/troubleshooting/nextjs#avoiding-multiple-prisma-client-instances): development module reloads reuse one client, while production uses the module instance without writing a development global. +Import `prisma` from `server/integrations/prisma/index.ts` rather than constructing clients in each caller. The integration uses Prisma's [documented global cache pattern](https://www.prisma.io/docs/orm/v7/more/troubleshooting/nextjs#avoiding-multiple-prisma-client-instances) to reuse one client across development module reloads. Production uses the module instance without setting the global variable. The integration requires `DATABASE_URL` and reads `.env` when present. Do not import server database code into the frontend. Keep queries in repositories and follow the [import boundaries](backend-architecture.md#import-boundaries). Domains may pass the shared client into repository functions; repositories must not import integrations directly. diff --git a/hub/docs/development.md b/hub/docs/development.md index 8e690189..b91ecf9f 100644 --- a/hub/docs/development.md +++ b/hub/docs/development.md @@ -6,7 +6,7 @@ Use the Hub devcontainer for Node.js and PostgreSQL, or run Node.js locally with Install a Docker-compatible runtime with Docker Compose and an editor with Dev Containers support. Open the repository's `hub/` directory in the editor and reopen it in the container using `.devcontainer/devcontainer.json`. -Compose starts the app container after PostgreSQL is healthy. The app uses a Node.js `bookworm-slim` base and runs as the non-root `node` user. The official PostgreSQL `bookworm` image also uses a Debian slim base; it has no separate `slim` tag. Container creation runs `npm ci --engine-strict`, including Prisma Client generation. +Compose starts the app container after PostgreSQL is healthy. Both images use Debian slim bases. The app runs as the non-root `node` user. Container creation runs `npm ci --engine-strict`, which also generates Prisma Client. See the [Dockerfile](../.devcontainer/Dockerfile) and [Compose configuration](../.devcontainer/compose.yaml) for the pinned images. The repository is mounted at `/workspaces/replica`, with `hub/` as the working directory. Compose supplies `DATABASE_URL` with the `db` service hostname. Local database credentials in Compose are for development only. The database port is published only on host loopback. @@ -48,7 +48,7 @@ If port 5432 is already in use, stop the conflicting local service or explicitly Follow the [database guide](database.md) for schema conventions, migrations, and the shared client. -`npm test` needs no database. To run database-enabled server tests, start PostgreSQL and run `npm run test:database`. The separate `TEST_DATABASE_URL` in `.env.example` provides a connection with permission to create disposable test databases. See [parallel database tests](testing-and-operations.md#parallel-database-tests) for isolation and cleanup. +`npm test` needs no database. For tests that save records, follow the [database test setup](testing-and-operations.md#parallel-database-tests). ## Build the app diff --git a/hub/docs/testing-and-operations.md b/hub/docs/testing-and-operations.md index f0798048..e7defc4f 100644 --- a/hub/docs/testing-and-operations.md +++ b/hub/docs/testing-and-operations.md @@ -1,6 +1,6 @@ # Hub testing and operations -The new Hub has a PostgreSQL and Prisma foundation but no product models, deployment setup, sign-in, or product APIs yet. These checks cover the new app setup and the frontend code we kept. They do not prove that the old Hub's features work. +These checks cover the Hub API, database setup, and frontend. See [Architecture](architecture.md) for the features that are implemented. Run commands directly from `hub/` after `npm ci`. The database command also needs PostgreSQL and `TEST_DATABASE_URL`, as described in [parallel database tests](#parallel-database-tests): @@ -14,33 +14,44 @@ npm run build npm run storybook:build ``` -`npm test` selects the Node project for API and pure utility tests and the jsdom project for frontend behavior. Neither requires PostgreSQL. A focused API check is `npm test -- server/create-app/index.test.ts`. HTTP tests listen on ephemeral loopback ports and close their servers afterward. +`npm test` runs API and utility tests in Node.js and frontend tests in jsdom, which provides a browser-like environment. Neither requires PostgreSQL. To run a focused API check, use `npm test -- server/create-app/index.test.ts`. HTTP tests listen on temporary loopback ports and close their servers afterward. The API health check proves only that Express responds. Routes that have not been built must return 404 rather than pretending to work. -Installation generates Prisma Client without a database connection. Client lifecycle tests cover reuse across development reloads, production global behavior, and missing configuration. +Installation generates Prisma Client without a database connection. Client tests check reuse across development reloads, production behavior, and errors for missing configuration. ## Clean test cases -[The shared setup](../test/setup.ts) and Vitest configuration reset mocks, spies, stubbed globals, stubbed environment variables, fake timers, and the module cache before every case. The browser setup also clears storage, navigation state, selections, and leftover document content. React Testing Library unmounts rendered components after cases. +[The shared setup](../test/setup.ts) and Vitest configuration reset test state before every case: -Arrange each case's inputs and records inside that case, act through its public interface, and assert the result. Do not depend on records, mocks, or effects left by another case. Use `beforeEach` only for repeated setup, not to hide a case's important inputs. Avoid local `afterEach` and `afterAll` for state already reset globally. +- Reset mocks and restore spies, stubbed globals, and stubbed environment variables. +- Restore real timers and clear the module cache. +- Clear browser storage, navigation state, selections, and leftover document content. -Global resets cannot dispose arbitrary servers, files, sockets, or event listeners. Release resources owned by a case with `try/finally` or Vitest's `onTestFinished`, registered when creating the resource. Module-cache resets affect subsequent imports, not references already imported by a test file; reset any deliberately shared fixture state before each case. +React Testing Library also unmounts rendered components after cases. + +Arrange each case's inputs and records inside that case. Act through its public interface, then assert the result. Do not depend on records, mocks, or effects left by another case. Use `beforeEach` only for repeated setup, not to hide a case's important inputs. Avoid local `afterEach` and `afterAll` for state already reset globally. + +Tests must release resources they create, such as servers, files, sockets, and event listeners. Use `try/finally` or register disposal with Vitest's `onTestFinished` when creating the resource. Clearing the module cache only affects subsequent imports. Reset any deliberately shared fixture state before each case. ## Parallel database tests -Vitest runs files in parallel with its [default forked-process pool](https://vitest.dev/config/pool). The configuration makes this explicit and keeps cases within a worker sequential, including cases marked concurrent, because resetting a shared worker database while another case runs would erase its records. `--maxWorkers` controls file-level parallelism. +Each test run uses separate databases so parallel tests cannot change another worker's records or clear the development database. Vitest runs files in parallel in [forked worker processes](https://vitest.dev/config/pool). Cases within a worker run sequentially, including cases marked concurrent, so database resets cannot erase another case's records. Use `--maxWorkers` to set file-level parallelism. + +Only `npm run test:database` runs database-enabled server tests. Name those tests `server/**/*.db.test.ts`; `npm test` excludes them. Start the PostgreSQL service as described in [Development](development.md) and set `TEST_DATABASE_URL` using `.env.example`. The devcontainer supplies this variable. Use a local database user with permission to create and drop databases, never a production connection. -Only `npm run test:database` selects the database-enabled server project. Name those tests `server/**/*.db.test.ts`; they are excluded from the ordinary Node project. Use the PostgreSQL service in [Development](development.md) and set `TEST_DATABASE_URL` from `.env.example`. The devcontainer supplies its internal service URL. The database user needs permission to create and drop databases; use a local development service, not a production connection. +The database setup follows this lifecycle: -Before workers start, global setup creates a run-unique template database and applies the current Prisma schema with `prisma db push`. It clones that template for every worker slot. Setup uses Vitest's [documented `VITEST_POOL_ID`](https://vitest.dev/guide/migration/jest#envs), which stays within the configured worker limit, rather than the ever-increasing worker instance ID. Each worker receives its own `DATABASE_URL` before application modules load. +1. Before workers start, setup creates a template database with a unique name for the run. It applies the current Prisma schema with `prisma db push`, then copies the template for every worker slot. +2. Each worker receives its own `DATABASE_URL` before application modules load. Setup uses Vitest's [`VITEST_POOL_ID`](https://vitest.dev/guide/migration/jest#envs), which stays within the configured worker limit. +3. Before every case, setup empties application tables across user schemas and resets generated identity counters. It preserves Prisma's migration records. Each case creates its own records; there is no shared seed data. +4. After the run, teardown drops only the databases it created, even if tests left connections open. -Before every case, the database setup truncates application tables across user schemas and restarts their identities. It preserves Prisma's migration records. Tests create their own records in an arrange, act, assert pattern; there is no shared seed data. At the end of the run, global teardown drops only the databases it created, even if tests left connections open. Unique run prefixes also isolate simultaneous Vitest runs. Cleanup rejects database names outside the test namespace and verifies the actual connected database before truncating. +Unique database names also isolate simultaneous runs. Before clearing tables, cleanup checks that the database name belongs to the test namespace and matches the connected database. -The test helpers under `test/` are test infrastructure, not application repositories, integrations, utilities, or domains. Runtime code must not import them. These helpers use direct PostgreSQL access to prepare databases before Prisma Client and application code load. +The helpers under `test/` prepare databases before Prisma Client and application code load. They are test infrastructure, not application repositories, integrations, utilities, or domains. Runtime code must not import them. -Hub CI runs this project against PostgreSQL on every supported Node major. Only that matrix command starts the database service; browser, pure unit, formatting, and build checks do not require it. +Hub CI runs database tests against PostgreSQL on every supported Node major. Only that command starts the database service. ## Shared code diff --git a/scripts/check-ci-workflows/index.ts b/scripts/check-ci-workflows/index.ts index 32d6f0f7..3189987d 100644 --- a/scripts/check-ci-workflows/index.ts +++ b/scripts/check-ci-workflows/index.ts @@ -21,7 +21,7 @@ export function checkWorkflows(root: string): void { ["actions-ci", ["test"]], ["starter-ci", ["test"]], ["learning-ci", ["test"]], - ["repository-ci", ["check"]], + ["scripts-ci", ["check"]], ] as const) { const { jobs, on: triggers } = workflow(name) assert(triggers, `${name}: missing workflow triggers`) @@ -94,18 +94,18 @@ export function checkWorkflows(root: string): void { assert(step, "Composite smoke fixture and action must run") assert.equal(expression(step.if), "matrix.node == 24", "Upload the composite fixture only once") } - const repository = workflow("repository-ci").jobs.check - assert.deepEqual(repository.strategy?.matrix.command, ["typecheck", "test", "check:workflows"]) - assert.equal(repository.strategy?.["fail-fast"], false) + const scripts = workflow("scripts-ci").jobs.check + assert.deepEqual(scripts.strategy?.matrix.command, ["typecheck", "test", "check:workflows"]) + assert.equal(scripts.strategy?.["fail-fast"], false) assert( - repository.steps?.some( + scripts.steps?.some( (step) => step.run === "npm ci --prefix scripts --ignore-scripts --engine-strict", ), - "Repository checks must install tooling", + "Scripts checks must install tooling", ) assert( - repository.steps?.some((step) => step.run === "npm run ${{ matrix.command }} --prefix scripts"), - "Repository checks must run each tooling command", + scripts.steps?.some((step) => step.run === "npm run ${{ matrix.command }} --prefix scripts"), + "Scripts checks must run each tooling command", ) const ageSteps = workflow("dependency-age").jobs.audit.steps assert( From 1c6f55ea6431f9ba9ac751b11c32962549d5acde Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:22:48 +0000 Subject: [PATCH 4/6] test(hub): simplify setup coverage and allow empty database runs Move Prisma schema and migrations to schema/. Co-authored-by: Nishiki Liu Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- hub/app/javascript/test/setup.test.ts | 34 ------ hub/docs/database.md | 6 +- hub/docs/testing-and-operations.md | 2 + hub/package.json | 2 +- hub/prisma7.config.ts | 4 +- hub/{prisma => schema}/schema.prisma | 0 .../integrations/prisma/isolation.db.test.ts | 111 ------------------ .../prisma/parallel-isolation.db.test.ts | 40 ------- 8 files changed, 8 insertions(+), 191 deletions(-) delete mode 100644 hub/app/javascript/test/setup.test.ts rename hub/{prisma => schema}/schema.prisma (100%) delete mode 100644 hub/server/integrations/prisma/isolation.db.test.ts delete mode 100644 hub/server/integrations/prisma/parallel-isolation.db.test.ts diff --git a/hub/app/javascript/test/setup.test.ts b/hub/app/javascript/test/setup.test.ts deleted file mode 100644 index 19523191..00000000 --- a/hub/app/javascript/test/setup.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { describe, expect, it } from "vitest" - -const initialLocation = window.location.href -const initialHistoryState: unknown = window.history.state - -describe("frontend test setup", () => { - it.each(["first case", "second case"])( - "gives the %s clean storage, navigation, selection and document body", - () => { - expect(window.localStorage.length).toBe(0) - expect(window.sessionStorage.length).toBe(0) - expect(window.location.href).toBe(initialLocation) - expect(window.history.state).toEqual(initialHistoryState) - expect(window.getSelection()?.rangeCount).toBe(0) - expect(document.body.childNodes.length).toBe(0) - expect(document.body.hasAttribute("style")).toBe(false) - - window.localStorage.setItem("replica-setup-test", "contaminated") - window.sessionStorage.setItem("replica-setup-test", "contaminated") - window.history.replaceState( - { replicaSetupTest: "contaminated" }, - "", - "/setup-contamination?dirty=1#selection", - ) - const leftover = document.createElement("p") - leftover.textContent = "Selected content from another case" - document.body.append(leftover) - document.body.style.overflow = "hidden" - const range = document.createRange() - range.selectNodeContents(leftover) - window.getSelection()?.addRange(range) - }, - ) -}) diff --git a/hub/docs/database.md b/hub/docs/database.md index 335a095a..8f288971 100644 --- a/hub/docs/database.md +++ b/hub/docs/database.md @@ -8,11 +8,11 @@ Choose the newest stable PostgreSQL major available from both Azure Database for ## Schema conventions -[prisma7.config.ts](../prisma7.config.ts) loads the entire `prisma/` directory. Organize schema files as follows: +[prisma7.config.ts](../prisma7.config.ts) loads the entire `schema/` directory. Organize schema files as follows: -- Keep the generator and datasource in `prisma/schema.prisma`. +- Keep the generator and datasource in `schema/schema.prisma`. - Put each model in a kebab-case `.prisma` file and each enum in a kebab-case `.enum.prisma` file. -- Store migrations in `prisma/migrations/` and commit them with their schema changes. +- Store migrations in `schema/migrations/` and commit them with their schema changes. Use singular PascalCase model and enum names. Follow these field and table conventions: diff --git a/hub/docs/testing-and-operations.md b/hub/docs/testing-and-operations.md index e7defc4f..22ef9643 100644 --- a/hub/docs/testing-and-operations.md +++ b/hub/docs/testing-and-operations.md @@ -40,6 +40,8 @@ Each test run uses separate databases so parallel tests cannot change another wo Only `npm run test:database` runs database-enabled server tests. Name those tests `server/**/*.db.test.ts`; `npm test` excludes them. Start the PostgreSQL service as described in [Development](development.md) and set `TEST_DATABASE_URL` using `.env.example`. The devcontainer supplies this variable. Use a local database user with permission to create and drop databases, never a production connection. +The command uses `--passWithNoTests` so an empty project exits successfully. It still fails when tests fail. There are no database test cases yet; an empty run does not prepare or exercise the databases. + The database setup follows this lifecycle: 1. Before workers start, setup creates a template database with a unique name for the run. It applies the current Prisma schema with `prisma db push`, then copies the template for every worker slot. diff --git a/hub/package.json b/hub/package.json index 57920de7..b5b5eb15 100644 --- a/hub/package.json +++ b/hub/package.json @@ -34,7 +34,7 @@ "storybook": "STORYBOOK_DISABLE_TELEMETRY=1 storybook dev --host 0.0.0.0 --port 6006 --initial-path /story/hub-design-overview--taxonomy", "storybook:build": "STORYBOOK_DISABLE_TELEMETRY=1 storybook build --output-dir storybook-static", "test": "vitest run --project=node --project=browser", - "test:database": "vitest run --project=database", + "test:database": "vitest run --project=database --passWithNoTests", "typecheck": "tsc --noEmit && tsc --project ../.github/extensions/replica-prototype/tsconfig.json" }, "dependencies": { diff --git a/hub/prisma7.config.ts b/hub/prisma7.config.ts index a17c9662..5f699bd0 100644 --- a/hub/prisma7.config.ts +++ b/hub/prisma7.config.ts @@ -2,7 +2,7 @@ import "dotenv/config" import { defineConfig } from "prisma/config" export default defineConfig({ - schema: "prisma", - migrations: { path: "prisma/migrations" }, + schema: "schema", + migrations: { path: "schema/migrations" }, datasource: { url: process.env.DATABASE_URL }, }) diff --git a/hub/prisma/schema.prisma b/hub/schema/schema.prisma similarity index 100% rename from hub/prisma/schema.prisma rename to hub/schema/schema.prisma diff --git a/hub/server/integrations/prisma/isolation.db.test.ts b/hub/server/integrations/prisma/isolation.db.test.ts deleted file mode 100644 index 1de2546c..00000000 --- a/hub/server/integrations/prisma/isolation.db.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { describe, expect, inject, it } from "vitest" - -import { resetDatabase } from "../../../test/database/reset-database" - -import { prisma } from "./index" - -describe("Prisma database isolation", () => { - it.each(["first case", "second case"])( - "starts the %s empty in its assigned worker database", - async () => { - try { - const database = inject("testDatabase") - const [connection] = await prisma.$queryRaw<{ name: string }[]>` - SELECT current_database() AS name - ` - expect(connection?.name).toBe(`${database.prefix}_${process.env.VITEST_POOL_ID}`) - - await prisma.$executeRaw` - CREATE TABLE IF NOT EXISTS public.test_worker_isolation ( - id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, - unique_key text NOT NULL UNIQUE - ) - ` - const [before] = await prisma.$queryRaw<{ count: bigint }[]>` - SELECT count(*) FROM public.test_worker_isolation - ` - expect(before?.count).toBe(0n) - - await prisma.$executeRaw` - INSERT INTO public.test_worker_isolation (unique_key) VALUES ('shared-key') - ` - - const records = await prisma.$queryRaw<{ id: number; unique_key: string }[]>` - SELECT id, unique_key FROM public.test_worker_isolation - ` - expect(records).toEqual([{ id: 1, unique_key: "shared-key" }]) - } finally { - await prisma.$disconnect() - } - }, - ) - - it("resets custom-schema tables, foreign keys and identities without deleting migrations", async () => { - try { - await prisma.$executeRaw`CREATE SCHEMA IF NOT EXISTS test_isolation` - await prisma.$executeRaw` - CREATE TABLE IF NOT EXISTS public.test_reset_parents ( - id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY - ) - ` - await prisma.$executeRaw` - CREATE TABLE IF NOT EXISTS test_isolation.children ( - id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, - parent_id integer NOT NULL REFERENCES public.test_reset_parents(id) - ) - ` - await prisma.$executeRaw` - CREATE TABLE IF NOT EXISTS public._prisma_migrations ( - id varchar(36) PRIMARY KEY, - checksum varchar(64) NOT NULL, - finished_at timestamptz, - migration_name varchar(255) NOT NULL, - logs text, - rolled_back_at timestamptz, - started_at timestamptz NOT NULL DEFAULT now(), - applied_steps_count integer NOT NULL DEFAULT 0 - ) - ` - await prisma.$executeRaw` - INSERT INTO public._prisma_migrations (id, checksum, migration_name) - VALUES ('test-reset-migration', 'test-checksum', 'test-only-reset-marker') - ` - const [parent] = await prisma.$queryRaw<{ id: number }[]>` - INSERT INTO public.test_reset_parents DEFAULT VALUES RETURNING id - ` - await prisma.$executeRaw` - INSERT INTO test_isolation.children (parent_id) VALUES (${parent!.id}) - ` - - await resetDatabase(process.env.DATABASE_URL!) - - const [counts] = await prisma.$queryRaw<{ parents: bigint; children: bigint }[]>` - SELECT - (SELECT count(*) FROM public.test_reset_parents) AS parents, - (SELECT count(*) FROM test_isolation.children) AS children - ` - expect(counts).toEqual({ parents: 0n, children: 0n }) - const migrations = await prisma.$queryRaw<{ migration_name: string }[]>` - SELECT migration_name FROM public._prisma_migrations WHERE id = 'test-reset-migration' - ` - expect(migrations).toEqual([{ migration_name: "test-only-reset-marker" }]) - - const parents = await prisma.$queryRaw<{ id: number }[]>` - INSERT INTO public.test_reset_parents DEFAULT VALUES RETURNING id - ` - const children = await prisma.$queryRaw<{ id: number }[]>` - INSERT INTO test_isolation.children (parent_id) VALUES (${parents[0]!.id}) RETURNING id - ` - expect(parents).toEqual([{ id: 1 }]) - expect(children).toEqual([{ id: 1 }]) - } finally { - try { - await prisma.$executeRaw` - DELETE FROM public._prisma_migrations WHERE id = 'test-reset-migration' - ` - } finally { - await prisma.$disconnect() - } - } - }) -}) diff --git a/hub/server/integrations/prisma/parallel-isolation.db.test.ts b/hub/server/integrations/prisma/parallel-isolation.db.test.ts deleted file mode 100644 index 7d75f999..00000000 --- a/hub/server/integrations/prisma/parallel-isolation.db.test.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { describe, expect, inject, it } from "vitest" - -import { prisma } from "./index" - -describe("Prisma parallel database isolation", () => { - it.each(["first case", "second case"])( - "starts the %s empty without sharing another file's records", - async () => { - try { - const database = inject("testDatabase") - const [connection] = await prisma.$queryRaw<{ name: string }[]>` - SELECT current_database() AS name - ` - expect(connection?.name).toBe(`${database.prefix}_${process.env.VITEST_POOL_ID}`) - - await prisma.$executeRaw` - CREATE TABLE IF NOT EXISTS public.test_worker_isolation ( - id integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, - unique_key text NOT NULL UNIQUE - ) - ` - const [before] = await prisma.$queryRaw<{ count: bigint }[]>` - SELECT count(*) FROM public.test_worker_isolation - ` - expect(before?.count).toBe(0n) - - await prisma.$executeRaw` - INSERT INTO public.test_worker_isolation (unique_key) VALUES ('shared-key') - ` - - const records = await prisma.$queryRaw<{ id: number; unique_key: string }[]>` - SELECT id, unique_key FROM public.test_worker_isolation - ` - expect(records).toEqual([{ id: 1, unique_key: "shared-key" }]) - } finally { - await prisma.$disconnect() - } - }, - ) -}) From 91b915a63e09590ce50f72f7a595c317524b516b Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:25:51 +0000 Subject: [PATCH 5/6] test(hub): remove redundant shared setup coverage Co-authored-by: Nishiki Liu Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- hub/test/setup.test.ts | 31 ------------------------------- 1 file changed, 31 deletions(-) delete mode 100644 hub/test/setup.test.ts diff --git a/hub/test/setup.test.ts b/hub/test/setup.test.ts deleted file mode 100644 index 1bc14fe4..00000000 --- a/hub/test/setup.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { describe, expect, it, vi } from "vitest" - -const environmentKey = "REPLICA_SHARED_SETUP_TEST" -const globalKey = "__replicaSharedSetupTest" -const initialEnvironment = process.env[environmentKey] -const initialGlobal: unknown = Reflect.get(globalThis, globalKey) -const mock = vi.fn((value: string) => value) - -describe("shared test setup", () => { - it.each(["first case", "second case"])( - "gives the %s clean timers, environment, globals, mocks and modules", - async () => { - expect(vi.isFakeTimers()).toBe(false) - expect(process.env[environmentKey]).toBe(initialEnvironment) - expect(Reflect.get(globalThis, globalKey)).toBe(initialGlobal) - expect(mock).not.toHaveBeenCalled() - expect(mock("fresh implementation")).toBe("fresh implementation") - - const { getWorkerDatabase } = await import("./database/get-worker-database") - expect(Reflect.has(getWorkerDatabase, globalKey)).toBe(false) - - vi.useFakeTimers() - setTimeout(() => mock("pending timer"), 1_000) - vi.stubEnv(environmentKey, "contaminated") - vi.stubGlobal(globalKey, "contaminated") - mock.mockReturnValue("contaminated") - mock("changed implementation") - Reflect.set(getWorkerDatabase, globalKey, "contaminated") - }, - ) -}) From 3157c139012428d168f7a549a39f4d91563bdd8d Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:38:08 +0000 Subject: [PATCH 6/6] test(hub): compose database access with both test environments Co-authored-by: Nishiki Liu Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- hub/.env.example | 2 +- hub/AGENTS.md | 2 +- hub/docs/coding-conventions.md | 2 +- hub/docs/database.md | 2 +- hub/docs/testing-and-operations.md | 19 ++++++++-- hub/package.json | 2 +- hub/vitest.config.ts | 60 ++++++++++++++++++------------ 7 files changed, 57 insertions(+), 32 deletions(-) diff --git a/hub/.env.example b/hub/.env.example index aaa77d99..35e842d8 100644 --- a/hub/.env.example +++ b/hub/.env.example @@ -4,5 +4,5 @@ PORT=3001 # Local development only. The devcontainer supplies its own database hostname. DATABASE_URL=postgresql://replica:replica@127.0.0.1:5432/replica -# Only the database test project requires this URL and CREATEDB permission. +# Only database-enabled tests require this URL and CREATEDB permission. TEST_DATABASE_URL=postgresql://replica:replica@127.0.0.1:5432/postgres diff --git a/hub/AGENTS.md b/hub/AGENTS.md index bef48a11..afced341 100644 --- a/hub/AGENTS.md +++ b/hub/AGENTS.md @@ -11,7 +11,7 @@ Hub is being rebuilt in TypeScript. The app starts locally, but its product feat Run Node commands from `hub/`, locally or inside the devcontainer. PostgreSQL and Prisma are configured without product models. Agree on the design before adding models, sign-in, deployment, or product API routes. -`npm test` is database-free. Use `npm run test:database` for `.db.test.ts` server cases, with the worker-specific databases and shared reset lifecycle described in [testing](docs/testing-and-operations.md). +`npm test` does not require PostgreSQL. Use `npm run test:database` for `.db.test.ts` and `.db.test.tsx` cases in Node.js or jsdom. Follow the database isolation and reset rules in [testing](docs/testing-and-operations.md). Keep the top-level `HubThemeProvider` around Hub and Storybook content. It synchronizes the resolved light/dark mode to `` so portals inherit Brand and `--hub-*` tokens. Do not add a fixed light theme to an overlay. diff --git a/hub/docs/coding-conventions.md b/hub/docs/coding-conventions.md index 8ae64a40..31360d5d 100644 --- a/hub/docs/coding-conventions.md +++ b/hub/docs/coding-conventions.md @@ -33,4 +33,4 @@ Preserve the existing `app/javascript/{application,pages,features,components,hoo Keep tests beside their TypeScript code. Use `describe("functionName()")` and plain-English cases. Keep fixtures and mutable state inside each case; use the global Vitest setup and configuration for shared environment cleanup and automatic mock resets. -Follow the [test lifecycle](testing-and-operations.md#clean-test-cases): arrange each case's records, act, and assert. Database-enabled server tests use `.db.test.ts` and the dedicated database project. Do not add local cleanup hooks for state already reset globally. +Follow the [test lifecycle](testing-and-operations.md#clean-test-cases): arrange each case's records, act, and assert. Use `.db.test.ts` or `.db.test.tsx` to opt into database access in Node.js or jsdom. Do not add local cleanup hooks for state already reset globally. diff --git a/hub/docs/database.md b/hub/docs/database.md index 8f288971..6cb52918 100644 --- a/hub/docs/database.md +++ b/hub/docs/database.md @@ -66,4 +66,4 @@ npm run db:generate Use `db:migrate` only with a local development database. It needs permission to create a shadow database to calculate migrations. `npm run db:deploy` applies committed migrations without creating new ones; it is not a production deployment setup. `npm run db:studio` opens Prisma's database editor and should be used only with an intended development database. -The [database test project](testing-and-operations.md#parallel-database-tests) uses separate databases for every worker and test run. It applies the current Prisma schema directly, clears records before each case, and does not use the development database for test data. +The [database-enabled test projects](testing-and-operations.md#parallel-database-tests) use separate databases for every project, worker, and test run. Setup applies the current Prisma schema directly and clears records before each case. Tests do not use the development database for records. diff --git a/hub/docs/testing-and-operations.md b/hub/docs/testing-and-operations.md index 22ef9643..4730dc20 100644 --- a/hub/docs/testing-and-operations.md +++ b/hub/docs/testing-and-operations.md @@ -38,13 +38,26 @@ Tests must release resources they create, such as servers, files, sockets, and e Each test run uses separate databases so parallel tests cannot change another worker's records or clear the development database. Vitest runs files in parallel in [forked worker processes](https://vitest.dev/config/pool). Cases within a worker run sequentially, including cases marked concurrent, so database resets cannot erase another case's records. Use `--maxWorkers` to set file-level parallelism. -Only `npm run test:database` runs database-enabled server tests. Name those tests `server/**/*.db.test.ts`; `npm test` excludes them. Start the PostgreSQL service as described in [Development](development.md) and set `TEST_DATABASE_URL` using `.env.example`. The devcontainer supplies this variable. Use a local database user with permission to create and drop databases, never a production connection. +Execution environment and database access are separate choices: -The command uses `--passWithNoTests` so an empty project exits successfully. It still fails when tests fail. There are no database test cases yet; an empty run does not prepare or exercise the databases. +| Environment | Without PostgreSQL | With PostgreSQL | +| ----------- | ------------------ | ------------------ | +| Node.js | `node` | `node-database` | +| jsdom | `browser` | `browser-database` | + +Use `.test.ts` or `.test.tsx` for ordinary cases. Use `.db.test.ts` or `.db.test.tsx` to opt into database access. Server and test-infrastructure files run in Node.js. Frontend and Storybook files run in jsdom, except the utility cases listed in `vitest.config.ts` for Node.js. Each file runs in exactly one project. + +`npm test` selects `node` and `browser` without requiring PostgreSQL. `npm run test:database` selects both database-enabled projects. To select one project, run `npx vitest run --project=node-database --passWithNoTests` or use `--project=browser-database` instead. + +Start the PostgreSQL service as described in [Development](development.md) and set `TEST_DATABASE_URL` using `.env.example`. The devcontainer supplies this variable. Use a local database user with permission to create and drop databases, never a production connection. + +The database command uses `--passWithNoTests` so an empty selection exits successfully. It still fails when tests fail. There are no database test cases yet; an empty run does not prepare or exercise the databases. + +Database-enabled jsdom tests use the browser setup and database hooks together. They can arrange records through server code, but frontend code must not import server modules. The setup does not start an API or connect frontend requests to one. The database setup follows this lifecycle: -1. Before workers start, setup creates a template database with a unique name for the run. It applies the current Prisma schema with `prisma db push`, then copies the template for every worker slot. +1. Before workers start, setup creates a template database with a unique name for each project and run. It applies the current Prisma schema with `prisma db push`, then copies the template for every worker slot. 2. Each worker receives its own `DATABASE_URL` before application modules load. Setup uses Vitest's [`VITEST_POOL_ID`](https://vitest.dev/guide/migration/jest#envs), which stays within the configured worker limit. 3. Before every case, setup empties application tables across user schemas and resets generated identity counters. It preserves Prisma's migration records. Each case creates its own records; there is no shared seed data. 4. After the run, teardown drops only the databases it created, even if tests left connections open. diff --git a/hub/package.json b/hub/package.json index b5b5eb15..ba09fc60 100644 --- a/hub/package.json +++ b/hub/package.json @@ -34,7 +34,7 @@ "storybook": "STORYBOOK_DISABLE_TELEMETRY=1 storybook dev --host 0.0.0.0 --port 6006 --initial-path /story/hub-design-overview--taxonomy", "storybook:build": "STORYBOOK_DISABLE_TELEMETRY=1 storybook build --output-dir storybook-static", "test": "vitest run --project=node --project=browser", - "test:database": "vitest run --project=database --passWithNoTests", + "test:database": "vitest run --project=node-database --project=browser-database --passWithNoTests", "typecheck": "tsc --noEmit && tsc --project ../.github/extensions/replica-prototype/tsconfig.json" }, "dependencies": { diff --git a/hub/vitest.config.ts b/hub/vitest.config.ts index 463a323a..23b7c7e4 100644 --- a/hub/vitest.config.ts +++ b/hub/vitest.config.ts @@ -46,35 +46,47 @@ export default defineConfig({ server: { deps: { inline: [/@primer\//] } }, projects: [ { - extends: true, - test: { - name: "node", - environment: "node", - include: [...nodeTests, "server/**/*.test.ts", "test/**/*.test.ts"], - exclude: [...configDefaults.exclude, "**/*.db.test.ts"], - }, + name: "node", + environment: "node", + include: [...nodeTests, "server/**/*.test.{ts,tsx}", "test/**/*.test.{ts,tsx}"], + exclude: [], + setupFiles: [], }, { - extends: true, - test: { - name: "browser", - environment: "jsdom", - include: ["app/javascript/**/*.test.{ts,tsx}", ".storybook/**/*.test.{ts,tsx}"], - exclude: [...configDefaults.exclude, ...nodeTests], - setupFiles: ["./app/javascript/test/setup.ts"], - }, + name: "browser", + environment: "jsdom", + include: ["app/javascript/**/*.test.{ts,tsx}", ".storybook/**/*.test.{ts,tsx}"], + exclude: nodeTests, + setupFiles: ["./app/javascript/test/setup.ts"], }, - { + ].flatMap(({ name, ...environment }) => + [false, true].map((database) => ({ extends: true, test: { - name: "database", - environment: "node", - include: ["server/**/*.db.test.ts"], - globalSetup: ["./test/database/global-setup/index.ts"], - setupFiles: ["./test/database/setup.ts"], - hookTimeout: 30_000, + ...environment, + name: database ? `${name}-database` : name, + include: environment.include.map((pattern) => + database ? pattern.replace(".test.", ".db.test.") : pattern, + ), + exclude: [ + ...configDefaults.exclude, + ...environment.exclude.map((pattern) => + database ? pattern.replace(".test.", ".db.test.") : pattern, + ), + ...(database ? [] : ["**/*.db.test.{ts,tsx}"]), + ], + setupFiles: [ + ...environment.setupFiles, + ...(database ? ["./test/database/setup.ts"] : []), + ], + ...(database + ? { + globalSetup: ["./test/database/global-setup/index.ts"], + hookTimeout: 30_000, + } + : {}), }, - }, - ], + })), + ), }, })