From cadc3153f7e1e2996337f9e0f81bdeca2179df01 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Wed, 9 Aug 2023 16:35:14 -0700
Subject: [PATCH] fix(test runner): failed + skipped = flaky (#26385)

Fixes #17652.
---
 packages/playwright-test/src/common/test.ts   | 19 +++++++++++++------
 .../src/isomorphic/teleReceiver.ts            | 19 +++++++++++++------
 tests/playwright-test/retry.spec.ts           | 19 +++++++++++++++++++
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/packages/playwright-test/src/common/test.ts b/packages/playwright-test/src/common/test.ts
index 067a955a67..3e0d125a3a 100644
--- 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
index e87698344c..2bda3d1b01 100644
--- 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
index 533720d40c..d6cad8a080 100644
--- 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' }]);
+});