fix(test runner): failed + skipped = flaky (#26385)

Fixes #17652.
microsoft · Aug 9, 2023 · cadc315 · cadc315
1 parent fe5cb16
commit cadc315
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 12 deletions.
diff --git a/packages/playwright-test/src/common/test.ts b/packages/playwright-test/src/common/test.ts
@@ -256,14 +256,21 @@ export class TestCase extends Base implements reporterTypes.TestCase {
   }
 
   outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky' {
-    const nonSkipped = this.results.filter(result => result.status !== 'skipped' && result.status !== 'interrupted');
-    if (!nonSkipped.length)
+    // Ignore initial skips that may be a result of "skipped because previous test in serial mode failed".
+    const results = [...this.results];
+    while (results[0]?.status === 'skipped' || results[0]?.status === 'interrupted')
+      results.shift();
+
+    // All runs were skipped.
+    if (!results.length)
       return 'skipped';
-    if (nonSkipped.every(result => result.status === this.expectedStatus))
+
+    const failures = results.filter(result => result.status !== 'skipped' && result.status !== 'interrupted' && result.status !== this.expectedStatus);
+    if (!failures.length) // all passed
       return 'expected';
-    if (nonSkipped.some(result => result.status === this.expectedStatus))
-      return 'flaky';
-    return 'unexpected';
+    if (failures.length === results.length) // all failed
+      return 'unexpected';
+    return 'flaky'; // mixed bag
   }
 
   ok(): boolean {

diff --git a/packages/playwright-test/src/isomorphic/teleReceiver.ts b/packages/playwright-test/src/isomorphic/teleReceiver.ts
@@ -482,14 +482,21 @@ export class TeleTestCase implements reporterTypes.TestCase {
   }
 
   outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky' {
-    const nonSkipped = this.results.filter(result => result.status !== 'skipped' && result.status !== 'interrupted');
-    if (!nonSkipped.length)
+    // Ignore initial skips that may be a result of "skipped because previous test in serial mode failed".
+    const results = [...this.results];
+    while (results[0]?.status === 'skipped' || results[0]?.status === 'interrupted')
+      results.shift();
+
+    // All runs were skipped.
+    if (!results.length)
       return 'skipped';
-    if (nonSkipped.every(result => result.status === this.expectedStatus))
+
+    const failures = results.filter(result => result.status !== 'skipped' && result.status !== 'interrupted' && result.status !== this.expectedStatus);
+    if (!failures.length) // all passed
       return 'expected';
-    if (nonSkipped.some(result => result.status === this.expectedStatus))
-      return 'flaky';
-    return 'unexpected';
+    if (failures.length === results.length) // all failed
+      return 'unexpected';
+    return 'flaky'; // mixed bag
   }
 
   ok(): boolean {

diff --git a/tests/playwright-test/retry.spec.ts b/tests/playwright-test/retry.spec.ts
@@ -243,3 +243,22 @@ test('should retry worker fixture setup failure', async ({ runInlineTest }) => {
   expect(result.output.split('\n')[2]).toBe('××F');
   expect(result.output).toContain('worker setup is bugged!');
 });
+
+test('failed and skipped on retry should be marked as flaky', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.spec.ts': `
+      import { test } from '@playwright/test';
+      test('flaky test', async ({}, testInfo) => {
+        if (!testInfo.retry)
+          throw new Error('Failed on first run');
+        test.skip(true, 'Skipped on first retry');
+      });
+    `
+  }, { retries: 1, reporter: 'dot' });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(0);
+  expect(result.failed).toBe(0);
+  expect(result.flaky).toBe(1);
+  expect(result.output).toContain('Failed on first run');
+  expect(result.report.suites[0].specs[0].tests[0].annotations).toEqual([{ type: 'skip', description: 'Skipped on first retry' }]);
+});