Fixes for issues exposed by testing

beaker-vue/package.json

@@ -14,7 +14,8 @@
     "build-and-serve": "run-s build serve",
     "routes": "NODE_ENV=production vite-node utils/extractRoutes.ts",
     "preview": "vite preview",
-    "test:unit": "vitest",
+    "test:unit": "vitest run",
+    "test:unit:watch": "vitest",
     "test:e2e": "playwright test",
     "prepack": "vite-node utils/prepack",
     "postpack": "vite-node utils/postpack",

beaker-vue/playwright.config.ts

@@ -18,8 +18,12 @@ export default defineConfig({
   // be cleanly separated from vitest unit tests (`src/**/__tests__/`) to avoid
   // the two runners collecting each other's specs.
   testDir: './tests',
+  /* Record pre-existing kernels, then reap anything the run leaked. See
+   * tests/global-setup.ts and tests/global-teardown.ts. */
+  globalSetup: './tests/global-setup.ts',
+  globalTeardown: './tests/global-teardown.ts',
   /* Maximum time one test can run for. */
-  timeout: 60 * 1000,
+  timeout: 120 * 1000,
   expect: {
     /**
      * Maximum time expect() should wait for the condition to be met.
@@ -31,10 +35,17 @@ export default defineConfig({
   forbidOnly: !!process.env.CI,
   /* Retry on CI only */
   retries: process.env.CI ? 2 : 0,
-  /* Opt out of parallel tests on CI. */
-  workers: process.env.CI ? 1 : undefined,
-  /* Reporter to use. See https://playwright.dev/docs/test-reporters */
-  reporter: 'html',
+  /* Each test spins up its own kernel (beaker kernel + subkernel), so concurrency
+   * is bounded by machine resources rather than by CPU count. Left unbounded,
+   * every added test widens the startup pile-up and pushes the per-assertion
+   * timeouts below. */
+  workers: 3,
+  /* Reporter to use. See https://playwright.dev/docs/test-reporters
+   * `list` gives live progress; the HTML report is still written, but never
+   * auto-opened -- the html reporter's default (`open: 'on-failure'`) starts a
+   * report server that blocks the terminal after a failed run. View it on
+   * demand with `npx playwright show-report`. */
+  reporter: [['list'], ['html', {open: 'never'}]],
   /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
   use: {
     /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */

beaker-vue/tests/attachments.spec.ts

@@ -1,5 +1,21 @@
 import {expect, test, type Page} from '@playwright/test';
 
+import {deleteSessionByName, e2eSessionName, resolveBaseUrl} from './helpers/session';
+
+/**
+ * This spec names its own session rather than going through the `sessionPage`
+ * fixture in example.spec.ts, because it asserts on session-scoped attachment
+ * state and needs the id up front. Cleanup is the same idea either way.
+ * `afterEach` also runs when the test fails, which is when a leaked kernel is
+ * most likely.
+ */
+let createdSession: string | null = null;
+
+test.afterEach(async () => {
+  if (!createdSession) return;
+  await deleteSessionByName(createdSession);
+  createdSession = null;
+});
 
 type BrowserFile = {
   name: string;
@@ -22,8 +38,9 @@ async function dropFiles(page: Page, files: BrowserFile[]) {
 }
 
 test('notebook chat attachments upload, remove, extract ZIPs, and send without text', async ({page}) => {
-  const baseUrl = process.env.BEAKER_E2E_URL ?? 'http://localhost:8888';
-  const sessionId = `attachment-e2e-${Date.now()}`;
+  const baseUrl = resolveBaseUrl();
+  const sessionId = e2eSessionName(`attachments-${Date.now()}`);
+  createdSession = sessionId;
   await page.goto(`${baseUrl}/?session=${sessionId}`);
   await expect(page.locator('span.status-label')).toHaveText('Ready', {timeout: 30_000});
 

beaker-vue/tests/example.spec.ts

@@ -1,12 +1,38 @@
 import { test as base, expect, type Page } from '@playwright/test';
 
+import { deleteSessionByName, e2eSessionName, resolveBaseUrl } from './helpers/session';
+
+/**
+ * One session per test, deleted as soon as the test finishes.
+ *
+ * Navigating to `/` (as this fixture used to) makes the server redirect to a
+ * freshly generated `?session=<uuid>`, so every test already got its own
+ * session -- it just had no name the suite could recognise afterwards, so
+ * nothing was ever cleaned up. Naming the session ourselves keeps that
+ * isolation and makes teardown a direct delete.
+ *
+ * Sharing one session per worker was tried and does not work: notebook state
+ * persists per session, so the second test on a worker inherits the previous
+ * test's notebook instead of a fresh one, and the `.code-cell` locators find
+ * nothing. Isolation here is load-bearing, not incidental.
+ *
+ * Deleting the session shuts down its kernel and that kernel's subkernel, so
+ * live kernels track the worker count rather than the test count.
+ *
+ * The name must be unique per *run*, not just per test: notebook state is
+ * snapshotted to disk under the session name and outlives the session itself,
+ * so a name derived from testId alone would make each run restore the previous
+ * run's notebook -- which shows up as cells the test didn't expect.
+ */
 const test = base.extend<{sessionPage: Page}>(
   {
-    sessionPage: async ({page}, use) => {
-      await page.goto('http://localhost:8888/');
+    sessionPage: async ({page}, use, testInfo) => {
+      const sessionName = e2eSessionName(`${testInfo.testId}-${Date.now()}`);
+      await page.goto(`${resolveBaseUrl()}/?session=${encodeURIComponent(sessionName)}`);
       await expect(page.locator("span.status-label")).toBeVisible();
-      await expect(page.locator("span.status-label")).toHaveText("Ready", {timeout: 20_000});
+      await expect(page.locator("span.status-label")).toHaveText("Ready", {timeout: 45_000});
       await use(page);
+      await deleteSessionByName(sessionName);
     }
   }
 );
@@ -17,9 +43,9 @@ test('Agent: Say Hello', async ({ sessionPage }) => {
   await expect(sessionPage.getByPlaceholder("Ask the AI or request an operation.")).toBeVisible();
   await sessionPage.getByPlaceholder("Ask the AI or request an operation.").fill("say hello");
   await sessionPage.getByLabel("Submit").click();
-  await expect(sessionPage.getByText("Beaker Agent")).toBeVisible({timeout: 10_000});
+  await expect(sessionPage.getByText("Beaker Agent")).toBeVisible({timeout: 30_000});
   await expect(sessionPage.locator(".agent-cell-content")).toBeVisible();
-  await expect(sessionPage.locator(".agent-cell-content")).toContainText("hello", {ignoreCase: true});
+  await expect(sessionPage.locator(".agent-cell-content")).toContainText("hello", {ignoreCase: true, timeout: 60_000});
 });
 
 test('Agent: Run Code', async ({ sessionPage }) => {
@@ -28,9 +54,9 @@ test('Agent: Run Code', async ({ sessionPage }) => {
     .getByPlaceholder("Ask the AI or request an operation.")
     .fill("use the python tool to compute 21 + 115");
   await sessionPage.getByLabel("Submit").click();
-  await expect(sessionPage.getByText("Beaker Agent")).toBeVisible({timeout: 10_000});
+  await expect(sessionPage.getByText("Beaker Agent")).toBeVisible({timeout: 30_000});
   await expect(sessionPage.locator(".code-cell")).toBeVisible();
-  await expect(sessionPage.locator(".code-cell-output-box")).toContainText(`${21 + 115}`, {timeout: 10_000});
+  await expect(sessionPage.locator(".code-cell-output-box")).toContainText(`${21 + 115}`, {timeout: 60_000});
 });
 
 test('Code Cell: Execute Python (control+enter)', async ({ sessionPage }) => {
@@ -77,10 +103,10 @@ test('File Browser: Show README.md with metadata', async ({ sessionPage }) => {
   await filesTabButton.click();
 
   const filePanel = sessionPage.locator('.file-container');
-  await expect(filePanel).toBeVisible({timeout: 10_000});
+  await expect(filePanel).toBeVisible({timeout: 30_000});
 
   const fileTable = sessionPage.locator('.file-table');
-  await expect(fileTable).toBeVisible();
+  await expect(fileTable).toBeVisible({timeout: 30_000});
 
   const readmeRow = fileTable.getByRole('row', { name: /README\.md/ });
   await expect(readmeRow).toBeVisible();

beaker-vue/tests/global-setup.ts

@@ -0,0 +1,28 @@
+/**
+ * Records which kernels already existed before the suite ran.
+ *
+ * `global-teardown.ts` uses this baseline so its kernel sweep only reaps what
+ * the run created. Without it, a sweep on a shared or local dev server would
+ * kill kernels a developer is actively using.
+ */
+import {mkdirSync, writeFileSync} from 'node:fs';
+import {dirname} from 'node:path';
+
+import {BASELINE_PATH, createApiContext, listKernelIds} from './helpers/session';
+
+export default async function globalSetup(): Promise<void> {
+  let kernelIds: string[] = [];
+  const api = await createApiContext();
+  try {
+    kernelIds = await listKernelIds(api);
+  } catch (error) {
+    // A server that isn't up yet is the tests' problem to report, not setup's.
+    console.warn('[e2e setup] could not read pre-existing kernels:', error);
+  } finally {
+    await api.dispose();
+  }
+
+  mkdirSync(dirname(BASELINE_PATH), {recursive: true});
+  writeFileSync(BASELINE_PATH, JSON.stringify({kernelIds}, null, 2));
+  console.log(`[e2e setup] ${kernelIds.length} pre-existing kernel(s) recorded; they will be left alone.`);
+}

beaker-vue/tests/global-teardown.ts

@@ -0,0 +1,72 @@
+/**
+ * Backstop cleanup for the e2e suite.
+ *
+ * Each test already deletes its own session as it finishes; this exists for the
+ * cases that can't cover -- a crashed worker, a killed run, or a session whose
+ * kernel outlived its session record.
+ *
+ * Two passes, in order:
+ *   1. Delete every session named with the e2e prefix. Deleting a Beaker
+ *      session shuts down its kernel, which shuts down that kernel's subkernel.
+ *   2. Delete any kernel that wasn't present before the run and isn't attached
+ *      to a surviving session -- i.e. orphans left by a shutdown that didn't
+ *      cascade. Kernels recorded by global setup are never touched.
+ */
+import {readFileSync} from 'node:fs';
+
+import {
+  BASELINE_PATH,
+  createApiContext,
+  deleteKernels,
+  deleteSessions,
+  isE2ESession,
+  listKernelIds,
+  listSessions,
+} from './helpers/session';
+
+function readBaselineKernelIds(): Set<string> | null {
+  try {
+    const {kernelIds} = JSON.parse(readFileSync(BASELINE_PATH, 'utf-8')) as {kernelIds: string[]};
+    return new Set(kernelIds);
+  } catch {
+    // No baseline means we can't tell ours from theirs, so the kernel sweep
+    // is skipped entirely rather than risking a developer's running kernels.
+    return null;
+  }
+}
+
+export default async function globalTeardown(): Promise<void> {
+  const api = await createApiContext();
+  try {
+    const deletedSessions = await deleteSessions(api, isE2ESession);
+    if (deletedSessions.length) {
+      console.log(`[e2e teardown] deleted ${deletedSessions.length} leftover session(s).`);
+    }
+
+    const baseline = readBaselineKernelIds();
+    if (!baseline) {
+      console.warn(`[e2e teardown] no baseline at ${BASELINE_PATH}; skipping kernel sweep.`);
+      return;
+    }
+
+    // Anything still attached to a session is in use by something that isn't us.
+    const attached = new Set(
+      (await listSessions(api))
+        .map((session) => session.kernel?.id)
+        .filter((id): id is string => Boolean(id)),
+    );
+    const orphans = (await listKernelIds(api)).filter(
+      (id) => !baseline.has(id) && !attached.has(id),
+    );
+
+    if (orphans.length) {
+      const deleted = await deleteKernels(api, orphans);
+      console.log(`[e2e teardown] deleted ${deleted.length} orphaned kernel(s).`);
+    }
+  } catch (error) {
+    // Never fail a green run over cleanup.
+    console.warn('[e2e teardown] cleanup did not complete:', error);
+  } finally {
+    await api.dispose();
+  }
+}