Skip to content
Open
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
20 changes: 20 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,23 @@ jobs:
- name: Typecheck and build
run: bun run build

browser:
runs-on: ubuntu-latest
timeout-minutes: 15
steps:
- uses: actions/checkout@v7
- uses: oven-sh/setup-bun@v2
with:
bun-version: 1.4.2
- run: bun install --frozen-lockfile
- name: Browser tests on a Modal GPU
run: bun run test:modal
env:
MODAL_TOKEN_ID: ${{ secrets.MODAL_TOKEN_ID }}
MODAL_TOKEN_SECRET: ${{ secrets.MODAL_TOKEN_SECRET }}
Comment on lines +41 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Fork pull requests fail browser CI

For fork pull requests, test:modal starts without Modal credentials. GitHub withholds repository secrets from fork pull_request jobs, so the browser job fails before running tests.

Learn more

The workflow starts the browser job for every pull request, including forks. GitHub withholds repository secrets from fork pull_request jobs, leaving both Modal token variables empty. Modal authentication then fails before it can create the GPU sandbox, and those pull requests get a failing CI check.

Example: A contributor opens a pull request from their fork. The check job can run, but the browser job receives empty MODAL_TOKEN_ID and MODAL_TOKEN_SECRET and cannot start the suite.

Recommended fix: Gate the Modal job on credentials being available or on an in-repository pull request, and decide how fork pull requests receive browser coverage without exposing credentials to untrusted code. Keep ordinary build and unit checks available to forks.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

- if: failure()
uses: actions/upload-artifact@v7
with:
name: test-results
path: test-results
if-no-files-found: ignore
4 changes: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ A professional photo editor with little, readable code: Vite, React, [vgpu](http

## Commands

`bun install`, then `bun dev`. On every code change run `bun run check` (writes fixes), `bun run build`, and `bun run test` (Bun; GPU tests use `vgpu/mock`). `bun run test:browser [file]` runs `*.e2e.ts` in Chromium with real pixels; install its browser once per Playwright version with `bunx --no-install playwright install chromium`. It starts its own Vite: stop any other Vite server first, since concurrent servers reload each other's pages mid-test. Any other scripted browser reuses the launch flags in [playwright.config.ts](playwright.config.ts). The assistant at `/?assistant` needs `vercel dev` with `AI_GATEWAY_API_KEY` in `.env.local`.
`bun install`, then `bun dev`. On every code change run `bun run check` (writes fixes), `bun run build`, and `bun run test` (Bun; GPU tests use `vgpu/mock`). `bun run test:browser [file]` runs `*.e2e.ts` in Chromium with real pixels; install its browser once per Playwright version with `bunx --no-install playwright install chromium`. It starts its own Vite: stop any other Vite server first, since concurrent servers reload each other's pages mid-test. Any other scripted browser reuses the launch flags in [playwright.config.ts](playwright.config.ts). `bun run test:modal [file]` runs the same suite on a Modal L4 GPU, signed in through the Modal CLI or `MODAL_TOKEN_ID` and `MODAL_TOKEN_SECRET`; CI runs it on every pull request. The assistant at `/?assistant` needs `vercel dev` with `AI_GATEWAY_API_KEY` in `.env.local`.

## Structure

Expand Down Expand Up @@ -51,7 +51,7 @@ Frame rate and low memory are product requirements, on phones as much as desktop

## Tests

Tests follow what a person verifies: open an image, use a tool, undo, export, check the output. Add one when a workflow or its processing changes or a regression is fixed; core algorithms may have unit tests. Layout, styling, and copy are checked by looking at the app. Tests run offline: the browser fixture fails any request beyond the dev server.
Tests follow what a person verifies: open an image, use a tool, undo, export, check the output. Add one when a workflow or its processing changes or a regression is fixed; core algorithms may have unit tests. Layout, styling, and copy are checked by looking at the app. Tests run offline: the browser fixture fails any request beyond the dev server. Browser tests share one editor page per worker: open images with `openImage` or `openPhoto` rather than reloading, and set `isolated` for a test that changes the page itself.

Browser tests pass on any GPU: GPUs round filtering, `pow`, and half floats differently. Check pixels as a person would, with `expectNear`, directions, and outcomes rather than exact bytes or golden constants; read image content from the export and the preview only for what only it shows.

Expand Down
95 changes: 95 additions & 0 deletions bun.lock

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"build": "tsc && vite build",
"check": "biome check --write",
"test": "bun test",
"test:browser": "playwright test"
"test:browser": "playwright test",
"test:modal": "bun tests/modal.ts"
},
"dependencies": {
"@base-ui/react": "^1.8.0",
Expand Down Expand Up @@ -41,6 +42,7 @@
"@types/react-dom": "^19.2.5",
"@vitejs/plugin-react": "^6.1.1",
"@xmldom/xmldom": "^0.9.12",
"modal": "^0.11.0",
"typescript": "^7.0.2",
"vite": "^8.2.2"
}
Expand Down
3 changes: 2 additions & 1 deletion playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ export default defineConfig({
testDir: "./tests",
testMatch: "**/*.e2e.ts",
use: {
channel: "chromium",
// Modal's GPU sandboxes give WebGPU to the headless shell build only.
channel: process.env.PLAYWRIGHT_CHANNEL ?? "chromium",
baseURL: "http://127.0.0.1:4173",
launchOptions: { args },
},
Expand Down
3 changes: 3 additions & 0 deletions tests/assistant.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { expect, test } from "./fixtures";

// A route stands in for the assistant's server.
test.use({ isolated: true });

test("the assistant at /?assistant edits the photo it was asked about and selects the layer it edits", async ({
page,
}) => {
Expand Down
8 changes: 3 additions & 5 deletions tests/auto-white-balance.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { readFile } from "node:fs/promises";
import type { Page } from "@playwright/test";
import { expect, expectNear, test } from "./fixtures";
import { expect, expectNear, openApp, openImage, test } from "./fixtures";
import { readImage } from "./images";

/** Gray stripes under warm light, as linear sRGB encoded in an SVG; the middle stripe is middle gray. */
Expand Down Expand Up @@ -36,8 +36,7 @@ const undoCount = (page: Page) =>
test("Auto takes most of a warm photo's cast away as one undoable edit, and drops a stale result", async ({
page,
}) => {
await page.goto("/");
await page.locator('input[type="file"]').setInputFiles({
await openImage(page, {
name: "cast.svg",
mimeType: "image/svg+xml",
buffer: Buffer.from(castWedge()),
Expand Down Expand Up @@ -79,8 +78,7 @@ test("Auto takes most of a warm photo's cast away as one undoable edit, and drop
test("Auto moves a RAW photo's white balance from As Shot toward neutral", async ({
page,
}) => {
await page.goto("/");
await page.waitForFunction(() => window.openlight);
await openApp(page);
const bytes = [...(await readFile("tests/fixtures/raw/bayer.dng"))];
await page.evaluate(
(bytes) =>
Expand Down
14 changes: 9 additions & 5 deletions tests/brush.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import { expect, expectNear, expectPixels, openPhoto, test } from "./fixtures";
import {
expect,
expectNear,
expectPixels,
openImage,
openPhoto,
test,
} from "./fixtures";
import { readImage, redness } from "./images";
import { box, drag } from "./pointer";

Expand Down Expand Up @@ -159,10 +166,7 @@ test.describe("on a touch phone", () => {
page,
context,
}) => {
await page.goto("/");
await page
.locator('input[type="file"]')
.setInputFiles("tests/fixtures/photo.svg");
await openImage(page, "tests/fixtures/photo.svg");
await expect(
page.getByRole("slider", { name: "Exposure", exact: true }),
).toHaveAttribute("aria-valuetext", "0.00");
Expand Down
5 changes: 2 additions & 3 deletions tests/clarity.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, test } from "./fixtures";
import { expect, openApp, test } from "./fixtures";

// Full-resolution Gaussian reference, independent of GPU passes and working-space conversion.
function expectedStep(x: number, amount: number, sigma: number) {
Expand All @@ -20,8 +20,7 @@ function expectedStep(x: number, amount: number, sigma: number) {
test("detail filters follow Gaussian unsharp masking and preserve flat fields and alpha", async ({
page,
}) => {
await page.goto("/");
await page.waitForFunction(() => window.openlight);
await openApp(page);
const xs = [0, 384, 448, 504, 509, 510, 511, 512, 513, 514, 519, 576, 640];
const settings = [
{ clarity: -100, sharpening: 0, sharpenRadius: 1 },
Expand Down
5 changes: 2 additions & 3 deletions tests/crop.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Page } from "@playwright/test";
import { expect, expectNear, openPhoto, test } from "./fixtures";
import { expect, expectNear, openImage, openPhoto, test } from "./fixtures";
import { readImage } from "./images";
import { box, choose, drag, zoom } from "./pointer";

Expand All @@ -21,8 +21,7 @@ const keystone = Buffer.from(
);

async function openKeystone(page: Page) {
await page.goto("/");
await page.locator('input[type="file"]').setInputFiles({
await openImage(page, {
name: "keystone.svg",
mimeType: "image/svg+xml",
buffer: keystone,
Expand Down
3 changes: 3 additions & 0 deletions tests/draft.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import type { Page } from "@playwright/test";
import { expect, expectNear, openPhoto, test } from "./fixtures";
import { readImage } from "./images";

// Reloads find the drafts these tests save, and one removes IndexedDB before the editor loads.
test.use({ isolated: true });

/** The stored draft's base exposure, read straight from IndexedDB. */
function savedExposure(page: Page) {
return page.evaluate(
Expand Down
125 changes: 109 additions & 16 deletions tests/fixtures.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { test as base, expect, type Page } from "@playwright/test";
import { basename } from "node:path";
import {
type BrowserContext,
test as base,
type ConsoleMessage,
expect,
type Page,
type Route,
} from "@playwright/test";

export { expect };

Expand All @@ -20,12 +28,53 @@ export function expectNear(
}
}

/** Shows the editor, loading it only when the page is not already on it. */
export async function openApp(page: Page) {
const ready = await page.evaluate(
() =>
location.pathname === "/" && !location.search && "openlight" in window,
);
if (!ready) {
await page.goto("/");
await page.waitForFunction(() => window.openlight);
}
}

/** Opens an image as a new document: a fixture path served by the dev server, or a file's contents. */
export async function openImage(
page: Page,
file: string | { name: string; mimeType: string; buffer: Buffer },
) {
await openApp(page);
const failure = await page.evaluate(
async ([file]) => {
const api = window.openlight;
if (typeof file === "string") {
await api.loadUrl(`/${file}`);
} else {
await api.loadImage(
new File([new Uint8Array(file.bytes)], file.name, {
type: file.mimeType,
}),
);
}
return api.getState().failure;
},
[
typeof file === "string"
? file
: { name: file.name, mimeType: file.mimeType, bytes: [...file.buffer] },
] as const,
);
expect(
failure,
`open ${typeof file === "string" ? basename(file) : file.name}`,
).toBeUndefined();
}

/** Opens the test photo and waits for its controls. */
export async function openPhoto(page: Page) {
await page.goto("/");
await page
.locator('input[type="file"]')
.setInputFiles("tests/fixtures/photo.svg");
await openImage(page, "tests/fixtures/photo.svg");
await expect(
page.getByRole("textbox", { name: "Exposure", exact: true }),
).toHaveValue("0.00");
Expand All @@ -43,26 +92,70 @@ export function expectPixels(
}
}

export const test = base.extend<{ browserErrors: undefined }>({
/**
* Each worker loads the editor once and its tests share the page: opening an image starts a new
* document, which the editor mounts afresh. A test that changes the page itself, with routes,
* init scripts, a fake clock, or touch, sets `isolated` for a page of its own; a failure drops the
* shared page so the next test starts clean.
*/
export const test = base.extend<
{ isolated: boolean; browserErrors: undefined },
{ shared: { context: BrowserContext; page?: Page } }
>({
isolated: [false, { option: true }],
shared: [
async ({ browser }, use, workerInfo) => {
const context = await browser.newContext({
baseURL: workerInfo.project.use.baseURL,
});
await use({ context });
await context.close();
},
{ scope: "worker" },
],
page: async (
{ context, viewport, hasTouch, isolated, shared },
use,
testInfo,
) => {
if (isolated || hasTouch) {
await use(await context.newPage());
return;
}
shared.page ??= await shared.context.newPage();
const page = shared.page;
if (viewport) {
await page.setViewportSize(viewport);
}
await use(page);
if (testInfo.status !== testInfo.expectedStatus) {
shared.page = undefined;
await page.close();
}
},
browserErrors: [
async ({ page, baseURL }, use) => {
const errors: string[] = [];
// Tests run offline: a request beyond the dev server fails the test.
const origin = new URL(baseURL ?? "").origin;
await page.route(
(url) => url.origin !== origin,
(route) => {
errors.push(`Request outside the app: ${route.request().url()}`);
return route.abort();
},
);
page.on("pageerror", (error) => errors.push(error.message));
page.on("console", (message) => {
const outside = (url: URL) => url.origin !== origin;
const abort = (route: Route) => {
errors.push(`Request outside the app: ${route.request().url()}`);
return route.abort();
};
const pageError = (error: Error) => errors.push(error.message);
const consoleError = (message: ConsoleMessage) => {
if (message.type() === "error") {
errors.push(message.text());
}
});
};
await page.route(outside, abort);
page.on("pageerror", pageError);
page.on("console", consoleError);
await use(undefined);
page.off("pageerror", pageError);
page.off("console", consoleError);
await page.unroute(outside, abort);
expect(errors).toEqual([]);
},
{ auto: true },
Expand Down
5 changes: 2 additions & 3 deletions tests/grain.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Page } from "@playwright/test";
import { expect, test } from "./fixtures";
import { expect, openImage, test } from "./fixtures";

/** The exported image's red channel, row by row; grain keeps a gray image gray. */
async function exportRed(page: Page) {
Expand Down Expand Up @@ -42,8 +42,7 @@ function expectSame(actual: readonly number[], expected: readonly number[]) {
test("grain textures a gray image without shifting its tone, holds through a crop, and undoes", async ({
page,
}) => {
await page.goto("/");
await page.locator('input[type="file"]').setInputFiles({
await openImage(page, {
name: "gray.svg",
mimeType: "image/svg+xml",
buffer: Buffer.from(
Expand Down
7 changes: 2 additions & 5 deletions tests/heal-edit.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { expect, expectNear, test } from "./fixtures";
import { expect, expectNear, openImage, test } from "./fixtures";
import { readImage } from "./images";
import { box } from "./pointer";

test("Remove adds and subtracts separate strokes on the selected patch, on release", async ({
page,
}) => {
await page.setViewportSize({ width: 1440, height: 1000 });
await page.goto("/");
await page
.locator('input[type="file"]')
.setInputFiles("tests/fixtures/spots.svg");
await openImage(page, "tests/fixtures/spots.svg");
await expect(
page.getByRole("textbox", { name: "Exposure", exact: true }),
).toHaveValue("0.00");
Expand Down
5 changes: 2 additions & 3 deletions tests/heal-settle.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import { expect, test } from "./fixtures";
import { expect, openImage, test } from "./fixtures";
import { box } from "./pointer";

for (const gesture of ["stroke", "destination"] as const) {
test(`Remove ${gesture} survives a background painting settle and commits on release`, async ({
page,
}) => {
await page.setViewportSize({ width: 1440, height: 1000 });
await page.goto("/");
await page.locator('input[type="file"]').setInputFiles({
await openImage(page, {
name: "small.svg",
mimeType: "image/svg+xml",
buffer: Buffer.from(
Expand Down
Loading
Loading