Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions apps/web/playwright.config.ts
Original file line number Diff line number Diff line change
@@ -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: [
Expand All @@ -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,
},
});
5 changes: 4 additions & 1 deletion apps/web/test/browser/app.spec.ts
Original file line number Diff line number Diff line change
@@ -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(
Expand Down Expand Up @@ -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("/");
Expand Down
3 changes: 2 additions & 1 deletion apps/web/test/browser/async-recovery.spec.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down
3 changes: 2 additions & 1 deletion apps/web/test/browser/design-refresh.spec.ts
Original file line number Diff line number Diff line change
@@ -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));
Expand Down
53 changes: 53 additions & 0 deletions apps/web/test/browser/fixtures.ts
Original file line number Diff line number Diff line change
@@ -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),
});
29 changes: 29 additions & 0 deletions apps/web/test/browser/global-setup.ts
Original file line number Diff line number Diff line change
@@ -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 });
}
3 changes: 2 additions & 1 deletion apps/web/test/browser/identity.spec.ts
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion apps/web/test/browser/local-workspace.spec.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down
2 changes: 1 addition & 1 deletion apps/web/test/browser/native-scrollbars.spec.ts
Original file line number Diff line number Diff line change
@@ -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"] } });
Expand Down
33 changes: 23 additions & 10 deletions apps/web/test/fixture-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"],
Expand Down Expand Up @@ -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);
Expand Down
33 changes: 31 additions & 2 deletions cmd/diffx/ingestion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package main

import (
"bytes"
"context"
"io"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
"time"

"github.com/flexdinesh/diffx/internal/ingestion"
)
Expand Down Expand Up @@ -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")
Expand Down
4 changes: 4 additions & 0 deletions cmd/diffx/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions cmd/diffx/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 7 additions & 3 deletions docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion docs/system.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion internal/remoteserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading