diff --git a/apps/web/playwright.config.ts b/apps/web/playwright.config.ts index 3af26302..3c2eb0a5 100644 --- a/apps/web/playwright.config.ts +++ b/apps/web/playwright.config.ts @@ -1,13 +1,14 @@ import { defineConfig, devices } from "@playwright/test"; -const port = Number(process.env.DIFFX_TEST_PORT ?? 4173); - export default defineConfig({ - workers: 1, + // Workers own isolated fixture servers (test/browser/fixtures.ts). + fullyParallel: true, + // Leave headroom for Pierre rendering and concurrent Go checks. + workers: "33%", + globalSetup: "./test/browser/global-setup.ts", testDir: "./test/browser", outputDir: "test-results", use: { - baseURL: `http://127.0.0.1:${port}`, trace: "retain-on-failure", }, projects: [ @@ -16,10 +17,4 @@ export default defineConfig({ use: { ...devices["Desktop Chrome"] }, }, ], - webServer: { - command: `pnpm dev --port ${port}`, - url: `http://127.0.0.1:${port}`, - reuseExistingServer: !process.env.CI, - timeout: 120_000, - }, }); diff --git a/apps/web/test/browser/app.spec.ts b/apps/web/test/browser/app.spec.ts index 81abd0bd..71445e3b 100644 --- a/apps/web/test/browser/app.spec.ts +++ b/apps/web/test/browser/app.spec.ts @@ -1,4 +1,5 @@ -import { expect, test, type Page } from "@playwright/test"; +import { type Page } from "@playwright/test"; +import { expect, test } from "./fixtures.ts"; import { resetFixtureState } from "./fixture-state.ts"; async function resolvedRadius( @@ -812,6 +813,8 @@ test("mobile keeps review progress visible beside the diff", async ({ test("inline comments use a distinct structured surface while sidebar comments remain cards", async ({ page, }) => { + // Rendering every file in a 4000px viewport takes ~20s even serially. + test.slow(); // 1280px of diff chrome beside the expanded projects drawer. await page.setViewportSize({ width: 1536, height: 4_000 }); await page.goto("/"); diff --git a/apps/web/test/browser/async-recovery.spec.ts b/apps/web/test/browser/async-recovery.spec.ts index 71362a65..e4970453 100644 --- a/apps/web/test/browser/async-recovery.spec.ts +++ b/apps/web/test/browser/async-recovery.spec.ts @@ -1,4 +1,5 @@ -import { expect, test, type Page, type Route } from "@playwright/test"; +import { type Page, type Route } from "@playwright/test"; +import { expect, test } from "./fixtures.ts"; import { createApiClient, type ApiRepositoryDiff } from "@diffx/api"; import type { ReviewComment } from "@diffx/shared"; diff --git a/apps/web/test/browser/design-refresh.spec.ts b/apps/web/test/browser/design-refresh.spec.ts index d519d2fb..1c32ecb3 100644 --- a/apps/web/test/browser/design-refresh.spec.ts +++ b/apps/web/test/browser/design-refresh.spec.ts @@ -1,4 +1,5 @@ -import { expect, test, type Page } from "@playwright/test"; +import { type Page } from "@playwright/test"; +import { expect, test } from "./fixtures.ts"; import { resetFixtureState } from "./fixture-state.ts"; test.beforeEach(async ({ request }) => resetFixtureState(request)); diff --git a/apps/web/test/browser/fixtures.ts b/apps/web/test/browser/fixtures.ts new file mode 100644 index 00000000..85ca573a --- /dev/null +++ b/apps/web/test/browser/fixtures.ts @@ -0,0 +1,53 @@ +import { test as base } from "@playwright/test"; +import { fileURLToPath } from "node:url"; +import { preview } from "vite"; +import { startFixtureProcess } from "../fixture-server.ts"; + +export { expect } from "@playwright/test"; + +type FixtureBuild = { binary: string | undefined; webDir: string | undefined }; + +/** + * Each worker owns one in-memory fixture server behind a Vite preview of the + * built app, which keeps SPA fallback for account routes. + */ +export const test = base.extend< + object, + { fixtureBuild: FixtureBuild; fixtureServer: string } +>({ + // Workers start after global setup, so its build paths are in the environment. + fixtureBuild: [ + { + binary: process.env.DIFFX_FIXTURE_BINARY, + webDir: process.env.DIFFX_FIXTURE_WEB_DIR, + }, + { scope: "worker", option: true }, + ], + fixtureServer: [ + async ({ fixtureBuild: { binary, webDir } }, use) => { + if (!binary || !webDir) throw new Error("Missing browser test build"); + const fixture = await startFixtureProcess({ binary }); + // vite.config.ts proxies the API to this worker's fixture. + process.env.DIFFX_API_URL = fixture.target; + const server = await preview({ + root: fileURLToPath(new URL("../..", import.meta.url)), + logLevel: "warn", + build: { outDir: webDir }, + preview: { host: "127.0.0.1", port: 0 }, + }).catch((error: unknown) => { + fixture.stop(); + throw error; + }); + const url = server.resolvedUrls?.local[0]; + try { + if (!url) throw new Error("Missing browser test server URL"); + await use(url.replace(/\/$/, "")); + } finally { + await server.close(); + fixture.stop(); + } + }, + { scope: "worker" }, + ], + baseURL: async ({ fixtureServer }, use) => use(fixtureServer), +}); diff --git a/apps/web/test/browser/global-setup.ts b/apps/web/test/browser/global-setup.ts new file mode 100644 index 00000000..45a75f3a --- /dev/null +++ b/apps/web/test/browser/global-setup.ts @@ -0,0 +1,29 @@ +import { execFile } from "node:child_process"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; +import { build } from "vite"; +import { repositoryRoot } from "../fixture-server.ts"; + +// Build the web app and CLI once; each worker then starts its own fixture +// server from them, so tests never share review state across workers. +export default async function globalSetup() { + const directory = await mkdtemp(join(tmpdir(), "diffx-browser-")); + const webDir = join(directory, "web"); + const binary = join(directory, "diffx"); + await Promise.all([ + build({ + root: fileURLToPath(new URL("../..", import.meta.url)), + logLevel: "warn", + build: { outDir: webDir, emptyOutDir: true }, + }), + promisify(execFile)("go", ["build", "-o", binary, "./cmd/diffx"], { + cwd: repositoryRoot, + }), + ]); + process.env.DIFFX_FIXTURE_BINARY = binary; + process.env.DIFFX_FIXTURE_WEB_DIR = webDir; + return () => rm(directory, { recursive: true, force: true }); +} diff --git a/apps/web/test/browser/identity.spec.ts b/apps/web/test/browser/identity.spec.ts index c3944e90..c2e076c8 100644 --- a/apps/web/test/browser/identity.spec.ts +++ b/apps/web/test/browser/identity.spec.ts @@ -1,4 +1,5 @@ -import { test, expect, type Page } from "@playwright/test"; +import { type Page } from "@playwright/test"; +import { expect, test } from "./fixtures.ts"; const account = { id: "account-a", username: "alice" }; function bootstrap(signedIn: boolean, signupEnabled = false) { diff --git a/apps/web/test/browser/local-workspace.spec.ts b/apps/web/test/browser/local-workspace.spec.ts index 90b48615..7dbeafb4 100644 --- a/apps/web/test/browser/local-workspace.spec.ts +++ b/apps/web/test/browser/local-workspace.spec.ts @@ -1,4 +1,5 @@ -import { expect, test, type Page, type Route } from "@playwright/test"; +import { type Page, type Route } from "@playwright/test"; +import { expect, test } from "./fixtures.ts"; import type { ApiContext, ApiRepositoryDiff } from "@diffx/api"; import { isDiffMode, type DiffMode, type ReviewComment } from "@diffx/shared"; diff --git a/apps/web/test/browser/native-scrollbars.spec.ts b/apps/web/test/browser/native-scrollbars.spec.ts index d82d9426..46b4e348 100644 --- a/apps/web/test/browser/native-scrollbars.spec.ts +++ b/apps/web/test/browser/native-scrollbars.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test } from "./fixtures.ts"; import { resetFixtureState } from "./fixture-state.ts"; test.use({ launchOptions: { ignoreDefaultArgs: ["--hide-scrollbars"] } }); diff --git a/apps/web/test/fixture-server.ts b/apps/web/test/fixture-server.ts index 12c57724..58a2272f 100644 --- a/apps/web/test/fixture-server.ts +++ b/apps/web/test/fixture-server.ts @@ -19,16 +19,21 @@ function stopProcess(child: ChildProcess) { } } -export async function startFixtureServer(): Promise<{ - target: string; - plugin: Plugin; -}> { - const root = fileURLToPath(new URL("../../..", import.meta.url)); +export const repositoryRoot = fileURLToPath( + new URL("../../..", import.meta.url), +); + +/** + * Starts an isolated in-memory fixture server. Browser tests pass a prebuilt + * binary and built web assets; Vite development compiles the CLI instead. + */ +export async function startFixtureProcess( + options: { binary?: string; webDir?: string } = {}, +): Promise<{ target: string; stop: () => void }> { const child = spawn( - "go", + options.binary ?? "go", [ - "run", - "./cmd/diffx", + ...(options.binary ? [] : ["run", "./cmd/diffx"]), "dev", "--fixture", "test/fixtures/sample.diff", @@ -37,9 +42,10 @@ export async function startFixtureServer(): Promise<{ "--no-browser", "--port", "0", + ...(options.webDir ? ["--web-dir", options.webDir] : []), ], { - cwd: root, + cwd: repositoryRoot, detached: process.platform !== "win32", env: { ...process.env, DIFFX_EXIT_ON_STDIN_CLOSE: "1" }, stdio: ["pipe", "pipe", "inherit"], @@ -81,7 +87,14 @@ export async function startFixtureServer(): Promise<{ resolve(`http://127.0.0.1:${port}`); }); }); - const stop = () => stopProcess(child); + return { target, stop: () => stopProcess(child) }; +} + +export async function startFixtureServer(): Promise<{ + target: string; + plugin: Plugin; +}> { + const { target, stop } = await startFixtureProcess(); const interrupt = () => { stop(); process.exit(130); diff --git a/cmd/diffx/ingestion_test.go b/cmd/diffx/ingestion_test.go index b36b63f0..05df4c20 100644 --- a/cmd/diffx/ingestion_test.go +++ b/cmd/diffx/ingestion_test.go @@ -2,12 +2,14 @@ package main import ( "bytes" + "context" "io" "os" "os/exec" "path/filepath" "strings" "testing" + "time" "github.com/flexdinesh/diffx/internal/ingestion" ) @@ -100,15 +102,42 @@ func TestCollectorOptionsRejectAmbiguousInputsBeforeStartup(t *testing.T) { {"sync", "--host", "0.0.0.0"}, {"service", "start", "--server", "http://example.test"}, } + // Isolate from a developer's running diffx; a regression that starts a + // server fails at the deadline instead of hanging the suite. + t.Chdir(t.TempDir()) + t.Setenv("DIFFX_CONFIG_PATH", filepath.Join(t.TempDir(), "config.json")) + t.Setenv("DIFFX_RUNTIME_DIR", t.TempDir()) + t.Setenv("XDG_STATE_HOME", t.TempDir()) for _, args := range cases { t.Run(strings.Join(args, " "), func(t *testing.T) { - if err := run(t.Context(), args, nil, io.Discard, io.Discard); err == nil { - t.Fatalf("accepted %v", args) + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + err := run(ctx, args, nil, io.Discard, io.Discard) + if err == nil || ctx.Err() != nil { + t.Fatalf("accepted %v: %v", args, err) } }) } } +func TestLocalInputRejectsMissingCheckoutPathBeforeStartup(t *testing.T) { + root := t.TempDir() + file := filepath.Join(root, "file.txt") + if err := os.WriteFile(file, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + for _, path := range []string{filepath.Join(root, "missing"), file} { + values := options{directory: path, sourceID: "test-source", trigger: "manual"} + if _, err := acquireLocalInput(values, nil); err == nil || !strings.Contains(err.Error(), path) { + t.Fatalf("accepted checkout path %s: %v", path, err) + } + } + values := options{directory: root, paths: []string{root, filepath.Join(root, "missing")}, sourceID: "test-source", trigger: "manual"} + if _, err := acquireLocalInput(values, nil); err == nil { + t.Fatal("accepted a missing repeated checkout path") + } +} + func TestHelpDoesNotExposeEnvironmentCredentials(t *testing.T) { t.Setenv("DIFFX_REMOTE_TOKEN", "secret-token") t.Setenv("DIFFX_REMOTE_URL", "https://secret-user:secret-password@example.test") diff --git a/cmd/diffx/local.go b/cmd/diffx/local.go index 88b51a5c..d6fe7303 100644 --- a/cmd/diffx/local.go +++ b/cmd/diffx/local.go @@ -152,6 +152,10 @@ func acquireLocalInput(values options, stdin *os.File) (initialInput, error) { if err != nil { return initialInput{}, err } + // Reject typos before serving; collection runs only after startup. + if info, err := os.Stat(absolute); err != nil || !info.IsDir() { + return initialInput{}, fmt.Errorf("checkout path %s is not a readable directory", absolute) + } paths = append(paths, absolute) } return initialInput{Kind: "worktree", Paths: paths}, nil diff --git a/cmd/diffx/local_test.go b/cmd/diffx/local_test.go index 49774bac..db060423 100644 --- a/cmd/diffx/local_test.go +++ b/cmd/diffx/local_test.go @@ -23,6 +23,11 @@ func TestLocalInputDefaultsToCurrentDirectory(t *testing.T) { if err != nil || input.Kind != "worktree" || !slices.Equal(input.Paths, []string{root}) { t.Fatalf("default input: %+v %v", input, err) } + for _, name := range []string{"one", "two"} { + if err := os.Mkdir(filepath.Join(root, name), 0o755); err != nil { + t.Fatal(err) + } + } values, err := parseOptions([]string{"--path", "one", "--path=two"}, io.Discard) if err != nil { t.Fatal(err) diff --git a/docs/development.md b/docs/development.md index 0ac31925..3c3cc89b 100644 --- a/docs/development.md +++ b/docs/development.md @@ -60,7 +60,9 @@ List available tasks with `mise tasks`. Keep personal overrides in | Run all pre-push checks, including Go race tests | `pnpm check:push` | | Run lightweight CI checks | `mise run check:ci` | -Set `DIFFX_TEST_PORT` for a separate browser-test server (default 4173), e.g. `DIFFX_TEST_PORT=4183 mise exec -- pnpm --filter @diffx/web test:browser`. +Browser tests build the web app and CLI once, then each Playwright worker runs its own +in-memory fixture server behind a Vite preview on a free port. Tests run in parallel +without shared review state, and concurrent runs from other worktrees do not collide. `mise run setup` enables the Husky `pre-push` hook. Every push runs `mise run check:push`: all repository suites, boundary contracts and Go race tests, then rejects @@ -72,8 +74,10 @@ local platform. Both CI and release verification run `mise run check:ci`: static checks, web and CLI builds, generated-file consistency, and the distribution API smoke test. Local `mise run check` adds dependency/design guards, behavioral contracts, -JavaScript unit/browser suites, release-tool tests and all Go tests; pre-push -adds Go race tests. Keep these full suites and guards local. Chromium is installed +JavaScript unit/browser suites, release-tool tests and all Go tests, running these +suites concurrently after the build; pre-push runs the Go tests with the race +detector instead. Go tests time out after 3 minutes (5 with the race detector) so a +hang fails fast. Keep these full suites and guards local. Chromium is installed by local setup, not CI/release workflows. CI does not run a native OS test matrix. GitHub's `ci-required` ruleset requires the lightweight `checks` status on an up-to-date `main` change, including administrators; it is configured in GitHub, diff --git a/docs/system.md b/docs/system.md index c55b1f06..b3eb687c 100644 --- a/docs/system.md +++ b/docs/system.md @@ -43,7 +43,7 @@ while slower checkouts continue. Publication commits and emits the existing ingestion event for each observation. The UI opens available snapshots and shows loading feedback only for missing data; failures remain visible with Retry. Omitted paths mean the current directory; `--path` repeats and `--scan-depth` (default 8) bounds the -search. Captured snapshots remain fixed after edits and branch switches until the +search. Every path must be an existing directory; missing paths fail before serving. Captured snapshots remain fixed after edits and branch switches until the user requests collection by loading a live URL or pressing Sync, which discovers the paths again and ingests a new snapshot of every checkout; nothing watches Git. Piped input is validated and collected once before serving. diff --git a/internal/remoteserver/server_test.go b/internal/remoteserver/server_test.go index 73cde4b6..ef38fcd8 100644 --- a/internal/remoteserver/server_test.go +++ b/internal/remoteserver/server_test.go @@ -340,7 +340,9 @@ func TestRemoteRunShutdownClosesActiveEventStream(t *testing.T) { if err != nil { t.Fatal(err) } - client := &http.Client{Timeout: 3 * time.Second} + // Without keep-alives the client never leaves a spare unused connection, + // which http.Server.Shutdown would wait on for 5s as a new connection. + client := &http.Client{Timeout: 3 * time.Second, Transport: &http.Transport{DisableKeepAlives: true}} login, err := client.Post("http://"+address+"/api/v2/auth/login", "application/json", strings.NewReader(`{"username":"account","password":"`+testPassword+`"}`)) if err != nil { t.Fatal(err) diff --git a/mise.toml b/mise.toml index bad591ec..fb7c576f 100644 --- a/mise.toml +++ b/mise.toml @@ -74,7 +74,7 @@ run = "pnpm test:go" [tasks."test:race"] description = "Run Go race tests (requires a C compiler)" -run = "go test -race ./..." +run = "go test -race -timeout 5m ./..." [tasks."test:conformance"] description = "Build the CLI and smoke-test its API contract" @@ -129,14 +129,25 @@ run = [ "pnpm test:conformance", ] +[tasks."test:js"] +description = "Run JavaScript unit and browser suites" +run = "pnpm test" + +[tasks."test:release"] +description = "Run release-tool tests" +run = "pnpm test:release" + +# Suites after the build are independent; run them concurrently. [tasks.check] description = "Run all repository checks and test suites" run = [ { task = "check:ci" }, - { task = "test:contracts:rules" }, - "pnpm test", - "pnpm test:release", - { task = "test:go" }, + { tasks = [ + "test:contracts:rules", + "test:js", + "test:release", + "test:go", + ] }, ] [tasks."test:contracts"] @@ -163,9 +174,19 @@ run = [ "node --test test/contracts/*.test.ts", ] +# Race tests run the same Go suite with the detector, replacing test:go. [tasks."check:push"] description = "Full pre-push checks, race tests, and generated-file consistency" -run = [{ task = "check" }, { task = "test:race" }, { task = "check:generated" }] +run = [ + { task = "check:ci" }, + { tasks = [ + "test:contracts:rules", + "test:js", + "test:release", + "test:race", + ] }, + { task = "check:generated" }, +] [tasks."release:publish"] description = "Publish the checked-out release tag with GoReleaser" diff --git a/package.json b/package.json index 073d62ba..cdcbc552 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "typecheck": "pnpm --recursive --if-present typecheck && tsc --noEmit -p test/conformance/tsconfig.json", "test": "pnpm --recursive --if-present test", "test:web": "pnpm --filter @diffx/web test", - "test:go": "go test ./...", + "test:go": "go test -timeout 3m ./...", "test:release": "node --test test/release/*.test.ts", "test:conformance": "node --test test/conformance/*.test.ts", "test:browser:install": "pnpm --filter @diffx/web test:browser:install",