feat(fixtures): respect tracing config for APIRequestContext (#11954)

packages/playwright-core/src/client/fetch.ts

@@ -74,13 +74,14 @@ export class APIRequest implements api.APIRequest {
     })).request);
     context._tracing._localUtils = this._playwright._utils;
     this._contexts.add(context);
+    context._request = this;
     await this._onDidCreateContext?.(context);
     return context;
   }
 }
 
 export class APIRequestContext extends ChannelOwner<channels.APIRequestContextChannel> implements api.APIRequestContext {
-  private _request?: APIRequest;
+  _request?: APIRequest;
   readonly _tracing: Tracing;
 
   static from(channel: channels.APIRequestContextChannel): APIRequestContext {
@@ -89,8 +90,6 @@ export class APIRequestContext extends ChannelOwner<channels.APIRequestContextCh
 
   constructor(parent: ChannelOwner, type: string, guid: string, initializer: channels.APIRequestContextInitializer) {
     super(parent, type, guid, initializer, createInstrumentation());
-    if (parent instanceof APIRequest)
-      this._request = parent;
     this._tracing = Tracing.from(initializer.tracing);
   }
 

packages/playwright-test/src/index.ts

@@ -16,7 +16,7 @@
 
 import * as fs from 'fs';
 import * as path from 'path';
-import type { LaunchOptions, BrowserContextOptions, Page, BrowserContext, Video, APIRequestContext } from 'playwright-core';
+import type { LaunchOptions, BrowserContextOptions, Page, BrowserContext, Video, APIRequestContext, Tracing } from 'playwright-core';
 import type { TestType, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, TestInfo } from '../types/test';
 import { rootTestType } from './testType';
 import { createGuid, removeFolders, debugMode } from 'playwright-core/lib/utils/utils';
@@ -257,41 +257,51 @@ export const test = _baseTest.extend<TestFixtures, WorkerFixtures>({
       };
     };
 
+    const startTracing = async (tracing: Tracing) => {
+      if (captureTrace) {
+        const title = [path.relative(testInfo.project.testDir, testInfo.file) + ':' + testInfo.line, ...testInfo.titlePath.slice(1)].join(' › ');
+        if (!(tracing as any)[kTracingStarted]) {
+          await tracing.start({ ...traceOptions, title });
+          (tracing as any)[kTracingStarted] = true;
+        } else {
+          await tracing.startChunk({ title });
+        }
+      } else {
+        (tracing as any)[kTracingStarted] = false;
+        await tracing.stop();
+      }
+    };
+
     const onDidCreateBrowserContext = async (context: BrowserContext) => {
       createdContexts.add(context);
       context.setDefaultTimeout(testInfo.timeout === 0 ? 0 : (actionTimeout || 0));
       context.setDefaultNavigationTimeout(testInfo.timeout === 0 ? 0 : (navigationTimeout || actionTimeout || 0));
-      if (captureTrace) {
+      await startTracing(context.tracing);
-        const title = [path.relative(testInfo.project.testDir, testInfo.file) + ':' + testInfo.line, ...testInfo.titlePath.slice(1)].join(' › ');
-        if (!(context.tracing as any)[kTracingStarted]) {
-          await context.tracing.start({ ...traceOptions, title });
-          (context.tracing as any)[kTracingStarted] = true;
-        } else {
-          await context.tracing.startChunk({ title });
-        }
-      } else {
-        (context.tracing as any)[kTracingStarted] = false;
-        await context.tracing.stop();
-      }
       const listener = createInstrumentationListener(context);
       (context as any)._instrumentation.addListener(listener);
       (context.request as any)._instrumentation.addListener(listener);
     };
     const onDidCreateRequestContext = async (context: APIRequestContext) => {
+      const tracing = (context as any)._tracing as Tracing;
+      await startTracing(tracing);
       (context as any)._instrumentation.addListener(createInstrumentationListener());
     };
 
     const startedCollectingArtifacts = Symbol('startedCollectingArtifacts');
 
-    const onWillCloseContext = async (context: BrowserContext) => {
+    const stopTracing = async (tracing: Tracing) => {
-      (context as any)[startedCollectingArtifacts] = true;
+      (tracing as any)[startedCollectingArtifacts] = true;
       if (captureTrace) {
         // Export trace for now. We'll know whether we have to preserve it
         // after the test finishes.
         const tracePath = path.join(_artifactsDir(), createGuid() + '.zip');
         temporaryTraceFiles.push(tracePath);
-        await context.tracing.stopChunk({ path: tracePath });
+        await tracing.stopChunk({ path: tracePath });
       }
+    };
+
+    const onWillCloseContext = async (context: BrowserContext) => {
+      await stopTracing(context.tracing);
       if (screenshot === 'on' || screenshot === 'only-on-failure') {
         // Capture screenshot for now. We'll know whether we have to preserve them
         // after the test finishes.
@@ -303,6 +313,11 @@ export const test = _baseTest.extend<TestFixtures, WorkerFixtures>({
       }
     };
 
+    const onWillCloseRequestContext =  async (context: APIRequestContext) => {
+      const tracing = (context as any)._tracing as Tracing;
+      await stopTracing(tracing);
+    };
+
     // 1. Setup instrumentation and process existing contexts.
     for (const browserType of [playwright.chromium, playwright.firefox, playwright.webkit]) {
       (browserType as any)._onDidCreateContext = onDidCreateBrowserContext;
@@ -312,7 +327,12 @@ export const test = _baseTest.extend<TestFixtures, WorkerFixtures>({
       const existingContexts = Array.from((browserType as any)._contexts) as BrowserContext[];
       await Promise.all(existingContexts.map(onDidCreateBrowserContext));
     }
-    (playwright.request as any)._onDidCreateContext = onDidCreateRequestContext;
+    {
+      (playwright.request as any)._onDidCreateContext = onDidCreateRequestContext;
+      (playwright.request as any)._onWillCloseContext = onWillCloseRequestContext;
+      const existingApiRequests: APIRequestContext[] =  Array.from((playwright.request as any)._contexts as Set<APIRequestContext>);
+      await Promise.all(existingApiRequests.map(onDidCreateRequestContext));
+    }
 
     // 2. Run the test.
     await use();
@@ -348,25 +368,35 @@ export const test = _baseTest.extend<TestFixtures, WorkerFixtures>({
       (browserType as any)._defaultLaunchOptions = undefined;
     }
     leftoverContexts.forEach(context => (context as any)._instrumentation.removeAllListeners());
-    (playwright.request as any)._onDidCreateContext = undefined;
     for (const context of (playwright.request as any)._contexts)
       context._instrumentation.removeAllListeners();
+    const leftoverApiRequests: APIRequestContext[] =  Array.from((playwright.request as any)._contexts as Set<APIRequestContext>);
+    (playwright.request as any)._onDidCreateContext = undefined;
+    (playwright.request as any)._onWillCloseContext = undefined;
 
-    // 5. Collect artifacts from any non-closed contexts.
+    const stopTraceChunk = async (tracing: Tracing): Promise<boolean> => {
-    await Promise.all(leftoverContexts.map(async context => {
       // When we timeout during context.close(), we might end up with context still alive
       // but artifacts being already collected. In this case, do not collect artifacts
       // for the second time.
-      if ((context as any)[startedCollectingArtifacts])
+      if ((tracing as any)[startedCollectingArtifacts])
-        return;
+        return false;
-
       if (preserveTrace)
-        await context.tracing.stopChunk({ path: addTraceAttachment() });
+        await tracing.stopChunk({ path: addTraceAttachment() });
       else if (captureTrace)
-        await context.tracing.stopChunk();
+        await tracing.stopChunk();
+      return true;
+    };
+
+    // 5. Collect artifacts from any non-closed contexts.
+    await Promise.all(leftoverContexts.map(async context => {
+      if (!await stopTraceChunk(context.tracing))
+        return;
       if (captureScreenshots)
         await Promise.all(context.pages().map(page => page.screenshot({ timeout: 5000, path: addScreenshotAttachment() }).catch(() => {})));
-    }));
+    }).concat(leftoverApiRequests.map(async context => {
+      const tracing = (context as any)._tracing as Tracing;
+      await stopTraceChunk(tracing);
+    })));
 
     // 6. Either remove or attach temporary traces and screenshots for contexts closed
     // before the test has finished.

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

@@ -54,6 +54,47 @@ test('should stop tracing with trace: on-first-retry, when not retrying', async
   expect(fs.existsSync(testInfo.outputPath('test-results', 'a-shared-flaky-retry1', 'trace.zip'))).toBeTruthy();
 });
 
+test('should record api trace', async ({ runInlineTest, server }, testInfo) => {
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = { use: { trace: 'on' } };
+    `,
+    'a.spec.ts': `
+      const { test } = pwt;
+
+      test('pass', async ({request, page}, testInfo) => {
+        await page.goto('about:blank');
+        await request.get('${server.EMPTY_PAGE}');
+      });
+
+      test('api pass', async ({playwright}, testInfo) => {
+        const request = await playwright.request.newContext();
+        await request.get('${server.EMPTY_PAGE}');
+      });
+
+      test('fail', async ({request, page}, testInfo) => {
+        await page.goto('about:blank');
+        await request.get('${server.EMPTY_PAGE}');
+        expect(1).toBe(2);
+      });
+    `,
+  }, { workers: 1 });
+
+  expect(result.exitCode).toBe(1);
+  expect(result.passed).toBe(2);
+  expect(result.failed).toBe(1);
+  // One trace file for request context and one for each APIRequestContext
+  expect(fs.existsSync(testInfo.outputPath('test-results', 'a-pass', 'trace.zip'))).toBeTruthy();
+  expect(fs.existsSync(testInfo.outputPath('test-results', 'a-pass', 'trace-1.zip'))).toBeTruthy();
+  expect(fs.existsSync(testInfo.outputPath('test-results', 'a-api-pass', 'trace.zip'))).toBeTruthy();
+  expect(fs.existsSync(testInfo.outputPath('test-results', 'a-api-pass', 'trace-1.zip'))).toBeFalsy();
+  expect(fs.existsSync(testInfo.outputPath('test-results', 'a-fail', 'trace.zip'))).toBeTruthy();
+  expect(fs.existsSync(testInfo.outputPath('test-results', 'a-fail', 'trace-1.zip'))).toBeTruthy();
+  // One leftover global APIRequestContext from 'api pass' test.
+  expect(fs.existsSync(testInfo.outputPath('test-results', 'a-fail', 'trace-2.zip'))).toBeTruthy();
+});
+
+
 test('should not throw with trace: on-first-retry and two retries in the same worker', async ({ runInlineTest }, testInfo) => {
   const files = {};
   for (let i = 0; i < 6; i++) {