From dfeb6f2f5d79b4b44c66b696a047ccf9fa43843a Mon Sep 17 00:00:00 2001 From: drakezhang Date: Tue, 27 Oct 2026 10:40:00 +0800 Subject: [PATCH 1/4] test(video): wait for presented frames and retain CI evidence --- .github/workflows/ci.yml | 16 ++++- apps/extension/src/video/browser-fixture.ts | 17 ++++++ .../src/video/capture.browser.test.ts | 26 +++++++-- .../src/video/encoder.browser.test.ts | 58 +++++++++++++------ .../src/video/playback-fixture.test.ts | 58 +++++++++++++++++++ apps/extension/src/video/playback-fixture.ts | 56 ++++++++++++++++++ 6 files changed, 207 insertions(+), 24 deletions(-) create mode 100644 apps/extension/src/video/playback-fixture.test.ts create mode 100644 apps/extension/src/video/playback-fixture.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 53bb828b..973c9290 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -288,6 +288,9 @@ jobs: # Allows Chrome for Testing's user-namespace sandbox without a custom # AppArmor profile for its temporary installation path. runs-on: ubuntu-22.04 + timeout-minutes: 15 + env: + BSK_VIDEO_ARTIFACTS: ${{ runner.temp }}/video-regression steps: - uses: actions/checkout@v6 @@ -305,6 +308,8 @@ jobs: curl --fail --silent --show-error --location --retry 3 "$video_chrome_download" --output "$RUNNER_TEMP/video-chrome.zip" unzip -q "$RUNNER_TEMP/video-chrome.zip" -d "$RUNNER_TEMP/video-chrome" echo "BSK_VIDEO_CHROME=$RUNNER_TEMP/video-chrome/chrome-linux64/chrome" >> "$GITHUB_ENV" + mkdir -p "$BSK_VIDEO_ARTIFACTS" + "$RUNNER_TEMP/video-chrome/chrome-linux64/chrome" --version | tee "$BSK_VIDEO_ARTIFACTS/chrome-version.txt" - name: Build CLI, extension and DSH plugin run: | cargo build -p bsk --locked @@ -314,7 +319,16 @@ jobs: working-directory: apps/extension env: BSK_VIDEO_BSK: ../../target/debug/bsk - run: pnpm exec vitest run src/video/*.browser.test.ts + run: pnpm exec vitest run src/video/*.browser.test.ts --reporter=default --reporter=json --outputFile.json="$BSK_VIDEO_ARTIFACTS/results.json" + + - name: Upload video regression evidence + if: always() + uses: actions/upload-artifact@v7 + with: + name: video-regression + path: ${{ env.BSK_VIDEO_ARTIFACTS }}/ + if-no-files-found: warn + retention-days: 7 - name: Build extension for the isolated native-dialog daemon env: diff --git a/apps/extension/src/video/browser-fixture.ts b/apps/extension/src/video/browser-fixture.ts index add22929..73b4e468 100644 --- a/apps/extension/src/video/browser-fixture.ts +++ b/apps/extension/src/video/browser-fixture.ts @@ -1,3 +1,5 @@ +import { copyFile, mkdir, writeFile } from "node:fs/promises"; +import path from "node:path"; import { expect } from "vitest"; export type BrowserSend = >( @@ -48,3 +50,18 @@ export async function attachVideoBackground(send: BrowserSend, name: string) { .toBe(true); return found!; } + +/** Keep the MP4 before pixel assertions so a failed CI run remains diagnosable. */ +export async function saveVideoEvidence( + name: string, + details: unknown, + video?: string | Uint8Array, +) { + const directory = process.env.BSK_VIDEO_ARTIFACTS; + if (!directory) return; + await mkdir(directory, { recursive: true }); + await writeFile(path.join(directory, `${name}.json`), JSON.stringify(details, null, 2)); + const destination = path.join(directory, `${name}.mp4`); + if (typeof video === "string") await copyFile(video, destination); + else if (video) await writeFile(destination, video); +} diff --git a/apps/extension/src/video/capture.browser.test.ts b/apps/extension/src/video/capture.browser.test.ts index 8664b22a..e3072cd4 100644 --- a/apps/extension/src/video/capture.browser.test.ts +++ b/apps/extension/src/video/capture.browser.test.ts @@ -9,8 +9,13 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { promisify } from "node:util"; import { describe, expect, it } from "vitest"; -import { attachVideoBackground, type BrowserSend as Send } from "./browser-fixture"; +import { + attachVideoBackground, + type BrowserSend as Send, + saveVideoEvidence, +} from "./browser-fixture"; import { createDshVideoFixture } from "./dsh-fixture"; +import { readVideoPixels } from "./playback-fixture"; const run = promisify(execFile); @@ -257,6 +262,8 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( ); const output = path.join(directory, "result.mp4"); await cli("video", "save", "--recording", id, "--out", output); + const version = await send("Browser.getVersion"); + await saveVideoEvidence("capture", { version, recording: stopped.recording }, output); expect((await stat(output)).size).toBe(stopped.recording.byte_size); await expect( cli("video", "save", "--recording", id, "--out", output), @@ -276,8 +283,9 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( .toBe(true); const pixels = await evaluate( preview.sessionId, - `(async()=>{const v=document.querySelector('video'),c=document.createElement('canvas');c.width=v.videoWidth;c.height=v.videoHeight;const ctx=c.getContext('2d'),pixels=[];for(const time of [.2,v.duration-.3]){v.currentTime=time;await new Promise(r=>v.onseeked=r);ctx.drawImage(v,0,0);pixels.push(Array.from(ctx.getImageData(c.width/2,c.height/2,1,1).data));}return pixels})()`, + `(${readVideoPixels.toString()})(document.querySelector('video'), [.2, document.querySelector('video').duration-.3])`, ); + await saveVideoEvidence("capture", { version, recording: stopped.recording, pixels }); expect(pixels[0][0]).toBeGreaterThan(240); expect(pixels[1][1]).toBeGreaterThan(240); expect(pixels[1][0]).toBeLessThan(15); @@ -783,6 +791,11 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( recordingId, output, }); + await saveVideoEvidence( + "capture-dsh", + { version, recording: finished.recording }, + output, + ); expect((await stat(output)).size).toBe(finished.recording.byte_size); await expect( dsh.call("bsk_inspect", { @@ -820,10 +833,15 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( }); // The preview intentionally preloads only metadata. Seek to // request and verify a decoded frame from the completed task. - const pixel = await evaluate( + const [pixel] = await evaluate( preview.sessionId, - `(async()=>{const v=document.querySelector('video'),c=document.createElement('canvas');c.width=v.videoWidth;c.height=v.videoHeight;v.currentTime=v.duration-.2;await new Promise(r=>v.onseeked=r);const ctx=c.getContext('2d');ctx.drawImage(v,0,0);return Array.from(ctx.getImageData(c.width/2,c.height/2,1,1).data)})()`, + `(${readVideoPixels.toString()})(document.querySelector('video'), [document.querySelector('video').duration-.2])`, ); + await saveVideoEvidence("capture-dsh", { + version, + recording: finished.recording, + pixel, + }); expect(pixel[0]).toBeGreaterThan(240); expect(pixel[1]).toBeLessThan(15); } finally { diff --git a/apps/extension/src/video/encoder.browser.test.ts b/apps/extension/src/video/encoder.browser.test.ts index 8b8b47ca..a3e5e41d 100644 --- a/apps/extension/src/video/encoder.browser.test.ts +++ b/apps/extension/src/video/encoder.browser.test.ts @@ -6,7 +6,11 @@ import { createRequire } from "node:module"; import { tmpdir } from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; -import { attachVideoBackground, type BrowserSend as Send } from "./browser-fixture"; +import { + attachVideoBackground, + type BrowserSend as Send, + saveVideoEvidence, +} from "./browser-fixture"; const entry = ` import { BrowserVideoHost } from '@/video/host'; @@ -40,17 +44,18 @@ globalThis.runEncoder = async (crash = false, width = 320, height = 180, quality const player = ` import { VideoArtifactStore } from '@/video/store'; +import { readVideoPixels } from '@/video/playback-fixture'; +globalThis.exportVideo=async(id)=>{ + const bytes=new Uint8Array(await (await new VideoArtifactStore().file(id)).arrayBuffer()); + let binary='';for(let i=0;i{ const file=await new VideoArtifactStore().file(id); const video=document.createElement('video');video.muted=true;document.body.append(video); const url=URL.createObjectURL(file);video.src=url; await new Promise((resolve,reject)=>{video.onloadeddata=resolve;video.onerror=()=>reject(Error('MP4 playback failed'));}); - const pixels=[]; - const canvas=document.createElement('canvas');canvas.width=320;canvas.height=180;const ctx=canvas.getContext('2d'); - for(const time of [.2,video.duration-.2]){ - video.currentTime=time;await new Promise(resolve=>video.onseeked=resolve); - ctx.drawImage(video,0,0);pixels.push(Array.from(ctx.getImageData(160,90,1,1).data)); - } + const pixels=await readVideoPixels(video,[.2,video.duration-.2]); const result={duration:video.duration,width:video.videoWidth,height:video.videoHeight,pixels}; URL.revokeObjectURL(url);video.remove();return result; }; @@ -164,12 +169,25 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME)("video encoder browser regression timeout: 10_000, }) .toBe(true); - const result = await evaluate<{ - duration: number; - width: number; - height: number; - pixels: number[][]; - }>(attached.sessionId, `inspectVideo(${JSON.stringify(done.value.id)})`); + const version = await send("Browser.getVersion"); + const inspect = async (id: string, name: string, recording: unknown) => { + if (process.env.BSK_VIDEO_ARTIFACTS) { + const data = await evaluate( + attached.sessionId, + `exportVideo(${JSON.stringify(id)})`, + ); + await saveVideoEvidence(name, { version, recording }, Buffer.from(data, "base64")); + } + const playback = await evaluate<{ + duration: number; + width: number; + height: number; + pixels: number[][]; + }>(attached.sessionId, `inspectVideo(${JSON.stringify(id)})`); + await saveVideoEvidence(name, { version, recording, playback }); + return playback; + }; + const result = await inspect(done.value.id, "encoder-standard", done.value.result); expect(result.width).toBe(320); expect(result.height).toBe(180); expect(result.duration).toBeGreaterThanOrEqual(2.3); @@ -205,9 +223,10 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME)("video encoder browser regression stop_reason: "browser_restarted", }); expect(recovered.value.result.byte_size).toBeGreaterThan(0); - const salvaged = await evaluate<{ duration: number }>( - attached.sessionId, - `inspectVideo(${JSON.stringify(recovered.value.id)})`, + const salvaged = await inspect( + recovered.value.id, + "encoder-recovered", + recovered.value.result, ); expect(salvaged.duration).toBeGreaterThan(0); @@ -224,9 +243,10 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME)("video encoder browser regression }>(background.sessionId, `runEncoder(false, ${width}, ${height}, 'clear')`); expect(recorded.result.error).toBeUndefined(); expect(recorded.result).toMatchObject({ state: "ready", completeness: "complete" }); - const playback = await evaluate( - attached.sessionId, - `inspectVideo(${JSON.stringify(recorded.id)})`, + const playback = await inspect( + recorded.id, + `encoder-${width}x${height}`, + recorded.result, ); expect(playback).toMatchObject({ width, height }); expect(playback.duration).toBeGreaterThanOrEqual(2.3); diff --git a/apps/extension/src/video/playback-fixture.test.ts b/apps/extension/src/video/playback-fixture.test.ts new file mode 100644 index 00000000..66ea3976 --- /dev/null +++ b/apps/extension/src/video/playback-fixture.test.ts @@ -0,0 +1,58 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { readVideoPixels } from "./playback-fixture"; + +afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); +}); + +function player() { + const video = document.createElement("video"); + let present!: VideoFrameRequestCallback; + video.requestVideoFrameCallback = vi.fn((callback) => { + present = callback; + return 1; + }); + video.cancelVideoFrameCallback = vi.fn(); + vi.spyOn(video, "pause").mockImplementation(() => {}); + const drawImage = vi.fn(); + vi.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue({ + drawImage, + getImageData: () => ({ data: new Uint8ClampedArray([0, 0, 255, 255]) }), + } as unknown as CanvasRenderingContext2D); + return { + video, + drawImage, + seeked: () => video.dispatchEvent(new Event("seeked")), + present: () => present(0, {} as VideoFrameCallbackMetadata), + }; +} + +describe("video pixel sampling", () => { + it.each([ + "seeked", + "present", + ] as const)("waits for both signals when %s arrives first", async (first) => { + const fixture = player(); + const pixels = readVideoPixels(fixture.video, [1.5]); + fixture[first](); + await Promise.resolve(); + expect(fixture.drawImage).not.toHaveBeenCalled(); + fixture[first === "seeked" ? "present" : "seeked"](); + await expect(pixels).resolves.toEqual([[0, 0, 255, 255]]); + expect(fixture.drawImage).toHaveBeenCalledTimes(1); + expect(fixture.video.cancelVideoFrameCallback).toHaveBeenCalledWith(1); + }); + + it("fails instead of reading a stale frame when presentation never arrives", async () => { + vi.useFakeTimers(); + const fixture = player(); + const pixels = readVideoPixels(fixture.video, [1.5]); + const rejected = expect(pixels).rejects.toThrow("seeked=true, presented=false"); + fixture.seeked(); + await vi.advanceTimersByTimeAsync(5000); + await rejected; + expect(fixture.drawImage).not.toHaveBeenCalled(); + expect(fixture.video.cancelVideoFrameCallback).toHaveBeenCalledWith(1); + }); +}); diff --git a/apps/extension/src/video/playback-fixture.ts b/apps/extension/src/video/playback-fixture.ts new file mode 100644 index 00000000..176ffdaa --- /dev/null +++ b/apps/extension/src/video/playback-fixture.ts @@ -0,0 +1,56 @@ +/** Runs in the player page (also serialized into CDP Runtime.evaluate). */ +export async function readVideoPixels(video: HTMLVideoElement, times: number[]) { + video.pause(); + const canvas = document.createElement("canvas"); + canvas.width = video.videoWidth; + canvas.height = video.videoHeight; + const context = canvas.getContext("2d")!; + const pixels: number[][] = []; + for (const time of times) { + // seeked can precede presentation of the decoded frame. In particular, + // drawImage immediately after seeking can still copy the previous frame. + // Subscribe before seeking so neither event can be missed on a fast decode. + await new Promise((resolve, reject) => { + let seeked = false; + let presented = false; + let callback = 0; + const cleanup = () => { + clearTimeout(timer); + video.cancelVideoFrameCallback(callback); + video.removeEventListener("seeked", onSeeked); + video.removeEventListener("error", onError); + }; + const complete = () => { + if (!seeked || !presented) return; + cleanup(); + resolve(); + }; + const onSeeked = () => { + seeked = true; + complete(); + }; + const onError = () => { + cleanup(); + reject(new Error(`MP4 playback failed: ${video.error?.message}`)); + }; + const timer = setTimeout(() => { + cleanup(); + reject( + new Error( + `MP4 frame at ${time}s did not become ready (seeked=${seeked}, presented=${presented}, readyState=${video.readyState})`, + ), + ); + }, 5000); + video.addEventListener("seeked", onSeeked); + video.addEventListener("error", onError); + callback = video.requestVideoFrameCallback(() => { + presented = true; + complete(); + }); + video.currentTime = time; + }); + context.drawImage(video, 0, 0); + pixels.push(Array.from(context.getImageData(canvas.width / 2, canvas.height / 2, 1, 1).data)); + } + return pixels; +} From 2a760bcd41a41df9942618f60dce3fbef871b3b0 Mon Sep 17 00:00:00 2001 From: drakezhang Date: Tue, 27 Oct 2026 10:41:54 +0800 Subject: [PATCH 2/4] ci: use workspace context for video artifact directory --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 973c9290..9902b89b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -290,7 +290,7 @@ jobs: runs-on: ubuntu-22.04 timeout-minutes: 15 env: - BSK_VIDEO_ARTIFACTS: ${{ runner.temp }}/video-regression + BSK_VIDEO_ARTIFACTS: ${{ github.workspace }}/evals/browser/results/video-regression steps: - uses: actions/checkout@v6 From bf134e0dc2efe1fa6efbb02859da1855e426a5a9 Mon Sep 17 00:00:00 2001 From: drakezhang Date: Tue, 27 Oct 2026 11:58:22 +0800 Subject: [PATCH 3/4] test(video): validate all sampled frames and harden CI diagnostics --- .github/workflows/ci.yml | 2 +- .../src/video/browser-fixture.test.ts | 57 ++++++++ apps/extension/src/video/browser-fixture.ts | 20 ++- .../src/video/capture.browser.test.ts | 72 ++++++++--- .../src/video/encoder.browser.test.ts | 24 +++- .../src/video/playback-fixture.test.ts | 122 ++++++++++++++++-- apps/extension/src/video/playback-fixture.ts | 114 ++++++++++++++-- 7 files changed, 358 insertions(+), 53 deletions(-) create mode 100644 apps/extension/src/video/browser-fixture.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9902b89b..2cd4dd88 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -288,7 +288,7 @@ jobs: # Allows Chrome for Testing's user-namespace sandbox without a custom # AppArmor profile for its temporary installation path. runs-on: ubuntu-22.04 - timeout-minutes: 15 + timeout-minutes: 30 env: BSK_VIDEO_ARTIFACTS: ${{ github.workspace }}/evals/browser/results/video-regression diff --git a/apps/extension/src/video/browser-fixture.test.ts b/apps/extension/src/video/browser-fixture.test.ts new file mode 100644 index 00000000..f26361c5 --- /dev/null +++ b/apps/extension/src/video/browser-fixture.test.ts @@ -0,0 +1,57 @@ +// @vitest-environment node +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, expect, it, vi } from "vitest"; +import { saveVideoEvidence } from "./browser-fixture"; + +let directory: string | undefined; +afterEach(async () => { + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + if (directory) await rm(directory, { recursive: true, force: true }); + directory = undefined; +}); + +async function evidence() { + directory = await mkdtemp(path.join(tmpdir(), "bsk-video-evidence-test-")); + vi.stubEnv("BSK_VIDEO_ARTIFACTS", directory); + return { directory, warning: vi.spyOn(console, "warn").mockImplementation(() => {}) }; +} + +it("does not load optional video evidence when collection is disabled", async () => { + vi.stubEnv("BSK_VIDEO_ARTIFACTS", ""); + const load = vi.fn(); + await saveVideoEvidence("disabled", {}, load); + expect(load).not.toHaveBeenCalled(); +}); + +it("preserves the MP4 before a serialization failure without rejecting", async () => { + const { directory, warning } = await evidence(); + const details: { cycle?: unknown } = {}; + details.cycle = details; + await expect( + saveVideoEvidence("cycle", details, new Uint8Array([1, 2])), + ).resolves.toBeUndefined(); + expect(await readFile(path.join(directory, "cycle.mp4"))).toEqual(Buffer.from([1, 2])); + expect(warning).toHaveBeenCalled(); +}); + +it("warns without rejecting when the evidence directory is not writable", async () => { + const { directory, warning } = await evidence(); + const file = path.join(directory, "not-a-directory"); + await writeFile(file, "fixture"); + vi.stubEnv("BSK_VIDEO_ARTIFACTS", file); + await expect(saveVideoEvidence("disk-error", {})).resolves.toBeUndefined(); + expect(warning).toHaveBeenCalled(); +}); + +it("does not let optional browser export failures replace the test result", async () => { + const { warning } = await evidence(); + await expect( + saveVideoEvidence("export-error", {}, async () => { + throw new Error("CDP export failed"); + }), + ).resolves.toBeUndefined(); + expect(warning).toHaveBeenCalled(); +}); diff --git a/apps/extension/src/video/browser-fixture.ts b/apps/extension/src/video/browser-fixture.ts index 73b4e468..6d57bca0 100644 --- a/apps/extension/src/video/browser-fixture.ts +++ b/apps/extension/src/video/browser-fixture.ts @@ -51,17 +51,23 @@ export async function attachVideoBackground(send: BrowserSend, name: string) { return found!; } -/** Keep the MP4 before pixel assertions so a failed CI run remains diagnosable. */ +/** Evidence is best effort: its failure must never replace the regression's result. */ export async function saveVideoEvidence( name: string, details: unknown, - video?: string | Uint8Array, + video?: string | Uint8Array | (() => Promise), ) { const directory = process.env.BSK_VIDEO_ARTIFACTS; if (!directory) return; - await mkdir(directory, { recursive: true }); - await writeFile(path.join(directory, `${name}.json`), JSON.stringify(details, null, 2)); - const destination = path.join(directory, `${name}.mp4`); - if (typeof video === "string") await copyFile(video, destination); - else if (video) await writeFile(destination, video); + try { + await mkdir(directory, { recursive: true }); + // Save the MP4 first, even if diagnostic metadata cannot be serialized. + const destination = path.join(directory, `${name}.mp4`); + if (typeof video === "function") await writeFile(destination, await video()); + else if (typeof video === "string") await copyFile(video, destination); + else if (video) await writeFile(destination, video); + await writeFile(path.join(directory, `${name}.json`), JSON.stringify(details, null, 2)); + } catch (error) { + console.warn(`Could not save video evidence ${name}:`, error); + } } diff --git a/apps/extension/src/video/capture.browser.test.ts b/apps/extension/src/video/capture.browser.test.ts index e3072cd4..36162dcf 100644 --- a/apps/extension/src/video/capture.browser.test.ts +++ b/apps/extension/src/video/capture.browser.test.ts @@ -15,7 +15,7 @@ import { saveVideoEvidence, } from "./browser-fixture"; import { createDshVideoFixture } from "./dsh-fixture"; -import { readVideoPixels } from "./playback-fixture"; +import { readVideoPixels, videoFrameTargets } from "./playback-fixture"; const run = promisify(execFile); @@ -281,9 +281,13 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( { timeout: 10_000 }, ) .toBe(true); + const captureTargets = await videoFrameTargets(await readFile(output), [ + 0.2, + stopped.recording.duration_ms / 1000 - 0.3, + ]); const pixels = await evaluate( preview.sessionId, - `(${readVideoPixels.toString()})(document.querySelector('video'), [.2, document.querySelector('video').duration-.3])`, + `(${readVideoPixels.toString()})(document.querySelector('video'), ${JSON.stringify(captureTargets)})`, ); await saveVideoEvidence("capture", { version, recording: stopped.recording, pixels }); expect(pixels[0][0]).toBeGreaterThan(240); @@ -520,6 +524,24 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( ).toContain(result.stop_reason); } else { expect(result, route).toMatchObject({ state: "ready", completeness: "complete" }); + const routeFile = path.join(directory, `route-${route}.mp4`); + await cli( + "video", + "save", + "--recording", + recording.recording_id, + "--out", + routeFile, + ); + await saveVideoEvidence( + `route-${route}`, + { version, recording: result }, + routeFile, + ); + const duration = result.duration_ms / 1000; + const times: number[] = []; + for (let time = 0.1; time < duration - 0.1; time += 0.1) times.push(time); + const targets = await videoFrameTargets(await readFile(routeFile), times); await send( "Page.navigate", { url: `${origin}/video.html?id=${recording.recording_id}` }, @@ -535,13 +557,14 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( .toBe(true); const pixels = await evaluate( preview.sessionId, - `(async()=>{ - const v=document.querySelector('video'),c=document.createElement('canvas');c.width=v.videoWidth;c.height=v.videoHeight; - const ctx=c.getContext('2d'),pixels=[]; - for(let time=.1;timev.onseeked=r);ctx.drawImage(v,0,0);pixels.push(Array.from(ctx.getImageData(c.width/2,c.height/2,1,1).data));} - return pixels; - })()`, + `(${readVideoPixels.toString()})(document.querySelector('video'), ${JSON.stringify(targets)})`, ); + await saveVideoEvidence(`route-${route}`, { + version, + recording: result, + targets, + pixels, + }); // Navigation must never paint the neutral user-confirmation slate. if (!["error", "pdf"].includes(route)) expect( @@ -559,26 +582,35 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( // Sample faster than the 15 fps capture rate, including the // initial frames. Uniform page colors make any control pill // or orange breathing border visible as a pixel deviation. - const deviation = await evaluate( + const overlayTimes: number[] = []; + for (let time = 0.01; time < duration - 0.01; time += 1 / 30) + overlayTimes.push(time); + const overlayTargets = await videoFrameTargets( + await readFile(routeFile), + overlayTimes, + ); + const deviations = await evaluate( preview.sessionId, `(async()=>{ const v=document.querySelector('video'),c=document.createElement('canvas'); c.width=160;c.height=Math.round(160*v.videoHeight/v.videoWidth); - const ctx=c.getContext('2d');let deviation=0; - for(let time=.01;timev.onseeked=r);ctx.drawImage(v,0,0,c.width,c.height); + const ctx=c.getContext('2d'); + return (${readVideoPixels.toString()})(v, ${JSON.stringify(overlayTargets)}, () => { + ctx.drawImage(v,0,0,c.width,c.height); const data=ctx.getImageData(0,0,c.width,c.height).data; const center=4*(Math.floor(c.height/2)*c.width+Math.floor(c.width/2)); + let deviation=0; for(let y=2;y=c.width-4||y<4||y>=c.height-4; const pill=y>c.height*.75&&x>c.width*.25&&x setTimeout(resolve, 1200)); const finished = await dsh.call<{ - recording: { state: string; completeness: string; byte_size: number }; + recording: { + state: string; + completeness: string; + byte_size: number; + duration_ms: number; + }; }>("bsk_inspect", { action: "video", videoAction: "stop", recordingId }); expect(finished.recording.state).toBe("ready"); expect(finished.recording.completeness).toBe("complete"); @@ -833,9 +870,12 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( }); // The preview intentionally preloads only metadata. Seek to // request and verify a decoded frame from the completed task. + const dshTargets = await videoFrameTargets(await readFile(output), [ + finished.recording.duration_ms / 1000 - 0.2, + ]); const [pixel] = await evaluate( preview.sessionId, - `(${readVideoPixels.toString()})(document.querySelector('video'), [document.querySelector('video').duration-.2])`, + `(${readVideoPixels.toString()})(document.querySelector('video'), ${JSON.stringify(dshTargets)})`, ); await saveVideoEvidence("capture-dsh", { version, diff --git a/apps/extension/src/video/encoder.browser.test.ts b/apps/extension/src/video/encoder.browser.test.ts index a3e5e41d..ac975b29 100644 --- a/apps/extension/src/video/encoder.browser.test.ts +++ b/apps/extension/src/video/encoder.browser.test.ts @@ -44,7 +44,7 @@ globalThis.runEncoder = async (crash = false, width = 320, height = 180, quality const player = ` import { VideoArtifactStore } from '@/video/store'; -import { readVideoPixels } from '@/video/playback-fixture'; +import { readVideoPixels, videoFrameTargets } from '@/video/playback-fixture'; globalThis.exportVideo=async(id)=>{ const bytes=new Uint8Array(await (await new VideoArtifactStore().file(id)).arrayBuffer()); let binary='';for(let i=0;i{ const video=document.createElement('video');video.muted=true;document.body.append(video); const url=URL.createObjectURL(file);video.src=url; await new Promise((resolve,reject)=>{video.onloadeddata=resolve;video.onerror=()=>reject(Error('MP4 playback failed'));}); - const pixels=await readVideoPixels(video,[.2,video.duration-.2]); + // Exercise a no-op seek at the initial position, repeated targets and two + // requested times within one sparse frame in the real browser too. + const targets=await videoFrameTargets(file,[0,0,.2,.2,video.duration-.2]); + const samples=await readVideoPixels(video,targets); + if(JSON.stringify(samples[0])!==JSON.stringify(samples[1])||JSON.stringify(samples[2])!==JSON.stringify(samples[3]))throw Error('Repeated frame samples disagree'); + const pixels=[samples[2],samples[4]]; const result={duration:video.duration,width:video.videoWidth,height:video.videoHeight,pixels}; URL.revokeObjectURL(url);video.remove();return result; }; @@ -172,11 +177,16 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME)("video encoder browser regression const version = await send("Browser.getVersion"); const inspect = async (id: string, name: string, recording: unknown) => { if (process.env.BSK_VIDEO_ARTIFACTS) { - const data = await evaluate( - attached.sessionId, - `exportVideo(${JSON.stringify(id)})`, - ); - await saveVideoEvidence(name, { version, recording }, Buffer.from(data, "base64")); + // Persist the file and initial metadata before playback; update + // the JSON below only after sampling succeeds. Failed playback + // must still leave the original MP4 and recording details. + await saveVideoEvidence(name, { version, recording }, async () => { + const data = await evaluate( + attached.sessionId, + `exportVideo(${JSON.stringify(id)})`, + ); + return Buffer.from(data, "base64"); + }); } const playback = await evaluate<{ duration: number; diff --git a/apps/extension/src/video/playback-fixture.test.ts b/apps/extension/src/video/playback-fixture.test.ts index 66ea3976..32f10b7e 100644 --- a/apps/extension/src/video/playback-fixture.test.ts +++ b/apps/extension/src/video/playback-fixture.test.ts @@ -1,19 +1,36 @@ +import { + BufferTarget, + EncodedPacket, + EncodedVideoPacketSource, + Mp4OutputFormat, + Output, +} from "mediabunny"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { readVideoPixels } from "./playback-fixture"; +import { readVideoPixels, type VideoFrameTarget, videoFrameTargets } from "./playback-fixture"; afterEach(() => { vi.restoreAllMocks(); vi.useRealTimers(); }); +const target: VideoFrameTarget = { time: 1.5, timestamp: 1, duration: 1 }; + function player() { const video = document.createElement("video"); + Object.defineProperties(video, { + duration: { configurable: true, value: 3 }, + readyState: { configurable: true, value: 2 }, + videoWidth: { value: 320 }, + videoHeight: { value: 180 }, + }); let present!: VideoFrameRequestCallback; + let sequence = 0; video.requestVideoFrameCallback = vi.fn((callback) => { present = callback; - return 1; + return ++sequence; }); video.cancelVideoFrameCallback = vi.fn(); + const removeListener = vi.spyOn(video, "removeEventListener"); vi.spyOn(video, "pause").mockImplementation(() => {}); const drawImage = vi.fn(); vi.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue({ @@ -23,8 +40,10 @@ function player() { return { video, drawImage, + removeListener, seeked: () => video.dispatchEvent(new Event("seeked")), - present: () => present(0, {} as VideoFrameCallbackMetadata), + present: (mediaTime = target.timestamp) => + present(0, { mediaTime } as VideoFrameCallbackMetadata), }; } @@ -34,7 +53,7 @@ describe("video pixel sampling", () => { "present", ] as const)("waits for both signals when %s arrives first", async (first) => { const fixture = player(); - const pixels = readVideoPixels(fixture.video, [1.5]); + const pixels = readVideoPixels(fixture.video, [target]); fixture[first](); await Promise.resolve(); expect(fixture.drawImage).not.toHaveBeenCalled(); @@ -42,17 +61,104 @@ describe("video pixel sampling", () => { await expect(pixels).resolves.toEqual([[0, 0, 255, 255]]); expect(fixture.drawImage).toHaveBeenCalledTimes(1); expect(fixture.video.cancelVideoFrameCallback).toHaveBeenCalledWith(1); + expect(fixture.removeListener).toHaveBeenCalledWith("seeked", expect.any(Function)); + expect(fixture.removeListener).toHaveBeenCalledWith("error", expect.any(Function)); }); - it("fails instead of reading a stale frame when presentation never arrives", async () => { - vi.useFakeTimers(); + it("ignores a previous frame's callback and accepts the target's PTS, not the seek time", async () => { const fixture = player(); - const pixels = readVideoPixels(fixture.video, [1.5]); - const rejected = expect(pixels).rejects.toThrow("seeked=true, presented=false"); + const pixels = readVideoPixels(fixture.video, [target]); fixture.seeked(); + fixture.present(0); + await Promise.resolve(); + expect(fixture.drawImage).not.toHaveBeenCalled(); + expect(fixture.video.requestVideoFrameCallback).toHaveBeenCalledTimes(2); + fixture.present(1); // The frame covers [1, 2), including the requested 1.5s. + await expect(pixels).resolves.toEqual([[0, 0, 255, 255]]); + }); + + it("moves a same-position seek within the target frame and reuses verified duplicate frames", async () => { + const fixture = player(); + fixture.video.currentTime = target.time; + const pixels = readVideoPixels(fixture.video, [target, target, { ...target, time: 1.9 }]); + expect(fixture.video.currentTime).not.toBe(target.time); + expect(fixture.video.currentTime).toBeGreaterThanOrEqual(1); + expect(fixture.video.currentTime).toBeLessThan(2); + fixture.seeked(); + fixture.present(); + await expect(pixels).resolves.toHaveLength(3); + expect(fixture.video.requestVideoFrameCallback).toHaveBeenCalledTimes(1); + }); + + it.each([ + "seeked", + "present", + ] as const)("fails without sampling when only %s arrives", async (signal) => { + vi.useFakeTimers(); + const fixture = player(); + const pixels = readVideoPixels(fixture.video, [target]); + const rejected = expect(pixels).rejects.toThrow("did not become ready"); + fixture[signal](); await vi.advanceTimersByTimeAsync(5000); await rejected; expect(fixture.drawImage).not.toHaveBeenCalled(); expect(fixture.video.cancelVideoFrameCallback).toHaveBeenCalledWith(1); + expect(fixture.removeListener).toHaveBeenCalledWith("seeked", expect.any(Function)); + expect(fixture.removeListener).toHaveBeenCalledWith("error", expect.any(Function)); + expect(vi.getTimerCount()).toBe(0); + }); + + it("reports media errors and cleans up without waiting for the timeout", async () => { + vi.useFakeTimers(); + const fixture = player(); + Object.defineProperty(fixture.video, "error", { value: { message: "decode failed" } }); + const pixels = readVideoPixels(fixture.video, [target]); + fixture.video.dispatchEvent(new Event("error")); + await expect(pixels).rejects.toThrow("MP4 playback failed: decode failed"); + expect(fixture.drawImage).not.toHaveBeenCalled(); + expect(fixture.video.cancelVideoFrameCallback).toHaveBeenCalledWith(1); + expect(vi.getTimerCount()).toBe(0); + }); + + it.each([-1, 3, NaN, Infinity])("rejects invalid time %s before seeking", async (time) => { + const fixture = player(); + await expect(readVideoPixels(fixture.video, [{ ...target, time }])).rejects.toThrow("outside"); + expect(fixture.video.requestVideoFrameCallback).not.toHaveBeenCalled(); + }); + + it("rejects unavailable duration before seeking", async () => { + const fixture = player(); + Object.defineProperty(fixture.video, "duration", { value: NaN }); + await expect(readVideoPixels(fixture.video, [target])).rejects.toThrow("metadata is not ready"); + expect(fixture.video.requestVideoFrameCallback).not.toHaveBeenCalled(); + }); +}); + +it("resolves times within sparse MP4 frames and rejects times outside the recording", async () => { + const destination = new BufferTarget(); + const output = new Output({ target: destination, format: new Mp4OutputFormat() }); + const source = new EncodedVideoPacketSource("avc"); + output.addVideoTrack(source); + await output.start(); + // Demuxing only: sample payloads are never sent to a decoder in this test. + const config = { + codec: "avc1.42001e", + codedWidth: 320, + codedHeight: 180, + description: new Uint8Array([1, 66, 0, 30, 255, 225, 0, 4, 103, 66, 0, 30, 1, 0, 2, 104, 0]), + }; + await source.add(new EncodedPacket(new Uint8Array([0, 0, 0, 1, 101]), "key", 0, 1), { + decoderConfig: config, }); + await source.add(new EncodedPacket(new Uint8Array([0, 0, 0, 1, 101]), "key", 1, 2)); + source.close(); + await output.finalize(); + const file = new Uint8Array(destination.buffer!); + await expect(videoFrameTargets(file, [0.2, 1.2, 2.8])).resolves.toEqual([ + { time: 0.2, timestamp: 0, duration: 1 }, + { time: 1.2, timestamp: 1, duration: 2 }, + { time: 2.8, timestamp: 1, duration: 2 }, + ]); + await expect(videoFrameTargets(file, [-0.3])).rejects.toThrow("outside"); + await expect(videoFrameTargets(file, [3])).rejects.toThrow("outside"); }); diff --git a/apps/extension/src/video/playback-fixture.ts b/apps/extension/src/video/playback-fixture.ts index 176ffdaa..2a470367 100644 --- a/apps/extension/src/video/playback-fixture.ts +++ b/apps/extension/src/video/playback-fixture.ts @@ -1,18 +1,88 @@ -/** Runs in the player page (also serialized into CDP Runtime.evaluate). */ -export async function readVideoPixels(video: HTMLVideoElement, times: number[]) { +import { BlobSource, BufferSource, EncodedPacketSink, Input, MP4 } from "mediabunny"; + +export interface VideoFrameTarget { + time: number; + timestamp: number; + duration: number; +} + +/** Resolve the actual MP4 frame covering each requested time, including long static frames. */ +export async function videoFrameTargets(file: Blob | Uint8Array, times: number[]) { + const input = new Input({ + source: file instanceof Blob ? new BlobSource(file) : new BufferSource(file), + formats: [MP4], + }); + try { + const track = await input.getPrimaryVideoTrack(); + if (!track) throw new Error("MP4 has no video track"); + const duration = await input.computeDuration(); + const sink = new EncodedPacketSink(track); + const targets: VideoFrameTarget[] = []; + for (const time of times) { + if (!Number.isFinite(time) || time < 0 || time >= duration) + throw new Error(`MP4 sample time ${time}s is outside [0, ${duration})`); + const packet = await sink.getPacket(time, { metadataOnly: true }); + if (!packet || packet.duration <= 0 || time >= packet.timestamp + packet.duration) + throw new Error(`MP4 has no frame covering ${time}s`); + targets.push({ time, timestamp: packet.timestamp, duration: packet.duration }); + } + return targets; + } finally { + input.dispose(); + } +} + +/** + * Also serialized with .toString() into CDP Runtime.evaluate: keep this function + * self-contained, with no module variables/imports referenced at runtime. + * sample, when supplied, must likewise be defined in the player page. + */ +export async function readVideoPixels( + video: HTMLVideoElement, + targets: VideoFrameTarget[], + sample?: (video: HTMLVideoElement) => number[], +) { + if (video.readyState < 1 || !Number.isFinite(video.duration) || video.duration <= 0) + throw new Error("MP4 metadata is not ready"); + for (const { time, timestamp, duration } of targets) { + if (!Number.isFinite(time) || time < 0 || time >= video.duration) + throw new Error(`MP4 sample time ${time}s is outside [0, ${video.duration})`); + if ( + !Number.isFinite(timestamp) || + !Number.isFinite(duration) || + duration <= 0 || + timestamp < 0 || + time < timestamp || + time >= timestamp + duration + ) + throw new Error(`Invalid MP4 frame interval for ${time}s`); + } video.pause(); const canvas = document.createElement("canvas"); canvas.width = video.videoWidth; canvas.height = video.videoHeight; const context = canvas.getContext("2d")!; const pixels: number[][] = []; - for (const time of times) { - // seeked can precede presentation of the decoded frame. In particular, - // drawImage immediately after seeking can still copy the previous frame. - // Subscribe before seeking so neither event can be missed on a fast decode. + let previous: { timestamp: number; pixels: number[] } | undefined; + for (const { time, timestamp, duration } of targets) { + // Dense scans often request the same encoded frame several times. Reuse + // only a frame whose presentation was already verified in this call. + if (previous?.timestamp === timestamp) { + pixels.push([...previous.pixels]); + continue; + } + // A no-op seek need not produce seeked/rVFC. Move within the SAME encoded + // frame when the requested time is already current, rather than timing out. + let seekTime = time; + if (seekTime === video.currentTime) { + const end = Math.min(timestamp + duration, video.duration); + seekTime = timestamp + (end - timestamp) / 2; + if (seekTime === video.currentTime) seekTime = timestamp + (end - timestamp) / 4; + } await new Promise((resolve, reject) => { let seeked = false; let presented = false; + let mediaTime: number | undefined; let callback = 0; const cleanup = () => { clearTimeout(timer); @@ -33,24 +103,40 @@ export async function readVideoPixels(video: HTMLVideoElement, times: number[]) cleanup(); reject(new Error(`MP4 playback failed: ${video.error?.message}`)); }; + const onFrame: VideoFrameRequestCallback = (_now, metadata) => { + mediaTime = metadata.mediaTime; + // rVFC reports the frame's PTS, not the seek position. A previous + // frame's queued callback must not satisfy a seek to a different frame. + presented = Math.abs(mediaTime - timestamp) < 0.0001; + if (presented) complete(); + else callback = video.requestVideoFrameCallback(onFrame); + }; const timer = setTimeout(() => { cleanup(); reject( new Error( - `MP4 frame at ${time}s did not become ready (seeked=${seeked}, presented=${presented}, readyState=${video.readyState})`, + `MP4 frame at ${time}s did not become ready (expected PTS=${timestamp}, actual PTS=${mediaTime}, seeked=${seeked}, presented=${presented}, readyState=${video.readyState})`, ), ); }, 5000); video.addEventListener("seeked", onSeeked); video.addEventListener("error", onError); - callback = video.requestVideoFrameCallback(() => { - presented = true; - complete(); - }); - video.currentTime = time; + callback = video.requestVideoFrameCallback(onFrame); + try { + video.currentTime = seekTime; + } catch (error) { + cleanup(); + reject(error); + } }); - context.drawImage(video, 0, 0); - pixels.push(Array.from(context.getImageData(canvas.width / 2, canvas.height / 2, 1, 1).data)); + let pixel: number[]; + if (sample) pixel = sample(video); + else { + context.drawImage(video, 0, 0); + pixel = Array.from(context.getImageData(canvas.width / 2, canvas.height / 2, 1, 1).data); + } + previous = { timestamp, pixels: pixel }; + pixels.push(pixel); } return pixels; } From d689515216178a9da67308889d5ae1b13a1f2fff Mon Sep 17 00:00:00 2001 From: drakezhang Date: Tue, 27 Oct 2026 12:00:09 +0800 Subject: [PATCH 4/4] test(video): keep optional screenshots from failing regressions --- .../src/video/capture.browser.test.ts | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/apps/extension/src/video/capture.browser.test.ts b/apps/extension/src/video/capture.browser.test.ts index 36162dcf..1ea9075c 100644 --- a/apps/extension/src/video/capture.browser.test.ts +++ b/apps/extension/src/video/capture.browser.test.ts @@ -295,16 +295,20 @@ describe.skipIf(!process.env.BSK_VIDEO_CHROME || !process.env.BSK_VIDEO_BSK)( expect(pixels[1][0]).toBeLessThan(15); const screenshot = async (sessionId: string, name: string) => { if (!process.env.BSK_VIDEO_ARTIFACTS) return; - await mkdir(process.env.BSK_VIDEO_ARTIFACTS, { recursive: true }); - const capture = await send<{ data: string }>( - "Page.captureScreenshot", - { format: "png" }, - sessionId, - ); - await writeFile( - path.join(process.env.BSK_VIDEO_ARTIFACTS, name), - Buffer.from(capture.data, "base64"), - ); + try { + await mkdir(process.env.BSK_VIDEO_ARTIFACTS, { recursive: true }); + const capture = await send<{ data: string }>( + "Page.captureScreenshot", + { format: "png" }, + sessionId, + ); + await writeFile( + path.join(process.env.BSK_VIDEO_ARTIFACTS, name), + Buffer.from(capture.data, "base64"), + ); + } catch (error) { + console.warn(`Could not save video screenshot ${name}:`, error); + } }; await screenshot(preview.sessionId, "preview.png"); // Exercise the real popup entry and its recording card.