From 760794df56aead20992a566b486fddf909f2eee8 Mon Sep 17 00:00:00 2001 From: Yusuf Mohsinally <463376+yusufm@users.noreply.github.com> Date: Sun, 3 May 2026 14:17:26 -0700 Subject: [PATCH 1/3] Upload Electron e2e failure artifacts --- .github/workflows/ci.yml | 37 +++++++++++++++++++++++++++------- src/hooks/useScreenRecorder.ts | 18 ++++++++--------- tests/e2e/gif-export.spec.ts | 32 +++++++++++++++++++++++++++-- 3 files changed, 69 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 41947974a..d0ed60fa0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,14 +44,37 @@ jobs: - run: npm run test:browser:install - run: npm run test:browser - build: - name: Build - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 + build: + name: Build + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: node-version: 22 cache: npm - - run: npm ci - - run: npx vite build + - run: npm ci + - run: npx vite build + + electron-e2e: + name: Electron E2E + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + - run: npm ci + - run: npm run build-vite + - run: npx playwright install --with-deps + - run: xvfb-run -a npm run test:e2e + - name: Upload Playwright artifacts + if: failure() + uses: actions/upload-artifact@v4 + with: + name: electron-e2e-artifacts + path: | + test-results/ + playwright-report/ + if-no-files-found: ignore diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index dc9758fbd..f14be62f2 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -408,6 +408,14 @@ export function useScreenRecorder(): UseScreenRecorderReturn { } }); + const safeHideCountdownOverlay = useCallback(async (runId: number) => { + try { + await window.electronAPI.hideCountdownOverlay(runId); + } catch (error) { + console.warn("Failed to hide countdown overlay:", error); + } + }, []); + useEffect(() => { let cleanup: (() => void) | undefined; @@ -450,7 +458,7 @@ export function useScreenRecorder(): UseScreenRecorderReturn { webcamRecorder.current = null; teardownMedia(); }; - }, [teardownMedia]); + }, [teardownMedia, safeHideCountdownOverlay]); const safeShowCountdownOverlay = async (value: number, runId: number) => { try { @@ -477,14 +485,6 @@ export function useScreenRecorder(): UseScreenRecorderReturn { } }; - const safeHideCountdownOverlay = async (runId: number) => { - try { - await window.electronAPI.hideCountdownOverlay(runId); - } catch (error) { - console.warn("Failed to hide countdown overlay:", error); - } - }; - const isCountdownRunActive = (runId?: number) => runId === undefined || countdownRunId.current === runId; diff --git a/tests/e2e/gif-export.spec.ts b/tests/e2e/gif-export.spec.ts index d1fa3f7aa..365b6e9a8 100644 --- a/tests/e2e/gif-export.spec.ts +++ b/tests/e2e/gif-export.spec.ts @@ -2,13 +2,41 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { _electron as electron, expect, test } from "@playwright/test"; +import { type ElectronApplication, _electron as electron, expect, test } from "@playwright/test"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const ROOT = path.join(__dirname, "../.."); const MAIN_JS = path.join(ROOT, "dist-electron/main.js"); const TEST_VIDEO = path.join(__dirname, "../fixtures/sample.webm"); +async function waitForProcessExit( + child: ReturnType, + timeoutMs: number, +) { + if (child.exitCode !== null || child.killed) return; + + await Promise.race([ + new Promise((resolve) => child.once("exit", () => resolve())), + new Promise((resolve) => setTimeout(resolve, timeoutMs)), + ]); +} + +async function closeElectronApp(app: ElectronApplication) { + const child = app.process(); + await app + .evaluate(({ app: electronApp }) => { + electronApp.exit(0); + }) + .catch(() => { + // App may already be closing. + }); + await waitForProcessExit(child, 2_000); + if (child.exitCode === null && !child.killed) { + child.kill("SIGKILL"); + await waitForProcessExit(child, 2_000); + } +} + test("exports a GIF from a loaded video", async () => { const outputPath = path.join(os.tmpdir(), `test-gif-export-${Date.now()}.gif`); let testVideoInRecordings = ""; @@ -126,7 +154,7 @@ test("exports a GIF from a loaded video", async () => { const stats = fs.statSync(outputPath); expect(stats.size).toBeGreaterThan(1024); // at least 1 KB } finally { - await app.close(); + await closeElectronApp(app); if (fs.existsSync(outputPath)) { fs.unlinkSync(outputPath); } From 5bed560be70c103d0a29a4083981bc1b007eac68 Mon Sep 17 00:00:00 2001 From: Yusuf Mohsinally <463376+yusufm@users.noreply.github.com> Date: Sun, 3 May 2026 14:27:27 -0700 Subject: [PATCH 2/3] Fix Electron E2E shutdown wait --- tests/e2e/gif-export.spec.ts | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/tests/e2e/gif-export.spec.ts b/tests/e2e/gif-export.spec.ts index 365b6e9a8..0f9f23658 100644 --- a/tests/e2e/gif-export.spec.ts +++ b/tests/e2e/gif-export.spec.ts @@ -13,12 +13,25 @@ async function waitForProcessExit( child: ReturnType, timeoutMs: number, ) { - if (child.exitCode !== null || child.killed) return; - - await Promise.race([ - new Promise((resolve) => child.once("exit", () => resolve())), - new Promise((resolve) => setTimeout(resolve, timeoutMs)), - ]); + if (child.exitCode !== null) return; + + await new Promise((resolve) => { + let timer: ReturnType; + const cleanup = () => { + clearTimeout(timer); + child.removeListener("exit", onExit); + }; + const finish = () => { + cleanup(); + resolve(); + }; + const onExit = () => finish(); + const onTimeout = () => finish(); + + timer = setTimeout(onTimeout, timeoutMs); + child.once("exit", onExit); + if (child.exitCode !== null) finish(); + }); } async function closeElectronApp(app: ElectronApplication) { @@ -31,7 +44,7 @@ async function closeElectronApp(app: ElectronApplication) { // App may already be closing. }); await waitForProcessExit(child, 2_000); - if (child.exitCode === null && !child.killed) { + if (child.exitCode === null) { child.kill("SIGKILL"); await waitForProcessExit(child, 2_000); } From 8fcdc4fc5c721281ee6b1ec56fc0c3fc25d13c56 Mon Sep 17 00:00:00 2001 From: Yusuf Mohsinally <463376+yusufm@users.noreply.github.com> Date: Sun, 3 May 2026 18:08:22 -0700 Subject: [PATCH 3/3] Tighten e2e process shutdown handling --- tests/e2e/gif-export.spec.ts | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/e2e/gif-export.spec.ts b/tests/e2e/gif-export.spec.ts index 0f9f23658..890a9bdd3 100644 --- a/tests/e2e/gif-export.spec.ts +++ b/tests/e2e/gif-export.spec.ts @@ -13,24 +13,24 @@ async function waitForProcessExit( child: ReturnType, timeoutMs: number, ) { - if (child.exitCode !== null) return; + if (child.exitCode !== null) return true; - await new Promise((resolve) => { + return new Promise((resolve) => { let timer: ReturnType; const cleanup = () => { clearTimeout(timer); child.removeListener("exit", onExit); }; - const finish = () => { + const finish = (exited: boolean) => { cleanup(); - resolve(); + resolve(exited); }; - const onExit = () => finish(); - const onTimeout = () => finish(); + const onExit = () => finish(true); + const onTimeout = () => finish(false); timer = setTimeout(onTimeout, timeoutMs); child.once("exit", onExit); - if (child.exitCode !== null) finish(); + if (child.exitCode !== null) finish(true); }); } @@ -43,10 +43,15 @@ async function closeElectronApp(app: ElectronApplication) { .catch(() => { // App may already be closing. }); - await waitForProcessExit(child, 2_000); + const exited = await waitForProcessExit(child, 2_000); if (child.exitCode === null) { child.kill("SIGKILL"); - await waitForProcessExit(child, 2_000); + const killed = await waitForProcessExit(child, 2_000); + if (!killed) { + throw new Error("Electron process did not exit after SIGKILL"); + } + } else if (!exited) { + throw new Error("Electron process exit timed out"); } }