From 39eafad47d77a98ed2597d70598e4631458859ff Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:15:39 -0500 Subject: [PATCH] test(e2e): shorten and split Smoke Lab coverage (#12506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip uses browser tests to protect critical operator flows. > - The trusted pull request workflow runs the E2E catalog on three existing runners. > - Smoke Lab was one 168-second spec, so the shard scheduler could not divide it. > - The spec also repeated service-start calls, page loads, and full-page screenshots. > - This pull request removes that repeated work and divides the scenario catalog into two independent specs. > - The benefit is a shorter Smoke Lab run and a balanced E2E lane without more AWS capacity. ## Linked Issues or Issue Description Refs: #10629 **What existing behavior does this improve?** This improves the trusted pull request E2E lane and its Smoke Lab Playwright coverage. **Current behavior** Smoke Lab is one indivisible 168-second CI spec. It starts services for every scenario, loads the same evidence page twice, and captures a full-page success screenshot for all 56 lifecycle steps. **Proposed behavior** Start Smoke Lab services once per spec. Keep the per-scenario fixture reset. Capture one representative success screenshot per scenario and keep every failure screenshot. Run P1–P4 and P5–P7 as separate specs so the existing duration-aware scheduler can put them on different runners. **Reason and benefit** The optimized lifecycle reduced local Smoke Lab wall time from 57.68 seconds to 37.04 seconds. This is a 35.8% reduction. The two halves also let the existing three runners target about 125, 124, and 124 seconds of recorded spec work instead of about 168, 125, and 124 seconds. **Breaking changes** None. The same seven scenarios and eight lifecycle steps still run. The result API still records every step. Successful non-connect steps no longer attach redundant screenshots. ## What Changed - Reused one Smoke Lab service start within each spec while retaining isolated fixture installation for every scenario. - Removed the duplicate catalog evidence navigation. - Reduced success screenshots from 56 to 7 while retaining screenshots for every failed step. - Split the shared lifecycle runner into P1–P4 and P5–P7 specs. - Mark each successful split result as partial and keep dashboard health amber until one run covers the full catalog. - Updated the duration manifest and contributor docs for the split. ## Verification - `pnpm -r typecheck` passed on Node.js 24.20.0. - `pnpm build` passed on Node.js 24.20.0. - `node --test scripts/__tests__/e2e-shard.test.mjs` passed 9 tests. - `pnpm exec vitest run ui/src/pages/tools/smoke-lab-matrix.test.ts` passed 8 tests. - Both split specs passed together on Node.js 24.20.0 after the review fixes: 2 passed in 35.7 seconds; shell wall time was 36.86 seconds. - The pre-change Smoke Lab baseline passed with a 57.68-second shell wall time. The optimized unsplit A/B run passed with a 37.04-second shell wall time. - The full local E2E catalog passed 44 tests and skipped 2 tests. One existing `pipelines-tutorial-flow.spec.ts` assertion failed again when run alone. - The broad local unit run reproduced failures in untouched workspace-runtime suites. Typecheck, build, shard tests, and all changed browser coverage pass. CI remains the authoritative full-suite result. ## Risks - The split duration weights use the measured local reduction and the previous 168-second CI weight. They should be refreshed after two real pull request runs. - Service state is shared within each half. Fixture installation still runs before every scenario to reset connection, policy, and catalog state. - Each half records passed execution with partial coverage. Dashboard health recognizes the partial flag and stays amber because no single runner covers the full catalog. A failed half still records failed/red. - Fewer success screenshots reduce redundant artifacts. Every scenario keeps its connect screenshot, and every failure still captures evidence. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5. The exact deployment ID and context window are not exposed in this session. The model used agentic reasoning, code editing, shell execution, browser testing, and GitHub tools. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/connections/SMOKE-LAB-BROWSER-RUNNER.md | 7 +-- doc/connections/SMOKE-LAB-TUTORIAL.md | 7 +-- scripts/e2e-shard-durations.json | 5 +- tests/e2e/smoke-lab-p1-p4.spec.ts | 4 ++ tests/e2e/smoke-lab-p5-p7.spec.ts | 4 ++ ...{smoke-lab.spec.ts => smoke-lab.shared.ts} | 53 +++++++++++-------- ui/src/pages/tools/smoke-lab-matrix.test.ts | 5 ++ ui/src/pages/tools/smoke-lab-matrix.ts | 1 + 8 files changed, 57 insertions(+), 29 deletions(-) create mode 100644 tests/e2e/smoke-lab-p1-p4.spec.ts create mode 100644 tests/e2e/smoke-lab-p5-p7.spec.ts rename tests/e2e/{smoke-lab.spec.ts => smoke-lab.shared.ts} (90%) diff --git a/doc/connections/SMOKE-LAB-BROWSER-RUNNER.md b/doc/connections/SMOKE-LAB-BROWSER-RUNNER.md index f5f491b2e6..65bfe876cc 100644 --- a/doc/connections/SMOKE-LAB-BROWSER-RUNNER.md +++ b/doc/connections/SMOKE-LAB-BROWSER-RUNNER.md @@ -4,9 +4,10 @@ **Scenario source of truth:** [`tests/e2e/smoke-lab.catalog.ts`](../../tests/e2e/smoke-lab.catalog.ts) — **do not fork the step list.** This is the manual/agent counterpart to the deterministic Playwright CI mirror -(`tests/e2e/smoke-lab.spec.ts`, S4). Where the CI mirror runs headless in a -throwaway instance for a red/green gate, this runbook has a **QA agent drive a -real browser** through the same P1–P7 lifecycle against a live instance, +(`tests/e2e/smoke-lab.shared.ts`, split between the P1–P4 and P5–P7 specs). +Where the CI mirror runs headless in a throwaway instance for a red/green gate, +this runbook has a **QA agent drive a real browser** through the same P1–P7 +lifecycle against a live instance, **typing demo credentials into the fake OAuth provider's real consent page**, and records every step + a viewable screenshot to the Smoke Lab results API so the run shows up in the **Smoke Lab tab** and the **dashboard "Integration smoke" card**. diff --git a/doc/connections/SMOKE-LAB-TUTORIAL.md b/doc/connections/SMOKE-LAB-TUTORIAL.md index c6093a9f43..0299fbda05 100644 --- a/doc/connections/SMOKE-LAB-TUTORIAL.md +++ b/doc/connections/SMOKE-LAB-TUTORIAL.md @@ -12,9 +12,10 @@ button or label is quoted, that's the exact text in the product. > Companion docs: the automated counterparts live in > [`SMOKE-LAB-BROWSER-RUNNER.md`](./SMOKE-LAB-BROWSER-RUNNER.md) (the agent-driven -> browser runner) and `tests/e2e/smoke-lab.spec.ts` (the headless CI mirror). The -> daily recurring routine that runs the browser smoke for you is described in -> [§8](#8-the-daily-routine-hands-off). +> browser runner), `tests/e2e/smoke-lab.shared.ts` (the shared headless runner), +> and the `tests/e2e/smoke-lab-p1-p4.spec.ts` and +> `tests/e2e/smoke-lab-p5-p7.spec.ts` CI halves. The daily recurring routine that +> runs the browser smoke for you is described in [§8](#8-the-daily-routine-hands-off). --- diff --git a/scripts/e2e-shard-durations.json b/scripts/e2e-shard-durations.json index b23c25efdb..af5549c94d 100644 --- a/scripts/e2e-shard-durations.json +++ b/scripts/e2e-shard-durations.json @@ -1,5 +1,5 @@ { - "$comment": "Per-spec wall-clock durations (ms) for the Playwright e2e lane, used by scripts/e2e-shard.mjs to balance specs across the PR shard matrix. Averaged from two real PR runs of .github/workflows/pr.yml (actions runs 29775184389 and 29769465682, 2026-07-20). Specs missing here get the median weight, so the manifest only needs occasional refreshes: pull the 'Run e2e tests' logs from a recent PR run and sum the Playwright list-reporter durations per spec file.", + "$comment": "Per-spec wall-clock durations (ms) for the Playwright e2e lane, used by scripts/e2e-shard.mjs to balance specs across the PR shard matrix. Averaged from two real PR runs of .github/workflows/pr.yml (actions runs 29775184389 and 29769465682, 2026-07-20). The split Smoke Lab weights scale its prior 168s CI duration by the measured local before/after ratio and scenario count; refresh them from the next two real PR runs. Specs missing here get the median weight, so the manifest only needs occasional refreshes: pull the 'Run e2e tests' logs from a recent PR run and sum the Playwright list-reporter durations per spec file.", "unit": "ms", "durations": { "tests/e2e/app-not-connected.spec.ts": 26750, @@ -15,6 +15,7 @@ "tests/e2e/planning-mode-visual-verification.spec.ts": 23250, "tests/e2e/sidebar-takeover.spec.ts": 16650, "tests/e2e/signoff-policy.spec.ts": 9696, - "tests/e2e/smoke-lab.spec.ts": 168000 + "tests/e2e/smoke-lab-p1-p4.spec.ts": 71000, + "tests/e2e/smoke-lab-p5-p7.spec.ts": 53000 } } diff --git a/tests/e2e/smoke-lab-p1-p4.spec.ts b/tests/e2e/smoke-lab-p1-p4.spec.ts new file mode 100644 index 0000000000..51afd51fd6 --- /dev/null +++ b/tests/e2e/smoke-lab-p1-p4.spec.ts @@ -0,0 +1,4 @@ +import { ciSmokeLabScenarios } from "./smoke-lab.catalog"; +import { defineSmokeLabSuite } from "./smoke-lab.shared"; + +defineSmokeLabSuite("P1-P4", ciSmokeLabScenarios.slice(0, 4)); diff --git a/tests/e2e/smoke-lab-p5-p7.spec.ts b/tests/e2e/smoke-lab-p5-p7.spec.ts new file mode 100644 index 0000000000..4f3b704ad9 --- /dev/null +++ b/tests/e2e/smoke-lab-p5-p7.spec.ts @@ -0,0 +1,4 @@ +import { ciSmokeLabScenarios } from "./smoke-lab.catalog"; +import { defineSmokeLabSuite } from "./smoke-lab.shared"; + +defineSmokeLabSuite("P5-P7", ciSmokeLabScenarios.slice(4)); diff --git a/tests/e2e/smoke-lab.spec.ts b/tests/e2e/smoke-lab.shared.ts similarity index 90% rename from tests/e2e/smoke-lab.spec.ts rename to tests/e2e/smoke-lab.shared.ts index 3b6c0904f0..d69568727f 100644 --- a/tests/e2e/smoke-lab.spec.ts +++ b/tests/e2e/smoke-lab.shared.ts @@ -85,14 +85,14 @@ async function enableSmokeLab(request: APIRequestContext) { await json(await request.patch("/api/instance/settings/experimental", { data: { enableSmokeLab: true, enableApps: true } })); } -async function createSmokeRun(request: APIRequestContext, companyId: string) { +async function createSmokeRun(request: APIRequestContext, companyId: string, scenarioCount: number) { const result = await json<{ run: SmokeRun }>( await request.post(`/api/companies/${companyId}/smoke-lab/runs`, { data: { trigger: "ci", summary: { catalog: "tests/e2e/smoke-lab.catalog.ts", - scenarioCount: ciSmokeLabScenarios.length, + scenarioCount, }, }, }), @@ -180,11 +180,12 @@ async function runRecordedStep( scenario: SmokeLabScenario, step: string, action: () => Promise, + captureSuccessScreenshot = false, ) { const start = Date.now(); try { const screenshotHint = await action(); - const screenshotPath = await screenshot(page, scenario, step); + const screenshotPath = captureSuccessScreenshot ? await screenshot(page, scenario, step) : null; await recordStep(request, seed.companyId, runId, { path: scenario.path, scenarioStep: step, @@ -207,8 +208,11 @@ async function runRecordedStep( } } -async function startAndInstallFixtures(request: APIRequestContext, companyId: string): Promise { +async function startSmokeLabServices(request: APIRequestContext, companyId: string) { await json(await request.post(`/api/companies/${companyId}/smoke-lab/services/start`)); +} + +async function installFixtures(request: APIRequestContext, companyId: string): Promise { return await json(await request.post(`/api/companies/${companyId}/smoke-lab/install-fixtures`)); } @@ -300,36 +304,37 @@ async function gatewayFetch(request: APIRequestContext, path: string, token: str return await request.get(path, { headers }); } -test.describe.serial("Smoke Lab scenario catalog mirror", () => { - // The lifecycle test walks all CI-safe scenarios, and each scenario records - // eight steps with a real navigation and a full-page screenshot. On a loaded - // CI runner the whole walk needs a little more than four minutes, so the - // earlier 240s budget could expire on the final step. Six minutes gives - // headroom for runner variance without slowing a healthy run, because this is - // a ceiling, not the normal run time. +export const defineSmokeLabSuite = (label: string, scenarios: SmokeLabScenario[]) => test.describe.serial(`Smoke Lab scenario catalog mirror (${label})`, () => { + // The lifecycle test walks all CI-safe scenarios and records eight steps per + // scenario. Each scenario keeps one representative success screenshot, while + // every failed step still captures its own evidence. On a loaded CI runner + // the whole walk can take several minutes, so this ceiling keeps enough + // headroom for runner variance without slowing a healthy run. test.setTimeout(360_000); - test("records the P1-P7 CI-safe Smoke Lab lifecycle into the results API @smoke-lab", async ({ page, request }) => { + test(`records the ${label} CI-safe Smoke Lab lifecycle into the results API @smoke-lab`, async ({ page, request }) => { const seed = await newCompany(request, "catalog"); const scout = await createScout(request, seed.companyId); await enableSmokeLab(request); - const smokeRun = await createSmokeRun(request, seed.companyId); + const smokeRun = await createSmokeRun(request, seed.companyId, scenarios.length); const failed: string[] = []; + const isPartialRun = scenarios.length < ciSmokeLabScenarios.length; try { - for (const scenario of ciSmokeLabScenarios) { - const fixtures = await startAndInstallFixtures(request, seed.companyId); + await startSmokeLabServices(request, seed.companyId); + + for (const scenario of scenarios) { + const fixtures = await installFixtures(request, seed.companyId); const connection = connectionForScenario(fixtures, scenario); await runRecordedStep(page, request, seed, smokeRun.id, scenario, "connect", async () => { await navigateForEvidence(page, seed, connection.id, scenario); return scenario.lifecycle.connect; - }); + }, true); await runRecordedStep(page, request, seed, smokeRun.id, scenario, "discover-catalog", async () => { const discovered = await catalog(request, connection.id); expect(discovered.catalog.map((entry) => entry.toolName)).toContain(scenario.lifecycle.allowedRead.name); - await navigateForEvidence(page, seed, connection.id, scenario); return `${scenario.lifecycle.discoverCatalog}: ${discovered.catalog.length} entries`; }); @@ -440,20 +445,26 @@ test.describe.serial("Smoke Lab scenario catalog mirror", () => { } finally { await updateSmokeRun(request, seed.companyId, smokeRun.id, failed.length > 0 ? "failed" : "passed", { catalog: "tests/e2e/smoke-lab.catalog.ts", - scenarioCount: ciSmokeLabScenarios.length, + scenarioCount: scenarios.length, + catalogScenarioCount: ciSmokeLabScenarios.length, + partial: isPartialRun, failed, }).catch(() => undefined); } - const completed = await json<{ run: SmokeRun; steps: Array<{ path: string; status: string; screenshotArtifactRef: Json | null }> }>( + const completed = await json<{ + run: SmokeRun; + steps: Array<{ path: string; scenarioStep: string; status: string; screenshotArtifactRef: Json | null }>; + }>( await request.get(`/api/companies/${seed.companyId}/smoke-lab/runs/${smokeRun.id}`), ); expect(completed.run.status).toBe("passed"); - for (const scenario of ciSmokeLabScenarios) { + for (const scenario of scenarios) { const steps = completed.steps.filter((step) => step.path === scenario.path); expect(steps.length, `${scenario.path} should record lifecycle steps`).toBeGreaterThanOrEqual(8); expect(steps.every((step) => step.status === "pass")).toBe(true); - expect(steps.every((step) => step.screenshotArtifactRef?.kind === "playwright_screenshot")).toBe(true); + const screenshotSteps = steps.filter((step) => step.screenshotArtifactRef?.kind === "playwright_screenshot"); + expect(screenshotSteps.map((step) => step.scenarioStep)).toEqual(["connect"]); } }); }); diff --git a/ui/src/pages/tools/smoke-lab-matrix.test.ts b/ui/src/pages/tools/smoke-lab-matrix.test.ts index 1b3a3560aa..49a55df11f 100644 --- a/ui/src/pages/tools/smoke-lab-matrix.test.ts +++ b/ui/src/pages/tools/smoke-lab-matrix.test.ts @@ -80,6 +80,11 @@ describe("runHealth + failingPaths", () => { expect(runHealth(run({ status: "passed" }), steps)).toBe("green"); }); + it("is amber for a passed partial-catalog run", () => { + const steps = [step({ path: "P1", scenarioStep: "connect", status: "pass" })]; + expect(runHealth(run({ status: "passed", summary: { partial: true } }), steps)).toBe("amber"); + }); + it("is amber for a running run or an empty run", () => { expect(runHealth(run({ status: "running" }), [])).toBe("amber"); expect(runHealth(run({ status: "passed" }), [])).toBe("amber"); diff --git a/ui/src/pages/tools/smoke-lab-matrix.ts b/ui/src/pages/tools/smoke-lab-matrix.ts index c609e4c37d..392dc40fe5 100644 --- a/ui/src/pages/tools/smoke-lab-matrix.ts +++ b/ui/src/pages/tools/smoke-lab-matrix.ts @@ -92,6 +92,7 @@ export function runHealth(run: SmokeRun | undefined, steps: SmokeRunStep[]): Smo if (!run) return "unknown"; if (run.status === "failed") return "red"; if (steps.some((s) => s.status === "fail")) return "red"; + if (run.summary.partial === true) return "amber"; if (run.status === "cancelled") return "amber"; if (run.status === "running") return "amber"; if (steps.length === 0) return "amber";