From 051afb9ce02242f97bcbf1a9ea7ac97b7ccb4eae Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Mon, 25 Mar 2024 17:04:03 -0700
Subject: [PATCH] fix(test runner): update fixture teardown error message
 (#30109)

With the recent change that gave after hooks a separate timeout, fixture
teardown does not imply that "test finished successfully, but fixture
teardown was slow".
---
 packages/playwright/src/worker/timeoutManager.ts | 10 +++-------
 tests/playwright-test/fixture-errors.spec.ts     |  8 ++++----
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/packages/playwright/src/worker/timeoutManager.ts b/packages/playwright/src/worker/timeoutManager.ts
index 6287bc232c..441a87eca0 100644
--- a/packages/playwright/src/worker/timeoutManager.ts
+++ b/packages/playwright/src/worker/timeoutManager.ts
@@ -137,14 +137,10 @@ export class TimeoutManager {
     switch (runnable.type) {
       case 'test': {
         if (runnable.fixture) {
-          if (runnable.fixture.phase === 'setup') {
+          if (runnable.fixture.phase === 'setup')
             message = `Test timeout of ${timeout}ms exceeded while setting up "${runnable.fixture.title}".`;
-          } else {
-            message = [
-              `Test finished within timeout of ${timeout}ms, but tearing down "${runnable.fixture.title}" ran out of time.`,
-              `Please allow more time for the test, since teardown is attributed towards the test timeout budget.`,
-            ].join('\n');
-          }
+          else
+            message = `Tearing down "${runnable.fixture.title}" exceeded the test timeout of ${timeout}ms.`;
         } else {
           message = `Test timeout of ${timeout}ms exceeded.`;
         }
diff --git a/tests/playwright-test/fixture-errors.spec.ts b/tests/playwright-test/fixture-errors.spec.ts
index ae323c287a..c17db65bce 100644
--- a/tests/playwright-test/fixture-errors.spec.ts
+++ b/tests/playwright-test/fixture-errors.spec.ts
@@ -37,7 +37,7 @@ test('should handle fixture timeout', async ({ runInlineTest }) => {
     `
   }, { timeout: 500 });
   expect(result.exitCode).toBe(1);
-  expect(result.output).toContain('Test finished within timeout of 500ms, but tearing down "timeout" ran out of time.');
+  expect(result.output).toContain('Tearing down "timeout" exceeded the test timeout of 500ms.');
   expect(result.failed).toBe(2);
 });
 
@@ -459,7 +459,7 @@ test('should not give enough time for second fixture teardown after timeout', as
   }, { timeout: 2000 });
   expect(result.exitCode).toBe(1);
   expect(result.failed).toBe(1);
-  expect(result.output).toContain('Test finished within timeout of 3000ms, but tearing down "fixture" ran out of time.');
+  expect(result.output).toContain('Tearing down "fixture" exceeded the test timeout of 3000ms.');
   expect(result.outputLines).toEqual([
     'teardown start',
     'teardown finished',
@@ -525,7 +525,7 @@ test('should not report fixture teardown timeout twice', async ({ runInlineTest
   }, { reporter: 'list', timeout: 1000 });
   expect(result.exitCode).toBe(1);
   expect(result.failed).toBe(1);
-  expect(result.output).toContain('Test finished within timeout of 1000ms, but tearing down "fixture" ran out of time.');
+  expect(result.output).toContain('Tearing down "fixture" exceeded the test timeout of 1000ms.');
   expect(result.output).not.toContain('base.extend'); // Should not point to the location.
   expect(result.output).not.toContain('Worker teardown timeout');
 });
@@ -730,7 +730,7 @@ test('should not continue with scope teardown after fixture teardown timeout', a
   }, { reporter: 'list', timeout: 1000 });
   expect(result.exitCode).toBe(1);
   expect(result.failed).toBe(1);
-  expect(result.output).toContain('Test finished within timeout of 1000ms, but tearing down "fixture2" ran out of time.');
+  expect(result.output).toContain('Tearing down "fixture2" exceeded the test timeout of 1000ms.');
   expect(result.output).not.toContain('in fixture teardown');
 });