mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-02 21:03:34 +03:00
Fix intermittently failing Playwright tests (#6886)
# Description of Changes Fixes intermittently failing tests (and replaces one that wasn't useful in its previous state) and also adds a CI check to warn if there are any Playwright tests which failed on their first go and succeeded on retries, to hopefully help find intermittently failing tests more quickly and avoid them being merged in the first place.
This commit is contained in:
James Brunton
@@ -167,6 +167,8 @@ jobs:
|
||||
wait_for_backend
|
||||
- name: Run enterprise OAuth Playwright tests
|
||||
id: oauth-tests
|
||||
env:
|
||||
PLAYWRIGHT_JSON_OUTPUT_FILE: ${{ github.workspace }}/frontend/playwright-report/results-oauth.json
|
||||
run: task e2e:enterprise -- --grep "OAuth"
|
||||
- name: Stop backend + tear down OAuth Keycloak
|
||||
if: always()
|
||||
@@ -240,6 +242,8 @@ jobs:
|
||||
wait_for_backend
|
||||
- name: Run enterprise SAML Playwright tests
|
||||
id: saml-tests
|
||||
env:
|
||||
PLAYWRIGHT_JSON_OUTPUT_FILE: ${{ github.workspace }}/frontend/playwright-report/results-saml.json
|
||||
run: task e2e:enterprise -- --grep "SAML"
|
||||
- name: Stop backend + tear down SAML Keycloak
|
||||
if: always()
|
||||
@@ -270,6 +274,8 @@ jobs:
|
||||
wait_for_backend
|
||||
- name: Run enterprise feature Playwright tests
|
||||
id: feature-tests
|
||||
env:
|
||||
PLAYWRIGHT_JSON_OUTPUT_FILE: ${{ github.workspace }}/frontend/playwright-report/results-feature.json
|
||||
run: task e2e:enterprise -- --grep "Enterprise license"
|
||||
- name: Print backend log on failure
|
||||
if: failure()
|
||||
@@ -282,6 +288,19 @@ jobs:
|
||||
run: |
|
||||
source /tmp/helpers.sh
|
||||
stop_backend
|
||||
- name: Flag flaky tests
|
||||
# Runs regardless of the test outcomes: a flaky test (passed on retry)
|
||||
# leaves its step green, so this is the only place it surfaces. Merges
|
||||
# all three phase reports (some may be absent if an earlier phase hard-
|
||||
# failed and skipped the rest). Emits ::warning:: annotations + a job
|
||||
# summary; never fails the job.
|
||||
if: always()
|
||||
working-directory: frontend
|
||||
run: >
|
||||
npx tsx editor/scripts/report-flaky-tests.mts
|
||||
"${{ github.workspace }}/frontend/playwright-report/results-oauth.json"
|
||||
"${{ github.workspace }}/frontend/playwright-report/results-saml.json"
|
||||
"${{ github.workspace }}/frontend/playwright-report/results-feature.json"
|
||||
- name: Upload Playwright report
|
||||
if: always()
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
|
||||
@@ -62,7 +62,17 @@ jobs:
|
||||
# .test-state/playwright/coverage-pw/ for the post-process step
|
||||
# to aggregate. Chromium-only - other engines silently skip.
|
||||
PW_COVERAGE: "1"
|
||||
PLAYWRIGHT_JSON_OUTPUT_FILE: ${{ github.workspace }}/frontend/playwright-report/results.json
|
||||
run: task e2e:live
|
||||
- name: Flag flaky tests
|
||||
# Runs regardless of the test outcome: a flaky test (passed on retry)
|
||||
# leaves the step green, so this is the only place it surfaces. Emits
|
||||
# ::warning:: annotations + a job summary; never fails the job.
|
||||
if: always()
|
||||
working-directory: frontend
|
||||
run: npx tsx editor/scripts/report-flaky-tests.mts "$PLAYWRIGHT_JSON_OUTPUT_FILE"
|
||||
env:
|
||||
PLAYWRIGHT_JSON_OUTPUT_FILE: ${{ github.workspace }}/frontend/playwright-report/results.json
|
||||
- name: Generate JaCoCo report from e2e:live .exec
|
||||
if: always()
|
||||
id: live-coverage
|
||||
|
||||
@@ -44,7 +44,18 @@ jobs:
|
||||
VITE_BUILD_FOR_PREVIEW: "1"
|
||||
run: task frontend:build
|
||||
- name: Run stubbed E2E tests (chromium)
|
||||
env:
|
||||
PLAYWRIGHT_JSON_OUTPUT_FILE: ${{ github.workspace }}/frontend/playwright-report/results.json
|
||||
run: task e2e:stubbed -- --workers=3
|
||||
- name: Flag flaky tests
|
||||
# Runs regardless of the test outcome: a flaky test (passed on retry)
|
||||
# leaves the step green, so this is the only place it surfaces. Emits
|
||||
# ::warning:: annotations + a job summary; never fails the job.
|
||||
if: always()
|
||||
working-directory: frontend
|
||||
run: npx tsx editor/scripts/report-flaky-tests.mts "$PLAYWRIGHT_JSON_OUTPUT_FILE"
|
||||
env:
|
||||
PLAYWRIGHT_JSON_OUTPUT_FILE: ${{ github.workspace }}/frontend/playwright-report/results.json
|
||||
- name: Upload Playwright report
|
||||
if: always()
|
||||
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
|
||||
|
||||
@@ -30,7 +30,18 @@ export default defineConfig({
|
||||
forbidOnly: !!process.env.CI,
|
||||
retries: process.env.CI ? 2 : 0,
|
||||
workers: process.env.CI ? 1 : "50%",
|
||||
reporter: [["html", { open: "never" }], ["list"]],
|
||||
// In CI, add a JSON report alongside the HTML/list output so the workflow
|
||||
// can flag flaky tests (passed only on retry) as warnings without failing
|
||||
// the job. Path is pinned via PLAYWRIGHT_JSON_OUTPUT_FILE in the workflow;
|
||||
// the outputFile here is just a sane default. Omitted locally to keep dev
|
||||
// runs' terminal output clean.
|
||||
reporter: process.env.CI
|
||||
? [
|
||||
["html", { open: "never" }],
|
||||
["list"],
|
||||
["json", { outputFile: "playwright-report/results.json" }],
|
||||
]
|
||||
: [["html", { open: "never" }], ["list"]],
|
||||
timeout: 60_000,
|
||||
expect: { timeout: 10_000 },
|
||||
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
// Reads a Playwright JSON report and surfaces "flaky" tests (tests that
|
||||
// failed at least once, then passed on retry) in GitHub Actions WITHOUT
|
||||
// failing the job:
|
||||
// - emits one ::warning:: workflow command per flaky test, so the run and
|
||||
// PR show a yellow warning triangle + count, and the annotation links to
|
||||
// the test's source line
|
||||
// - appends a summary table to the job summary ($GITHUB_STEP_SUMMARY)
|
||||
//
|
||||
// A green-but-flaky job is otherwise invisible (Playwright exits 0 once a
|
||||
// retry passes), which lets flakes accrete unnoticed. This makes them visible
|
||||
// without turning them into hard failures.
|
||||
//
|
||||
// Run: `npx tsx editor/scripts/report-flaky-tests.mts <results.json> [more.json...]`
|
||||
// (a single path is also read from PLAYWRIGHT_JSON_OUTPUT_FILE). Multiple
|
||||
// reports are merged + de-duplicated, so a job that runs Playwright in
|
||||
// several segments (e.g. the enterprise OAuth/SAML/license phases) can
|
||||
// pass one report per phase. A missing report or zero flaky tests is a
|
||||
// silent no-op, so it is safe to run with `if: always()` after any
|
||||
// Playwright step.
|
||||
|
||||
import { appendFileSync, existsSync, readFileSync } from "fs";
|
||||
import { isAbsolute, join, relative } from "path";
|
||||
import type { JSONReport, JSONReportSuite } from "@playwright/test/reporter";
|
||||
|
||||
interface FlakyTest {
|
||||
file: string;
|
||||
line: number;
|
||||
title: string;
|
||||
}
|
||||
|
||||
// Playwright records each test's outcome as expected|unexpected|flaky|skipped.
|
||||
// "flaky" means it needed a retry to pass, which is exactly what we surface.
|
||||
function collectFlaky(
|
||||
report: JSONReport,
|
||||
workspace: string,
|
||||
rootDir: string,
|
||||
): FlakyTest[] {
|
||||
const flaky: FlakyTest[] = [];
|
||||
const walk = (suite: JSONReportSuite, trail: string[], depth: number) => {
|
||||
// The outermost suite per file has title === the file path; skip it so the
|
||||
// human-readable title is just "describe > test" (the path is shown
|
||||
// separately as the location). Nested suites are the describe() blocks.
|
||||
const titles = depth > 0 && suite.title ? [...trail, suite.title] : trail;
|
||||
for (const spec of suite.specs ?? []) {
|
||||
if ((spec.tests ?? []).some((t) => t.status === "flaky")) {
|
||||
const abs = spec.file
|
||||
? isAbsolute(spec.file)
|
||||
? spec.file
|
||||
: join(rootDir, spec.file)
|
||||
: "";
|
||||
const rel = abs ? relative(workspace, abs) : "";
|
||||
flaky.push({
|
||||
// Drop the path from the annotation if it escapes the workspace, so
|
||||
// we never emit a broken file= link (the warning still shows).
|
||||
file: rel && !rel.startsWith("..") ? rel : "",
|
||||
line: spec.line || 0,
|
||||
title: [...titles, spec.title].filter(Boolean).join(" > "),
|
||||
});
|
||||
}
|
||||
}
|
||||
for (const child of suite.suites ?? []) walk(child, titles, depth + 1);
|
||||
};
|
||||
for (const suite of report.suites ?? []) walk(suite, [], 0);
|
||||
return flaky;
|
||||
}
|
||||
|
||||
// Deliberately no process.exit() calls: every path falls through to a natural
|
||||
// exit(0). This step must never fail the job, and it keeps CI green even when
|
||||
// the report is missing or clean.
|
||||
function main(): void {
|
||||
// Accept one or more report paths: a job may run Playwright in several
|
||||
// segments, each writing its own report (the enterprise job does this for
|
||||
// OAuth / SAML / license phases). Fall back to the env var when no paths are
|
||||
// passed. Missing files are skipped, not fatal.
|
||||
const reportPaths = process.argv.slice(2);
|
||||
const envPath = process.env.PLAYWRIGHT_JSON_OUTPUT_FILE;
|
||||
if (reportPaths.length === 0 && envPath) {
|
||||
reportPaths.push(envPath);
|
||||
}
|
||||
|
||||
const workspace = process.env.GITHUB_WORKSPACE || process.cwd();
|
||||
const seen = new Set<string>();
|
||||
const flaky: FlakyTest[] = [];
|
||||
for (const reportPath of reportPaths) {
|
||||
if (!reportPath || !existsSync(reportPath)) {
|
||||
// No report (e.g. the build failed before this segment ran).
|
||||
continue;
|
||||
}
|
||||
const report = JSON.parse(readFileSync(reportPath, "utf8")) as JSONReport;
|
||||
const rootDir = report.config?.rootDir || process.cwd();
|
||||
for (const test of collectFlaky(report, workspace, rootDir)) {
|
||||
const key = `${test.file}:${test.line}:${test.title}`;
|
||||
if (!seen.has(key)) {
|
||||
seen.add(key);
|
||||
flaky.push(test);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (flaky.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const f of flaky) {
|
||||
const loc = f.file ? `file=${f.file},line=${f.line},` : "";
|
||||
process.stdout.write(
|
||||
`::warning ${loc}title=Flaky test::${f.title} passed only on retry\n`,
|
||||
);
|
||||
}
|
||||
|
||||
const summaryPath = process.env.GITHUB_STEP_SUMMARY;
|
||||
if (summaryPath) {
|
||||
const plural = flaky.length === 1 ? "" : "s";
|
||||
const lines = [
|
||||
`### :warning: ${flaky.length} flaky test${plural} (passed on retry)`,
|
||||
"",
|
||||
"These passed, but not on the first attempt. Worth fixing before they turn into hard failures.",
|
||||
"",
|
||||
"| Test | Location |",
|
||||
"| --- | --- |",
|
||||
...flaky.map((f) => `| ${f.title} | \`${f.file || "?"}:${f.line}\` |`),
|
||||
"",
|
||||
];
|
||||
appendFileSync(summaryPath, lines.join("\n") + "\n");
|
||||
}
|
||||
}
|
||||
|
||||
main();
|
||||
@@ -9,6 +9,11 @@ const SAMPLE_PDF = path.join(FIXTURES_DIR, "sample.pdf");
|
||||
* Files uploaded on one tool page should remain in the workbench when the
|
||||
* user navigates to a different tool. This is FileContext behaviour and
|
||||
* easy to break with a stale-effect or unmount-clear bug.
|
||||
*
|
||||
* Covered for both navigation mechanisms, which take different code paths:
|
||||
* - a full page reload (page.goto) -> FileContext re-hydrates from IndexedDB
|
||||
* - an in-app tool-link click -> client-side nav, FileContext stays
|
||||
* in memory
|
||||
*/
|
||||
test.describe("File state persists across tool navigation", () => {
|
||||
test("file uploaded on /merge survives navigation to /split", async ({
|
||||
@@ -32,4 +37,37 @@ test.describe("File state persists across tool navigation", () => {
|
||||
timeout: 5_000,
|
||||
});
|
||||
});
|
||||
|
||||
test("file uploaded on /merge survives an in-app tool-link navigation", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.goto("/merge");
|
||||
await page.waitForLoadState("domcontentloaded");
|
||||
await uploadFiles(page, SAMPLE_PDF);
|
||||
|
||||
// Navigate via the in-app tool link (client-side React Router nav) rather
|
||||
// than a full reload, so this exercises the in-memory FileContext path the
|
||||
// page.goto test above doesn't. Fall back to a direct visit if the nav
|
||||
// rail isn't showing the link yet.
|
||||
const splitNav = page.getByRole("link", { name: /^Split$/i }).first();
|
||||
if (await splitNav.isVisible({ timeout: 1_000 }).catch(() => false)) {
|
||||
await splitNav.click();
|
||||
} else {
|
||||
await page.goto("/split");
|
||||
}
|
||||
|
||||
// A client-side nav has no document load event, so waitForLoadState is a
|
||||
// no-op here. Wait for the route to actually commit before opening the
|
||||
// file manager; otherwise the my-files click fires mid-transition and
|
||||
// opens it against a not-yet-settled workbench, which renders a permanent
|
||||
// empty state (the flaky "0 items" that then passes on retry).
|
||||
await expect(page).toHaveURL(/\/split(?:$|[/?#])/);
|
||||
|
||||
// The upload must still be listed after the tool switch. A "no files"
|
||||
// empty state here would mean the client-side nav silently dropped it.
|
||||
await page.getByTestId("my-files-button").click();
|
||||
await expect(page.getByText(/sample\.pdf/i).first()).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -825,11 +825,10 @@ test.describe("Files page", () => {
|
||||
// and direct user shares.)
|
||||
await page.locator("#filesPage-tab-sharedByMe").click();
|
||||
const sharedByMeCards = page.locator(".files-page-card:not(.is-folder)");
|
||||
await expect(sharedByMeCards).toHaveCount(2, { timeout: 3_000 });
|
||||
await expect(sharedByMeCards).toContainText([
|
||||
"link-shared.pdf",
|
||||
"user-shared.pdf",
|
||||
]);
|
||||
await expect(sharedByMeCards).toHaveCount(2, { timeout: 5_000 });
|
||||
for (const name of ["link-shared.pdf", "user-shared.pdf"]) {
|
||||
await expect(sharedByMeCards.filter({ hasText: name })).toHaveCount(1);
|
||||
}
|
||||
|
||||
// "Shared with me" -> only from-someone-else.pdf
|
||||
await page.locator("#filesPage-tab-shared").click();
|
||||
|
||||
@@ -1,43 +0,0 @@
|
||||
import { test, expect } from "@app/tests/helpers/stub-test-base";
|
||||
import { uploadFiles } from "@app/tests/helpers/ui-helpers";
|
||||
import path from "path";
|
||||
|
||||
const SAMPLE_PDF = path.join(__dirname, "../test-fixtures/sample.pdf");
|
||||
|
||||
/**
|
||||
* The NavigationGuard context warns the user when they have unsaved work
|
||||
* (uploaded files or modified config) and try to navigate away. The guard
|
||||
* surface is a Mantine modal asking to confirm or cancel the navigation.
|
||||
*
|
||||
* Today the guard logic exists but is silently bypassed by tests that go
|
||||
* through the workbench. This spec asserts the modal appears and that
|
||||
* cancelling keeps the user on the current tool.
|
||||
*/
|
||||
test.describe("Unsaved changes navigation guard", () => {
|
||||
test("uploading then navigating away surfaces the guard prompt", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.goto("/merge");
|
||||
await page.waitForLoadState("domcontentloaded");
|
||||
await uploadFiles(page, SAMPLE_PDF);
|
||||
|
||||
// Triggering a tool-level navigation while files are loaded should
|
||||
// either prompt or clear-and-navigate cleanly. A regression that
|
||||
// discards files silently is the failure we want to catch.
|
||||
const splitNav = page.getByRole("link", { name: /^Split$/i }).first();
|
||||
if (await splitNav.isVisible({ timeout: 1_000 }).catch(() => false)) {
|
||||
await splitNav.click();
|
||||
} else {
|
||||
await page.goto("/split");
|
||||
}
|
||||
|
||||
// After arriving at /split the My Files page should still list the
|
||||
// previously uploaded sample (NavigationGuard either kept us on
|
||||
// /merge or moved us with state intact). A "no files" empty state
|
||||
// here would indicate the guard silently dropped the workbench.
|
||||
await page.getByTestId("my-files-button").click();
|
||||
await expect(page.getByText(/sample\.pdf/i).first()).toBeVisible({
|
||||
timeout: 5_000,
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user