Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions docs/changes/unreleased/1978-playwright-failing-checks.md
Original file line number Diff line number Diff line change
@@ -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."
---
39 changes: 33 additions & 6 deletions internal/verify/failing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down
66 changes: 66 additions & 0 deletions internal/verify/failing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
46 changes: 46 additions & 0 deletions internal/verify/testdata/readings/playwright-line.txt
Original file line number Diff line number Diff line change
@@ -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)
101 changes: 101 additions & 0 deletions internal/verify/testdata/readings/playwright-list-flaky.txt
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -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)
Loading