fix: preserve trace if context if closed manually during test run (#22442)

Reference #22122, #22120

packages/playwright-test/src/index.ts

@@ -325,13 +325,16 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
     };
 
     const startedCollectingArtifacts = Symbol('startedCollectingArtifacts');
-    const stopTracing = async (tracing: Tracing) => {
+    const stopTracing = async (tracing: Tracing, contextTearDownStarted: boolean) => {
       if ((tracing as any)[startedCollectingArtifacts])
         return;
       (tracing as any)[startedCollectingArtifacts] = true;
       if (captureTrace) {
         let tracePath;
-        if (preserveTrace()) {
+        // Create a trace file if we know that:
+        // - it is's going to be used due to the config setting and the test status or
+        // - we are inside a test or afterEach and the user manually closed the context.
+        if (preserveTrace() || !contextTearDownStarted) {
           tracePath = path.join(_artifactsDir(), createGuid() + '.zip');
           temporaryTraceFiles.push(tracePath);
         }
@@ -363,7 +366,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
       // Do not record empty traces and useless screenshots for them.
       if (reusedContexts.has(context))
         return;
-      await stopTracing(context.tracing);
+      await stopTracing(context.tracing, (context as any)[kStartedContextTearDown]);
       if (screenshotMode === 'on' || screenshotMode === 'only-on-failure') {
         // Capture screenshot for now. We'll know whether we have to preserve them
         // after the test finishes.
@@ -373,7 +376,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
 
     const onWillCloseRequestContext =  async (context: APIRequestContext) => {
       const tracing = (context as any)._tracing as Tracing;
-      await stopTracing(tracing);
+      await stopTracing(tracing, (context as any)[kStartedContextTearDown]);
     };
 
     // 1. Setup instrumentation and process existing contexts.
@@ -435,7 +438,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
 
     // 5. Collect artifacts from any non-closed contexts.
     await Promise.all(leftoverContexts.map(async context => {
-      await stopTracing(context.tracing);
+      await stopTracing(context.tracing, true);
       if (captureScreenshots) {
         await Promise.all(context.pages().map(async page => {
           if ((page as any)[screenshottedSymbol])
@@ -447,7 +450,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
       }
     }).concat(leftoverApiRequests.map(async context => {
       const tracing = (context as any)._tracing as Tracing;
-      await stopTracing(tracing);
+      await stopTracing(tracing, true);
     })));
 
     // 6. Save test trace.
@@ -508,6 +511,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
 
     let counter = 0;
     await Promise.all([...contexts.keys()].map(async context => {
+      (context as any)[kStartedContextTearDown] = true;
       await context.close();
 
       const testFailed = testInfo.status !== testInfo.expectedStatus;
@@ -568,6 +572,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
   request: async ({ playwright }, use) => {
     const request = await playwright.request.newContext();
     await use(request);
+    (request as any)[kStartedContextTearDown] = true;
     await request.dispose();
   },
 });
@@ -657,6 +662,7 @@ function attachConnectedHeaderIfNeeded(testInfo: TestInfo, browser: Browser | nu
 
 const kTracingStarted = Symbol('kTracingStarted');
 const kIsReusedContext = Symbol('kReusedContext');
+const kStartedContextTearDown = Symbol('kStartedContextTearDown');
 
 function connectOptionsFromEnv() {
   const wsEndpoint = process.env.PW_TEST_CONNECT_WS_ENDPOINT;

tests/playwright-test/playwright.trace.spec.ts

@@ -392,3 +392,65 @@ for (const mode of ['off', 'retain-on-failure', 'on-first-retry', 'on-all-retrie
     expect(result.passed).toBe(1);
   });
 }
+
+test(`trace:retain-on-failure should create trace if context is closed before failure in the test`, async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = { use: { trace: 'retain-on-failure' } };
+    `,
+    'a.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('passing test', 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');
+  const trace = await parseTrace(tracePath);
+  expect(trace.actions).toContain('page.goto');
+  expect(result.failed).toBe(1);
+});
+
+test(`trace:retain-on-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: 'retain-on-failure' } };
+    `,
+    'a.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('passing test', async ({ page, context }) => {
+      });
+      test.afterEach(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');
+  const trace = await parseTrace(tracePath);
+  expect(trace.actions).toContain('page.goto');
+  expect(result.failed).toBe(1);
+});
+
+test(`trace:retain-on-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: 'retain-on-failure' } };
+    `,
+    'a.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('passing test', 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');
+  const trace = await parseTrace(tracePath);
+  expect(trace.actions).toContain('apiRequestContext.get');
+  expect(result.failed).toBe(1);
+});