mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 06:40:50 +00:00
test(e2e): run the delegated-writes journey and fail on silent skips
The delegated-writes spec checked for the invitation form's level control right after page load, before the form had painted, found nothing and skipped every test on every run, CI included. - Give the journey its own account pair, seeded and reset in global setup, so it no longer invites the same pair as the account-sharing journey (a second live grant between two accounts is refused with 409). - Wait for the form through its data-slot and fail when the WRITE level control is missing instead of skipping. - Revoke the journey's grant in afterAll. - Keep it out of the mobile project in the config rather than through a runtime skip. Add two checks so a skip cannot hide a spec again: - e2e-conditional-skip-guard.test.ts requires every conditional test.skip/test.fixme in e2e/ to carry a reason and refuses a bare test.skip(). - scripts/check-e2e-skipped-specs.mjs reads the Playwright JSON reports of both shards in the e2e gate job and fails when a spec file skipped all of its tests across every project, apart from an allowlist with reasons.
This commit is contained in:
1 parent
1f725326b0
commit
ed05772a8e
10 files changed
+719
-68
No files matched your search
@@ -0,0 +1,192 @@
|
||||
/**
|
||||
* `scripts/check-e2e-skipped-specs.mjs`: the CI half of "a spec file that
|
||||
* skips everything must say so". The logic is exercised on hand-built reports
|
||||
* in Playwright's JSON shape, the CLI is run once end to end, and the workflow
|
||||
* wiring is pinned so the check cannot be dropped from the gate job silently.
|
||||
*/
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterAll, describe, expect, it } from "vitest";
|
||||
import { parse } from "yaml";
|
||||
|
||||
import {
|
||||
ALLOWED_ALL_SKIPPED,
|
||||
findProblems,
|
||||
tallySpecFiles,
|
||||
} from "../check-e2e-skipped-specs.mjs";
|
||||
|
||||
const ROOT = join(__dirname, "../..");
|
||||
const SCRIPT = join(ROOT, "scripts/check-e2e-skipped-specs.mjs");
|
||||
|
||||
type Status = "expected" | "unexpected" | "flaky" | "skipped";
|
||||
|
||||
/** One report in Playwright's JSON shape: file suite → describe → spec → tests. */
|
||||
function report(
|
||||
entries: Array<{ file: string; tests: Array<[string, Status]> }>,
|
||||
) {
|
||||
return {
|
||||
suites: entries.map(({ file, tests }) => ({
|
||||
title: file,
|
||||
file,
|
||||
specs: [],
|
||||
suites: [
|
||||
{
|
||||
title: "describe",
|
||||
file,
|
||||
specs: tests.map(([projectName, status], i) => ({
|
||||
title: `t${i}`,
|
||||
file,
|
||||
tests: [{ projectName, status }],
|
||||
})),
|
||||
},
|
||||
],
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
||||
describe("check-e2e-skipped-specs", () => {
|
||||
it("flags a file whose every test skipped", () => {
|
||||
const problems = findProblems(
|
||||
[
|
||||
report([
|
||||
{ file: "a.spec.ts", tests: [["chromium-desktop", "expected"]] },
|
||||
{
|
||||
file: "b.spec.ts",
|
||||
tests: [
|
||||
["chromium-desktop", "skipped"],
|
||||
["chromium-mobile", "skipped"],
|
||||
],
|
||||
},
|
||||
]),
|
||||
],
|
||||
{},
|
||||
);
|
||||
expect(problems).toEqual([
|
||||
"b.spec.ts: all 2 test(s) skipped in every project and shard",
|
||||
]);
|
||||
});
|
||||
|
||||
it("aggregates across projects and shards before judging a file", () => {
|
||||
// The desktop half skipped in shard 1, the mobile half ran in shard 2.
|
||||
const reports = [
|
||||
report([{ file: "m.spec.ts", tests: [["chromium-desktop", "skipped"]] }]),
|
||||
report([{ file: "m.spec.ts", tests: [["chromium-mobile", "expected"]] }]),
|
||||
];
|
||||
expect(tallySpecFiles(reports).get("m.spec.ts")).toEqual({
|
||||
total: 2,
|
||||
ran: 1,
|
||||
});
|
||||
expect(findProblems(reports, {})).toEqual([]);
|
||||
});
|
||||
|
||||
it("counts a failed or flaky test as having run", () => {
|
||||
const reports = [
|
||||
report([
|
||||
{ file: "f.spec.ts", tests: [["chromium-desktop", "unexpected"]] },
|
||||
{ file: "g.spec.ts", tests: [["chromium-desktop", "flaky"]] },
|
||||
]),
|
||||
];
|
||||
expect(findProblems(reports, {})).toEqual([]);
|
||||
});
|
||||
|
||||
it("accepts an allowlisted file and rejects a stale allowlist entry", () => {
|
||||
const reports = [
|
||||
report([
|
||||
{ file: "a.spec.ts", tests: [["chromium-desktop", "skipped"]] },
|
||||
{ file: "b.spec.ts", tests: [["chromium-desktop", "expected"]] },
|
||||
]),
|
||||
];
|
||||
expect(findProblems(reports, { "a.spec.ts": "reason" })).toEqual([]);
|
||||
expect(findProblems(reports, { "b.spec.ts": "reason" })).toEqual([
|
||||
"a.spec.ts: all 1 test(s) skipped in every project and shard",
|
||||
"b.spec.ts: allowlisted but 1 test(s) ran",
|
||||
]);
|
||||
expect(
|
||||
findProblems(reports, {
|
||||
"a.spec.ts": "reason",
|
||||
"gone.spec.ts": "reason",
|
||||
}),
|
||||
).toEqual(["gone.spec.ts: allowlisted but absent from the report"]);
|
||||
});
|
||||
|
||||
it("refuses an empty report rather than passing over nothing", () => {
|
||||
expect(findProblems([{ suites: [] }], {})).toEqual([
|
||||
"the report(s) hold no tests at all",
|
||||
]);
|
||||
});
|
||||
|
||||
it("gives every allowlist entry a written reason", () => {
|
||||
for (const [file, reason] of Object.entries(ALLOWED_ALL_SKIPPED)) {
|
||||
expect(file).toMatch(/\.spec\.ts$/);
|
||||
expect(reason.trim().length, file).toBeGreaterThan(10);
|
||||
}
|
||||
});
|
||||
|
||||
describe("the command line", () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "e2e-skip-check-"));
|
||||
afterAll(() => rmSync(dir, { recursive: true, force: true }));
|
||||
|
||||
function run(payload: unknown): number {
|
||||
const path = join(dir, `r${Math.random()}.json`);
|
||||
writeFileSync(path, JSON.stringify(payload));
|
||||
try {
|
||||
execFileSync("node", [SCRIPT, path], { stdio: "pipe" });
|
||||
return 0;
|
||||
} catch (err) {
|
||||
return (err as { status: number }).status;
|
||||
}
|
||||
}
|
||||
|
||||
it("exits 0 on a clean report and 1 on an all-skipped file", () => {
|
||||
expect(
|
||||
run(
|
||||
report([
|
||||
{ file: "a.spec.ts", tests: [["chromium-desktop", "expected"]] },
|
||||
]),
|
||||
),
|
||||
).toBe(0);
|
||||
expect(
|
||||
run(
|
||||
report([
|
||||
{ file: "z.spec.ts", tests: [["chromium-desktop", "skipped"]] },
|
||||
]),
|
||||
),
|
||||
).toBe(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("the e2e workflow runs the check", () => {
|
||||
type Step = { uses?: string; run?: string; with?: Record<string, unknown> };
|
||||
const workflow = parse(
|
||||
readFileSync(join(ROOT, ".github/workflows/e2e.yml"), "utf8"),
|
||||
) as { jobs: Record<string, { steps?: Step[] }> };
|
||||
|
||||
it("each shard uploads its JSON report", () => {
|
||||
const upload = workflow.jobs.shard?.steps?.find(
|
||||
(step) =>
|
||||
step.uses?.startsWith("actions/upload-artifact@") &&
|
||||
step.with?.name === "playwright-json-shard-${{ matrix.shard }}",
|
||||
);
|
||||
expect(upload?.with).toMatchObject({
|
||||
path: "playwright-json/results.json",
|
||||
"if-no-files-found": "error",
|
||||
});
|
||||
});
|
||||
|
||||
it("the gate job downloads every shard's report and runs the script", () => {
|
||||
const steps = workflow.jobs.e2e?.steps ?? [];
|
||||
const download = steps.find((step) =>
|
||||
step.uses?.startsWith("actions/download-artifact@"),
|
||||
);
|
||||
expect(download?.with).toMatchObject({
|
||||
pattern: "playwright-json-shard-*",
|
||||
path: "playwright-json",
|
||||
});
|
||||
expect(steps.map((step) => step.run ?? "")).toContain(
|
||||
"node scripts/check-e2e-skipped-specs.mjs playwright-json/*/results.json",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -44,7 +44,11 @@ describe("Playwright CI determinism", () => {
|
||||
|
||||
expect(config.retries).toBe(2);
|
||||
expect(config.failOnFlakyTests).toBe(true);
|
||||
expect(config.reporter).toEqual([["github"], ["html", { open: "never" }]]);
|
||||
expect(config.reporter).toEqual([
|
||||
["github"],
|
||||
["html", { open: "never" }],
|
||||
["json", { outputFile: "playwright-json/results.json" }],
|
||||
]);
|
||||
// Three servers, and which is which matters. The suite's own server runs
|
||||
// with the dashboard's RSC prefetch OFF so `page.route` fixtures govern
|
||||
// what the dashboard paints; the second runs it ON, which is the default
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* Fail CI when a whole e2e spec file ran nothing.
|
||||
*
|
||||
* A spec that skips all of its tests reports green, and in a summary a quiet
|
||||
* skip looks exactly like a pass. `delegated-writes.spec.ts` skipped every
|
||||
* test on every run for months that way: its skip condition was evaluated
|
||||
* before the form it looked for had painted.
|
||||
*
|
||||
* This reads Playwright's JSON report (one per shard; the e2e workflow's gate
|
||||
* job passes all of them) and counts, per spec FILE, the tests that ran across
|
||||
* every project and shard together. A file is only reported when none of its
|
||||
* tests ran anywhere, so a mobile-only spec that skips on the desktop project
|
||||
* is fine as long as the mobile project ran it. Aggregating across shards is
|
||||
* the reason this runs in the gate job and not inside each shard: a shard can
|
||||
* receive only the skipped half of a file.
|
||||
*
|
||||
* A file that is meant to skip entirely goes in `ALLOWED_ALL_SKIPPED` below
|
||||
* with the reason written next to it. The per-call half of the rule, that
|
||||
* every conditional skip carries a reason, is the unit guard
|
||||
* `src/__tests__/e2e-conditional-skip-guard.test.ts`.
|
||||
*
|
||||
* Usage: node scripts/check-e2e-skipped-specs.mjs <report.json> [...]
|
||||
*/
|
||||
import { readFileSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
/**
|
||||
* Spec files allowed to report every test skipped, keyed by the path the JSON
|
||||
* report prints (relative to `testDir`), each with the reason it is allowed.
|
||||
*
|
||||
* @type {Record<string, string>}
|
||||
*/
|
||||
export const ALLOWED_ALL_SKIPPED = {};
|
||||
|
||||
/**
|
||||
* Walk a report's suites and tally each spec file's tests.
|
||||
*
|
||||
* @param {Array<{ suites?: unknown[] }>} reports parsed Playwright JSON reports
|
||||
* @returns {Map<string, { total: number; ran: number }>}
|
||||
*/
|
||||
export function tallySpecFiles(reports) {
|
||||
/** @type {Map<string, { total: number; ran: number }>} */
|
||||
const files = new Map();
|
||||
const visit = (suite) => {
|
||||
for (const spec of suite.specs ?? []) {
|
||||
const entry = files.get(spec.file) ?? { total: 0, ran: 0 };
|
||||
for (const test of spec.tests ?? []) {
|
||||
entry.total += 1;
|
||||
if (test.status !== "skipped") entry.ran += 1;
|
||||
}
|
||||
files.set(spec.file, entry);
|
||||
}
|
||||
for (const child of suite.suites ?? []) visit(child);
|
||||
};
|
||||
for (const report of reports) {
|
||||
for (const suite of report.suites ?? []) visit(suite);
|
||||
}
|
||||
return files;
|
||||
}
|
||||
|
||||
/**
|
||||
* The problems to report: files whose every test skipped and that are not
|
||||
* allowlisted, allowlist entries that no longer apply, and an empty report.
|
||||
*
|
||||
* @param {Array<{ suites?: unknown[] }>} reports
|
||||
* @param {Record<string, string>} [allowed]
|
||||
* @returns {string[]}
|
||||
*/
|
||||
export function findProblems(reports, allowed = ALLOWED_ALL_SKIPPED) {
|
||||
const files = tallySpecFiles(reports);
|
||||
const problems = [];
|
||||
let total = 0;
|
||||
for (const entry of files.values()) total += entry.total;
|
||||
if (total === 0) {
|
||||
// A check over nothing passes for the wrong reason; say so instead.
|
||||
problems.push("the report(s) hold no tests at all");
|
||||
return problems;
|
||||
}
|
||||
for (const [file, { total: count, ran }] of [...files].sort()) {
|
||||
if (ran > 0) continue;
|
||||
if (Object.hasOwn(allowed, file)) continue;
|
||||
problems.push(
|
||||
`${file}: all ${count} test(s) skipped in every project and shard`,
|
||||
);
|
||||
}
|
||||
for (const file of Object.keys(allowed)) {
|
||||
const entry = files.get(file);
|
||||
if (!entry) {
|
||||
problems.push(`${file}: allowlisted but absent from the report`);
|
||||
} else if (entry.ran > 0) {
|
||||
problems.push(`${file}: allowlisted but ${entry.ran} test(s) ran`);
|
||||
}
|
||||
}
|
||||
return problems;
|
||||
}
|
||||
|
||||
function main(paths) {
|
||||
if (paths.length === 0) {
|
||||
console.error("usage: check-e2e-skipped-specs.mjs <report.json> [...]");
|
||||
return 2;
|
||||
}
|
||||
const reports = paths.map((p) => JSON.parse(readFileSync(p, "utf8")));
|
||||
const problems = findProblems(reports);
|
||||
const files = tallySpecFiles(reports);
|
||||
if (problems.length > 0) {
|
||||
console.error("e2e spec files that ran nothing:");
|
||||
for (const problem of problems) console.error(` - ${problem}`);
|
||||
return 1;
|
||||
}
|
||||
console.log(
|
||||
`check-e2e-skipped-specs: ${files.size} spec file(s) across ${paths.length} report(s), each ran at least one test`,
|
||||
);
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (process.argv[1] === fileURLToPath(import.meta.url)) {
|
||||
process.exit(main(process.argv.slice(2)));
|
||||
}
|
||||
Reference in new issue
Block a user