From e0500d387740dadceddc0e517cfe1d12a2e3fab3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 07:58:59 +0000 Subject: [PATCH] test(e2e): the console smoke test asserts a boot state the app actually settles in (#4086) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Build & E2E` went red on `main` at 9154d9e90 and green again at 8497579db with the suspected commit (#4081) still fully in place. Nothing in either diff can explain either transition, because the cause was never in the diff: the smoke test at `e2e/smoke.spec.ts:78` asserted `nav` OR the text /Initializing|Loading|Connecting/ — and against the harness's own artifact (a production bundle served by `vite preview` with no backend behind it) the app settles into neither. Measured on that exact artifact at 30 ms polling granularity, three consecutive boots agreeing to within 10 ms: + ~70 ms "Initializing application... Connecting to data source" splash + ~105 ms redirected to the signed-out sign-in screen, and stays there So the only state the old assertion could ever match was on screen for roughly 35 ms. The test passed by catching that window, and failed for the full 30 s timeout — identically on all three retries, since each retry re-runs the same race — whenever Playwright's first poll landed after it. That is a coin flip on runner speed, which is precisely the observed behaviour: identical code red at 06:15Z, green at 07:40Z. The app is not at fault. With `/api/v1/auth/get-session` unable to resolve a session, the shell is never entitled to render a `nav`, and `/login` is the correct destination; the test simply never listed it. The assertion now names all three recognised boot destinations, each of them terminal-stable, so it no longer depends on winning a race — while staying a closed set, so a blank page, a crashed render or an error boundary still fails it. Also fixes the missing-artifact rider named in the issue. CI selects Playwright's `github` reporter, which writes annotations and no `playwright-report/` directory at all, so the upload step warned `No files were found with the provided path` and every red E2E job discarded its own evidence. The screenshot, trace and `error-context.md` land in `test-results/`; both paths are uploaded now. Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 18 +++++++++++++++++- e2e/smoke.spec.ts | 40 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 05d500a311..e3b0809710 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -535,12 +535,28 @@ jobs: if: steps.relevant.outputs.should_run == 'true' run: pnpm test:e2e --project=chromium + # `playwright.config.ts` selects the `github` reporter when CI is set, and + # that reporter writes annotations only — it produces NO + # `playwright-report/` directory, so this step uploaded nothing and said + # so in a warning rather than a failure: `No files were found with the + # provided path: playwright-report/` (objectui#4086). Every failure's + # actual evidence — screenshot, trace and `error-context.md` — is written + # by the `use.screenshot` / `use.trace` settings into `test-results/` + # instead, which is why a red E2E job left nothing behind to diagnose and + # #4086 had to be reproduced from scratch locally. + # + # Both paths are listed: `test-results/` is the one that exists today, and + # `playwright-report/` keeps working if the HTML reporter is ever enabled + # on CI. upload-artifact only warns when NO path matches, so the absent + # one costs nothing. - name: Upload Playwright report uses: actions/upload-artifact@v7 if: ${{ steps.relevant.outputs.should_run == 'true' && !cancelled() && failure() }} with: name: playwright-report - path: playwright-report/ + path: | + playwright-report/ + test-results/ retention-days: 14 docs: diff --git a/e2e/smoke.spec.ts b/e2e/smoke.spec.ts index e6fc8d069f..53d6f976c9 100644 --- a/e2e/smoke.spec.ts +++ b/e2e/smoke.spec.ts @@ -75,16 +75,48 @@ test.describe('Console App – Smoke', () => { await expect(page).toHaveTitle(/.+/); }); - test('should show the app shell or loading screen', async ({ page }) => { + /** + * The app must settle into one of its RECOGNISED boot destinations. Which one + * it reaches depends on the session, and in this suite — a production bundle + * served by `vite preview` with no backend behind it — that is always the + * signed-out sign-in screen: `/api/v1/auth/get-session` cannot resolve to a + * session, so the shell is never entitled to render a `nav` and the router + * lands on `/login`. + * + * Until objectui#4086 this test listed only the first two destinations, and + * the boot splash it named is on screen for roughly 35 ms — measured against + * this same production bundle at 30 ms polling granularity: blank until + * ~70 ms, the "Initializing application…" splash from ~70 ms, replaced by the + * sign-in screen from ~105 ms and never returning. So the assertion could + * only ever pass by catching that window. On a runner where Playwright's + * first poll landed after it, the expectation was ALREADY unsatisfiable and + * burned the full 30 s timeout — identically on all three retries, because + * each retry re-runs the same race. That is the whole of the `Build & E2E` + * red on `main` at 9154d9e90, which no code change could explain and which + * cleared on its own at 8497579db with the suspected commit still in place. + * + * The set below stays CLOSED on purpose: a blank page, a crashed render or an + * error boundary matches none of these three and still fails the test. What + * changes is that every state the app is legitimately allowed to be in is now + * terminal-stable, so passing no longer depends on winning a race. + */ + test('should settle into a recognised boot state (app shell, loading screen or sign-in)', async ({ + page, + }) => { await page.goto(`${CONSOLE_BASE}/`); - // Either the app shell (nav / sidebar) or the loading screen should appear - // within a reasonable time. Both are acceptable initial states. const appShell = page.locator('nav').first(); const loadingScreen = page.getByText(/Initializing|Loading|Connecting/i).first(); + // The signed-out sign-in screen, matched structurally rather than by its + // copy so a locale change cannot silently stop matching it: the auth-config + // spinner it opens with, then the identifier field of whichever sign-in + // mode the server reports (`LoginForm.tsx`). + const signInScreen = page + .locator('[data-testid="login-config-loading"], #login-email, #login-phone') + .first(); await expect( - appShell.or(loadingScreen), + appShell.or(loadingScreen).or(signInScreen), ).toBeVisible({ timeout: 30_000 }); }); });