diff --git a/docs/changes/unreleased/1978-playwright-failing-checks.md b/docs/changes/unreleased/1978-playwright-failing-checks.md new file mode 100644 index 0000000000..0075f253c7 --- /dev/null +++ b/docs/changes/unreleased/1978-playwright-failing-checks.md @@ -0,0 +1,8 @@ +--- +kind: fixed +title: a red playwright suite names its failing checks to the gate instead of none +pr: 1978 +surface: [engine] +invalidates: + - "verify.FailingTests read nothing out of playwright's list, line or dot reporter output, so a red playwright suite named no failing checks. It now reads the closing `N failed` section, keeps the project in each name, leaves out checks listed as flaky, and drops the spec file's line and column so an edit above a check does not make it look new." +--- diff --git a/internal/verify/failing.go b/internal/verify/failing.go index 05b3657c19..af9765b9de 100644 --- a/internal/verify/failing.go +++ b/internal/verify/failing.go @@ -91,6 +91,24 @@ var ( // file is not a check: reducing it the way a qualified name reduces would // leave every such reading identified as `py`. sourceFileSuffix = lazyRegexp(`(?i)\.(?:py|go|ts|tsx|js|jsx|mjs|cjs|rb|java|kt|kts|cs|rs|php|swift|scala|c|cc|cpp|h|hpp|m|mm|ex|exs|sh)$`) + // playwrightLocation is the line and column playwright prints after a spec + // file inside its ` › ` chain. Both move whenever a line above the check is + // edited, so a name that kept them would read one unchanged red check as a + // new failure. Only the ` › ` chain is touched: rspec's `./spec/a_spec.rb:12` + // carries a single number and is its runner's identity for the check. + playwrightLocation = lazyRegexp(`(\.[cm]?[jt]sx?):\d+:\d+(\s+›)`) + // playwrightFailedBlock is the `N failed` section of playwright's closing + // summary, which its list, line and dot reporters all print: a count, then + // one indented line per red check. The section is the only place it names + // red checks and nothing else. The numbered banners above it also describe + // a check that went red and then passed on retry, which the summary lists + // under `N flaky` instead, and the `✘` progress line carries a counter + // that changes with worker scheduling. + playwrightFailedBlock = lazyRegexp(`(?m)^[ ]*\d+ failed[ ]*\n((?:[ ]+\S[^\n]*›[^\n]*(?:\n|$))+)`) + // playwrightCheck is one line of that section: an optional project in + // brackets, the spec file with its line and column, and the chain of titles. + // The project is kept because one test in two browsers is two checks. + playwrightCheck = lazyRegexp(`(?m)^[ ]+((?:\[[^\]\n]+\][ ]+›[ ]+)?\S+:\d+:\d+[ ]+›[ ]+.+?)[ ─]*$`) ) // FailingTests reads every test identity a runner named as failing, sorted and @@ -100,14 +118,22 @@ func FailingTests(output string) []string { clean := ansiEscape().ReplaceAllString(output, "") seen := map[string]bool{} var names []string + add := func(raw string) { + name := normalizeTestName(raw) + if name == "" || seen[name] { + return + } + seen[name] = true + names = append(names, name) + } for _, pattern := range failingTestPatterns() { for _, match := range pattern.FindAllStringSubmatch(clean, -1) { - name := normalizeTestName(match[1]) - if name == "" || seen[name] { - continue - } - seen[name] = true - names = append(names, name) + add(match[1]) + } + } + for _, block := range playwrightFailedBlock().FindAllStringSubmatch(clean, -1) { + for _, match := range playwrightCheck().FindAllStringSubmatch(block[1], -1) { + add(match[1]) } } sort.Strings(names) @@ -122,6 +148,7 @@ func FailingTests(output string) []string { // two readings of. func normalizeTestName(raw string) string { name := strings.TrimSpace(trailingTiming().ReplaceAllString(strings.TrimSpace(raw), "")) + name = playwrightLocation().ReplaceAllString(name, "$1$2") name = spaceRun().ReplaceAllString(name, " ") name = strings.Trim(name, ":.,") // A "name" that is a count, a bare verb or a punctuation run is a false diff --git a/internal/verify/failing_test.go b/internal/verify/failing_test.go index e5f12be23c..f172b146c1 100644 --- a/internal/verify/failing_test.go +++ b/internal/verify/failing_test.go @@ -149,3 +149,69 @@ func TestAPathIsNotQualifiedByItsDots(t *testing.T) { } } } + +// Playwright's list, line and dot reporters name red checks in a closing `N +// failed` section and nowhere else in a form that is stable between runs. The +// fixtures are real output of @playwright/test 1.64.1 against a four-check +// spec, with the absolute workspace path replaced by /work/app. +func TestTheChecksPlaywrightReportedAsFailedAreTheOnesReadBackOutOfIt(t *testing.T) { + inTwoBrowsers := []string{ + "[chromium] › tests/e2e/login.spec.ts › login › rejects a wrong password", + "[chromium] › tests/e2e/login.spec.ts › shows the footer", + "[firefox] › tests/e2e/login.spec.ts › login › rejects a wrong password", + "[firefox] › tests/e2e/login.spec.ts › shows the footer", + } + for _, sample := range []struct { + fixture string + want []string + }{{ + // The list reporter with two projects: one test in two browsers is + // two checks, so the project stays in the name. + fixture: "playwright-list-projects.txt", + want: inTwoBrowsers, + }, { + // The same suite after a line was added above every check. Playwright + // prints each check at a new line and column, and a name that kept them + // would read two unchanged red checks as two new failures. + fixture: "playwright-list-projects-moved.txt", + want: inTwoBrowsers, + }, { + // The line reporter with no projects configured prints no brackets. + fixture: "playwright-line.txt", + want: []string{ + "tests/e2e/login.spec.ts › login › rejects a wrong password", + "tests/e2e/login.spec.ts › shows the footer", + }, + }, { + // A check that failed and then passed on retry gets a numbered failure + // banner like the red ones, and the summary lists it under `1 flaky`. + // It passed, so it is not read as failing. + fixture: "playwright-list-flaky.txt", + want: []string{ + "[chromium] › tests/e2e/login.spec.ts › login › rejects a wrong password", + "[chromium] › tests/e2e/login.spec.ts › shows the footer", + }, + }} { + got := FailingTests(fixture(t, sample.fixture)) + if !reflect.DeepEqual(got, sample.want) { + t.Errorf("%s: read %q, want %q", sample.fixture, got, sample.want) + } + } +} + +// Only playwright's own location grammar loses its numbers: a spec file inside +// a ` › ` chain. rspec names a red example by file and line, and that line is +// the example's identity to rspec. +func TestOnlyPlaywrightsMovingLocationIsStrippedFromAName(t *testing.T) { + // The leading `./` is trimmed by normalizeTestName as it always was; the + // line number is what this asserts. + if got := FailingTests("rspec ./spec/api_spec.rb:12 # Api returns headers\n"); !reflect.DeepEqual(got, []string{"/spec/api_spec.rb:12"}) { + t.Errorf("an rspec location was rewritten: %q", got) + } + // mocha also numbers its failures `1) ...`, but prints no `N failed` + // section in this shape, so nothing of it is read as a playwright check. + mocha := " 2 passing (5ms)\n 1 failing\n\n 1) Array\n #indexOf()\n returns -1:\n AssertionError\n" + if got := FailingTests(mocha); len(got) != 0 { + t.Errorf("mocha's numbered failure list was read as playwright checks: %q", got) + } +} diff --git a/internal/verify/testdata/readings/playwright-line.txt b/internal/verify/testdata/readings/playwright-line.txt new file mode 100644 index 0000000000..a442f27c5e --- /dev/null +++ b/internal/verify/testdata/readings/playwright-line.txt @@ -0,0 +1,46 @@ + +Running 3 tests using 1 worker + +[1/3] tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password + 1) tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password ──────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: 2 + Received: 1 + + 3 | + 4 | test.describe('login', () => { + > 5 | test('rejects a wrong password', async () => { expect(1).toBe(2); }); + | ^ + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + at /private/work/app/tests/e2e/login.spec.ts:5:60 + + Error Context: test-results/e2e-login-login-rejects-a-wrong-password/error-context.md + + +[2/3] tests/e2e/login.spec.ts:6:7 › login › accepts the right password +[3/3] tests/e2e/login.spec.ts:8:5 › shows the footer + 2) tests/e2e/login.spec.ts:8:5 › shows the footer ──────────────────────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: "b" + Received: "a" + + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + > 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + | ^ + 9 | + at /private/work/app/tests/e2e/login.spec.ts:8:52 + + Error Context: test-results/e2e-login-shows-the-footer/error-context.md + + + 2 failed + tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password ───────────────────────────────── + tests/e2e/login.spec.ts:8:5 › shows the footer ───────────────────────────────────────────────── + 1 passed (5ms) diff --git a/internal/verify/testdata/readings/playwright-list-flaky.txt b/internal/verify/testdata/readings/playwright-list-flaky.txt new file mode 100644 index 0000000000..647e737220 --- /dev/null +++ b/internal/verify/testdata/readings/playwright-list-flaky.txt @@ -0,0 +1,101 @@ + +Running 4 tests using 1 worker + + ✘ 1 [chromium] › tests/e2e/flaky.spec.ts:2:5 › loads the dashboard (5ms) + ✓ 2 [chromium] › tests/e2e/flaky.spec.ts:2:5 › loads the dashboard (retry #1) (5ms) + ✘ 3 [chromium] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password (5ms) + ✘ 4 [chromium] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password (retry #1) (5ms) + ✓ 5 [chromium] › tests/e2e/login.spec.ts:6:7 › login › accepts the right password (5ms) + ✘ 6 [chromium] › tests/e2e/login.spec.ts:8:5 › shows the footer (5ms) + ✘ 7 [chromium] › tests/e2e/login.spec.ts:8:5 › shows the footer (retry #1) (5ms) + + + 1) [chromium] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password ─────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: 2 + Received: 1 + + 3 | + 4 | test.describe('login', () => { + > 5 | test('rejects a wrong password', async () => { expect(1).toBe(2); }); + | ^ + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + at /private/work/app/tests/e2e/login.spec.ts:5:60 + + Error Context: test-results/e2e-login-login-rejects-a-wrong-password-chromium/error-context.md + + Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: 2 + Received: 1 + + 3 | + 4 | test.describe('login', () => { + > 5 | test('rejects a wrong password', async () => { expect(1).toBe(2); }); + | ^ + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + at /private/work/app/tests/e2e/login.spec.ts:5:60 + + Error Context: test-results/e2e-login-login-rejects-a-wrong-password-chromium-retry1/error-context.md + + 2) [chromium] › tests/e2e/login.spec.ts:8:5 › shows the footer ─────────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: "b" + Received: "a" + + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + > 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + | ^ + 9 | + at /private/work/app/tests/e2e/login.spec.ts:8:52 + + Error Context: test-results/e2e-login-shows-the-footer-chromium/error-context.md + + Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: "b" + Received: "a" + + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + > 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + | ^ + 9 | + at /private/work/app/tests/e2e/login.spec.ts:8:52 + + Error Context: test-results/e2e-login-shows-the-footer-chromium-retry1/error-context.md + + 3) [chromium] › tests/e2e/flaky.spec.ts:2:5 › loads the dashboard ──────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: 1 + Received: 0 + + 1 | import { test, expect } from '@playwright/test'; + > 2 | test('loads the dashboard', async ({}, info) => { expect(info.retry).toBe(1); }); + | ^ + 3 | + at /private/work/app/tests/e2e/flaky.spec.ts:2:70 + + Error Context: test-results/e2e-flaky-loads-the-dashboard-chromium/error-context.md + + 2 failed + [chromium] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password ──────────────────── + [chromium] › tests/e2e/login.spec.ts:8:5 › shows the footer ──────────────────────────────────── + 1 flaky + [chromium] › tests/e2e/flaky.spec.ts:2:5 › loads the dashboard ───────────────────────────────── + 1 passed (5ms) diff --git a/internal/verify/testdata/readings/playwright-list-projects-moved.txt b/internal/verify/testdata/readings/playwright-list-projects-moved.txt new file mode 100644 index 0000000000..201d0ed925 --- /dev/null +++ b/internal/verify/testdata/readings/playwright-list-projects-moved.txt @@ -0,0 +1,85 @@ + +Running 6 tests using 1 worker + + ✘ 1 [chromium] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password (5ms) + ✓ 2 [chromium] › tests/e2e/login.spec.ts:6:7 › login › accepts the right password (5ms) + ✘ 3 [chromium] › tests/e2e/login.spec.ts:8:5 › shows the footer (5ms) + ✘ 4 [firefox] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password (5ms) + ✓ 5 [firefox] › tests/e2e/login.spec.ts:6:7 › login › accepts the right password (5ms) + ✘ 6 [firefox] › tests/e2e/login.spec.ts:8:5 › shows the footer (5ms) + + + 1) [chromium] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password ─────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: 2 + Received: 1 + + 3 | + 4 | test.describe('login', () => { + > 5 | test('rejects a wrong password', async () => { expect(1).toBe(2); }); + | ^ + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + at /private/work/app/tests/e2e/login.spec.ts:5:60 + + Error Context: test-results/e2e-login-login-rejects-a-wrong-password-chromium/error-context.md + + 2) [chromium] › tests/e2e/login.spec.ts:8:5 › shows the footer ─────────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: "b" + Received: "a" + + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + > 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + | ^ + 9 | + at /private/work/app/tests/e2e/login.spec.ts:8:52 + + Error Context: test-results/e2e-login-shows-the-footer-chromium/error-context.md + + 3) [firefox] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password ──────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: 2 + Received: 1 + + 3 | + 4 | test.describe('login', () => { + > 5 | test('rejects a wrong password', async () => { expect(1).toBe(2); }); + | ^ + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + at /private/work/app/tests/e2e/login.spec.ts:5:60 + + Error Context: test-results/e2e-login-login-rejects-a-wrong-password-firefox/error-context.md + + 4) [firefox] › tests/e2e/login.spec.ts:8:5 › shows the footer ──────────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: "b" + Received: "a" + + 6 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 7 | }); + > 8 | test('shows the footer', async () => { expect('a').toBe('b'); }); + | ^ + 9 | + at /private/work/app/tests/e2e/login.spec.ts:8:52 + + Error Context: test-results/e2e-login-shows-the-footer-firefox/error-context.md + + 4 failed + [chromium] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password ──────────────────── + [chromium] › tests/e2e/login.spec.ts:8:5 › shows the footer ──────────────────────────────────── + [firefox] › tests/e2e/login.spec.ts:5:7 › login › rejects a wrong password ───────────────────── + [firefox] › tests/e2e/login.spec.ts:8:5 › shows the footer ───────────────────────────────────── + 2 passed (5ms) diff --git a/internal/verify/testdata/readings/playwright-list-projects.txt b/internal/verify/testdata/readings/playwright-list-projects.txt new file mode 100644 index 0000000000..b2ba9ef9df --- /dev/null +++ b/internal/verify/testdata/readings/playwright-list-projects.txt @@ -0,0 +1,85 @@ + +Running 6 tests using 1 worker + + ✘ 1 [chromium] › tests/e2e/login.spec.ts:3:7 › login › rejects a wrong password (5ms) + ✓ 2 [chromium] › tests/e2e/login.spec.ts:4:7 › login › accepts the right password (5ms) + ✘ 3 [chromium] › tests/e2e/login.spec.ts:6:5 › shows the footer (5ms) + ✘ 4 [firefox] › tests/e2e/login.spec.ts:3:7 › login › rejects a wrong password (5ms) + ✓ 5 [firefox] › tests/e2e/login.spec.ts:4:7 › login › accepts the right password (5ms) + ✘ 6 [firefox] › tests/e2e/login.spec.ts:6:5 › shows the footer (5ms) + + + 1) [chromium] › tests/e2e/login.spec.ts:3:7 › login › rejects a wrong password ─────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: 2 + Received: 1 + + 1 | import { test, expect } from '@playwright/test'; + 2 | test.describe('login', () => { + > 3 | test('rejects a wrong password', async () => { expect(1).toBe(2); }); + | ^ + 4 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 5 | }); + 6 | test('shows the footer', async () => { expect('a').toBe('b'); }); + at /private/work/app/tests/e2e/login.spec.ts:3:60 + + Error Context: test-results/e2e-login-login-rejects-a-wrong-password-chromium/error-context.md + + 2) [chromium] › tests/e2e/login.spec.ts:6:5 › shows the footer ─────────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: "b" + Received: "a" + + 4 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 5 | }); + > 6 | test('shows the footer', async () => { expect('a').toBe('b'); }); + | ^ + 7 | + at /private/work/app/tests/e2e/login.spec.ts:6:52 + + Error Context: test-results/e2e-login-shows-the-footer-chromium/error-context.md + + 3) [firefox] › tests/e2e/login.spec.ts:3:7 › login › rejects a wrong password ──────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: 2 + Received: 1 + + 1 | import { test, expect } from '@playwright/test'; + 2 | test.describe('login', () => { + > 3 | test('rejects a wrong password', async () => { expect(1).toBe(2); }); + | ^ + 4 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 5 | }); + 6 | test('shows the footer', async () => { expect('a').toBe('b'); }); + at /private/work/app/tests/e2e/login.spec.ts:3:60 + + Error Context: test-results/e2e-login-login-rejects-a-wrong-password-firefox/error-context.md + + 4) [firefox] › tests/e2e/login.spec.ts:6:5 › shows the footer ──────────────────────────────────── + + Error: expect(received).toBe(expected) // Object.is equality + + Expected: "b" + Received: "a" + + 4 | test('accepts the right password', async () => { expect(1).toBe(1); }); + 5 | }); + > 6 | test('shows the footer', async () => { expect('a').toBe('b'); }); + | ^ + 7 | + at /private/work/app/tests/e2e/login.spec.ts:6:52 + + Error Context: test-results/e2e-login-shows-the-footer-firefox/error-context.md + + 4 failed + [chromium] › tests/e2e/login.spec.ts:3:7 › login › rejects a wrong password ──────────────────── + [chromium] › tests/e2e/login.spec.ts:6:5 › shows the footer ──────────────────────────────────── + [firefox] › tests/e2e/login.spec.ts:3:7 › login › rejects a wrong password ───────────────────── + [firefox] › tests/e2e/login.spec.ts:6:5 › shows the footer ───────────────────────────────────── + 2 passed (5ms)