From 4fc2d1e24cf444c2949fca4e3fadffb2deb52268 Mon Sep 17 00:00:00 2001
From: Stevan Freeborn <65925598+StevanFreeborn@users.noreply.github.com>
Date: Fri, 23 Feb 2024 19:58:00 -0600
Subject: [PATCH] test: correct tests for on-first-failure mode

---
 .../playwright-test/playwright.trace.spec.ts  | 50 ++++++++++++-------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts
index f020552916..27e6884962 100644
--- a/tests/playwright-test/playwright.trace.spec.ts
+++ b/tests/playwright-test/playwright.trace.spec.ts
@@ -1034,21 +1034,39 @@ test('should attribute worker fixture teardown to the right test', async ({ runI
   ]);
 });
 
-test('trace:on-first-failure should create trace if context is closed before failure in the test', async ({ runInlineTest }) => {
+test('trace:on-first-failure should create trace but only on first failure', async ({ runInlineTest }) => {
   const result = await runInlineTest({
-    'playwright.config.ts': `
-      module.exports = { use: { trace: 'on-first-failure' } };
-    `,
     'a.spec.ts': `
       import { test, expect } from '@playwright/test';
-      test('passing test', async ({ page, context }) => {
+      test('fail', async ({ page }) => {
+        await page.goto('about:blank');
+        expect(true).toBe(false);
+      });
+    `,
+  }, { trace: 'on-first-failure', retries: 1 });
+
+  const retryTracePath = test.info().outputPath('test-results', 'a-fail-retry1', 'trace.zip');
+  const retryTraceExists = fs.existsSync(retryTracePath);
+  expect(retryTraceExists).toBe(false);
+
+  const tracePath = test.info().outputPath('test-results', 'a-fail', 'trace.zip');
+  const trace = await parseTrace(tracePath);
+  expect(trace.apiNames).toContain('page.goto');
+  expect(result.failed).toBe(1);
+});
+
+test('trace:on-first-failure should create trace if context is closed before failure in the test', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('fail', async ({ page, context }) => {
         await page.goto('about:blank');
         await context.close();
         expect(1).toBe(2);
       });
     `,
-  }, { trace: 'retain-on-failure' });
-  const tracePath = test.info().outputPath('test-results', 'a-passing-test', 'trace.zip');
+  }, { trace: 'on-first-failure' });
+  const tracePath = test.info().outputPath('test-results', 'a-fail', 'trace.zip');
   const trace = await parseTrace(tracePath);
   expect(trace.apiNames).toContain('page.goto');
   expect(result.failed).toBe(1);
@@ -1056,12 +1074,9 @@ test('trace:on-first-failure should create trace if context is closed before fai
 
 test('trace:on-first-failure should create trace if context is closed before failure in afterEach', async ({ runInlineTest }) => {
   const result = await runInlineTest({
-    'playwright.config.ts': `
-      module.exports = { use: { trace: 'on-first-failure' } };
-    `,
     'a.spec.ts': `
       import { test, expect } from '@playwright/test';
-      test('passing test', async ({ page, context }) => {
+      test('fail', async ({ page, context }) => {
       });
       test.afterEach(async ({ page, context }) => {
         await page.goto('about:blank');
@@ -1069,8 +1084,8 @@ test('trace:on-first-failure should create trace if context is closed before fai
         expect(1).toBe(2);
       });
     `,
-  }, { trace: 'retain-on-failure' });
-  const tracePath = test.info().outputPath('test-results', 'a-passing-test', 'trace.zip');
+  }, { trace: 'on-first-failure' });
+  const tracePath = test.info().outputPath('test-results', 'a-fail', 'trace.zip');
   const trace = await parseTrace(tracePath);
   expect(trace.apiNames).toContain('page.goto');
   expect(result.failed).toBe(1);
@@ -1078,19 +1093,16 @@ test('trace:on-first-failure should create trace if context is closed before fai
 
 test('trace:on-first-failure should create trace if request context is disposed before failure', async ({ runInlineTest, server }) => {
   const result = await runInlineTest({
-    'playwright.config.ts': `
-      module.exports = { use: { trace: 'on-first-failure' } };
-    `,
     'a.spec.ts': `
       import { test, expect } from '@playwright/test';
-      test('passing test', async ({ request }) => {
+      test('fail', async ({ request }) => {
         expect(await request.get('${server.EMPTY_PAGE}')).toBeOK();
         await request.dispose();
         expect(1).toBe(2);
       });
     `,
-  }, { trace: 'retain-on-failure' });
-  const tracePath = test.info().outputPath('test-results', 'a-passing-test', 'trace.zip');
+  }, { trace: 'on-first-failure' });
+  const tracePath = test.info().outputPath('test-results', 'a-fail', 'trace.zip');
   const trace = await parseTrace(tracePath);
   expect(trace.apiNames).toContain('apiRequestContext.get');
   expect(result.failed).toBe(1);