mirror of
https://github.com/yusufipk/OpenFrame.git
synced 2026-09-11 09:36:08 +00:00
Second pass over the suite, driven by the inventory in the gaps document. Nine agents wrote suites in parallel against private databases, then a tenth read all of it adversarially and five of its findings were fixed. unit + component 2076 -> 2079 (+888 over the round) api 647 -> 1015 e2e 18 -> 29 What was closed: - lib/route-access.ts, the page-level authorization layer, went from zero tests to 48. Every API route was guarded and none of the pages were. - The five media proxy routes now have a real 2xx beside every 403. The blocker was the positive control, solved by stubbing r2Client.send() and leaving lib/r2-media-proxy.ts itself real. - Every remaining server-side lib module: invitations, email verification, the upload tokens, the logger, request origin, the whole R2 and Bunny lifecycle, notifications and admin stats. - Six video-page hooks, and the chunking arithmetic extracted out of lib/client/r2-video-upload.ts as a pure module. - Five end-to-end flows: workspace members, bulk operations, the admin area, player interaction and failure recovery. Three things about the harness itself turned out to be wrong: - Two @/lib/r2 stubs in tests/setup/api.ts had the wrong return shape, so every route reaching finalizeR2VideoUpload silently took the "not a valid video" branch and no test noticed. - The auth matrix asserted only "not 2xx", which two entries satisfied without their guard existing. It now requires 401 or 403, which makes both load-bearing, and all 60 routes pass the stricter form. - Both admin API routes had no positive control anywhere: replacing their guard with an unconditional refusal left the entire suite green. Found by the adversarial review, now covered. Process: - bun run test:mutation runs StrykerJS over the authorization and validation modules. Diagnostic, not a gate, weekly in CI rather than on a push. - playwright.config.ts gains an opt-in webkit project for the player spec. - AGENTS.md now requires a batch of new tests to be reviewed by somebody who did not write them. Only two production files change, both deliberate: lib/auth.ts loses a verbatim copy of its own permission formulas, and lib/client/r2-video-upload.ts calls the extracted arithmetic. No behaviour change in either.
152 lines
6.8 KiB
TypeScript
152 lines
6.8 KiB
TypeScript
// Player interaction against a real <video> element.
|
|
//
|
|
// The pure arithmetic behind these controls lives in
|
|
// components/video-page/hooks/video-player-utils.ts and is unit tested there.
|
|
// This spec deliberately covers only what a unit test cannot: that the numbers
|
|
// the hook computes are actually written to a media element, and that a
|
|
// keystroke on the document reaches that element. Every assertion below reads
|
|
// `HTMLVideoElement.currentTime` out of the browser, so a control that renders
|
|
// but is wired to nothing fails here.
|
|
//
|
|
// A real file has to be uploaded first. `<video>` is rendered only for the
|
|
// `bunny` and `r2` providers (components/video-page/player-core.tsx), and the
|
|
// seeded `youtube` versions every other spec uses render an iframe instead, so
|
|
// there is no media element to interrogate. The upload goes through the same
|
|
// form video-upload.spec.ts drives, against the MinIO service in
|
|
// docker-compose.test.yml.
|
|
//
|
|
// tests/fixtures/sample.mp4 is 2.0 seconds at 10 fps. Those two numbers are
|
|
// hardcoded in the expectations below on purpose: deriving them from the file
|
|
// at runtime would let a broken seek agree with a broken measurement.
|
|
import path from 'node:path';
|
|
import type { Page } from '@playwright/test';
|
|
import { test, expect } from './fixtures';
|
|
import { REPO_ROOT } from '../helpers/env';
|
|
|
|
const SAMPLE_VIDEO = path.join(REPO_ROOT, 'tests', 'fixtures', 'sample.mp4');
|
|
const SAMPLE_DURATION_SECONDS = 2;
|
|
|
|
// One upload, three network round trips and a MinIO PUT before the first
|
|
// assertion.
|
|
test.setTimeout(120_000);
|
|
|
|
/** `video.currentTime` as the browser currently reports it. */
|
|
function currentTime(page: Page): Promise<number> {
|
|
return page.locator('video').evaluate((el) => (el as HTMLVideoElement).currentTime);
|
|
}
|
|
|
|
/**
|
|
* Uploads sample.mp4 into a fresh project and opens the video page.
|
|
*
|
|
* Returns nothing: everything the assertions need is read off the page.
|
|
*/
|
|
async function uploadAndOpen(page: Page, projectId: string, title: string): Promise<void> {
|
|
await page.goto(`/projects/${projectId}/videos/new`);
|
|
|
|
const directUploadTab = page.getByRole('tab', { name: 'Direct Upload' });
|
|
await expect(directUploadTab).toBeVisible();
|
|
await directUploadTab.click();
|
|
|
|
await page.getByLabel('Video Files').setInputFiles(SAMPLE_VIDEO);
|
|
await page.getByLabel('Title').fill(title);
|
|
await page.getByRole('button', { name: 'Add Video', exact: true }).click();
|
|
|
|
await expect(page).toHaveURL(new RegExp(`/projects/${projectId}$`), { timeout: 90_000 });
|
|
await page.getByRole('heading', { name: title, level: 3 }).click();
|
|
await expect(page).toHaveURL(new RegExp(`/projects/${projectId}/videos/[^/]+$`));
|
|
}
|
|
|
|
/** Waits until the media element has read its metadata, then checks it is ours. */
|
|
async function waitForMetadata(page: Page): Promise<void> {
|
|
const video = page.locator('video');
|
|
await expect(video).toBeVisible();
|
|
await expect
|
|
.poll(() => video.evaluate((el) => (el as HTMLVideoElement).readyState), { timeout: 30_000 })
|
|
.toBeGreaterThanOrEqual(1);
|
|
|
|
const duration = await video.evaluate((el) => (el as HTMLVideoElement).duration);
|
|
expect(duration).toBeGreaterThan(1.9);
|
|
expect(duration).toBeLessThan(2.2);
|
|
}
|
|
|
|
test('the timeline, the arrow keys and frame mode all move the video element', async ({
|
|
page,
|
|
seed,
|
|
seededUser,
|
|
}) => {
|
|
const { project } = await seed.project(seededUser);
|
|
await uploadAndOpen(page, project.id, `Player Video ${Date.now()}`);
|
|
await waitForMetadata(page);
|
|
|
|
expect(await currentTime(page)).toEqual(0);
|
|
await expect(page.getByText(`0:00 / 0:0${SAMPLE_DURATION_SECONDS}`)).toBeVisible();
|
|
|
|
// --- scrubbing ------------------------------------------------------------
|
|
// The scrub bar carries no role, no label and no id, so it is located by the
|
|
// class list it is built with in player-core.tsx. Reported rather than worked
|
|
// around: a keyboard user cannot reach this control at all.
|
|
const timeline = page.locator('div.h-8.bg-muted.cursor-pointer');
|
|
await expect(timeline).toBeVisible();
|
|
const box = await timeline.boundingBox();
|
|
if (!box) throw new Error('The scrub bar has no layout box.');
|
|
|
|
// Three quarters along a two second video is 1.5s. mousedown alone commits
|
|
// the seek (handleTimelineMouseDown), so a plain click is enough.
|
|
await timeline.click({ position: { x: box.width * 0.75, y: box.height / 2 } });
|
|
await expect.poll(() => currentTime(page)).toBeGreaterThan(1.2);
|
|
await expect(page.getByText(`0:01 / 0:0${SAMPLE_DURATION_SECONDS}`)).toBeVisible();
|
|
|
|
// --- keyboard -------------------------------------------------------------
|
|
// ArrowLeft is 'skip-back' by five seconds, clamped at zero. Starting from
|
|
// 1.5s means the clamp is the only thing that can produce this value, and it
|
|
// cannot be the initial state because the scrub above moved off it.
|
|
await page.keyboard.press('ArrowLeft');
|
|
await expect.poll(() => currentTime(page)).toEqual(0);
|
|
|
|
// ArrowRight is 'skip-forward' by five, clamped at the duration.
|
|
await page.keyboard.press('ArrowRight');
|
|
await expect.poll(() => currentTime(page)).toBeGreaterThan(1.9);
|
|
|
|
await page.keyboard.press('ArrowLeft');
|
|
await expect.poll(() => currentTime(page)).toEqual(0);
|
|
|
|
// --- frame mode -----------------------------------------------------------
|
|
// No frame rate has been measured yet (that only happens during playback), so
|
|
// one step is one second, and the button labels say so. The point of the
|
|
// assertion is the *difference* from the 10s and 5s jumps above: a step that
|
|
// lands on 1.0 could not have come from either.
|
|
await expect(page.getByRole('button', { name: 'Forward 10s' })).toBeVisible();
|
|
await page.getByRole('button', { name: /^Frame / }).click();
|
|
const forwardOneStep = page.getByRole('button', { name: 'Forward 1s' });
|
|
await expect(forwardOneStep).toBeVisible();
|
|
|
|
await forwardOneStep.click();
|
|
await expect.poll(() => currentTime(page)).toBeGreaterThan(0.9);
|
|
expect(await currentTime(page)).toBeLessThan(1.2);
|
|
|
|
await page.getByRole('button', { name: 'Back 1s' }).click();
|
|
await expect.poll(() => currentTime(page)).toEqual(0);
|
|
});
|
|
|
|
test('the arrow keys are not hijacked while a comment is being typed', async ({
|
|
page,
|
|
seed,
|
|
seededUser,
|
|
}) => {
|
|
const { project } = await seed.project(seededUser);
|
|
await uploadAndOpen(page, project.id, `Player Typing ${Date.now()}`);
|
|
await waitForMetadata(page);
|
|
|
|
const composer = page.getByPlaceholder('Add a comment...');
|
|
await composer.fill('cursor keys belong to this box');
|
|
await composer.click();
|
|
// Put the caret in the middle so ArrowLeft has somewhere to go inside the
|
|
// field; if the player claimed the key, the video would seek instead.
|
|
await page.keyboard.press('End');
|
|
await page.keyboard.press('ArrowLeft');
|
|
await page.keyboard.press('ArrowRight');
|
|
|
|
expect(await currentTime(page)).toEqual(0);
|
|
await expect(composer).toHaveValue('cursor keys belong to this box');
|
|
});
|