fix(tracing): fix inconsistent calls of tracing.{start,stop} in fixtures (#9594)

Drive-by: fix error logging.

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

@@ -39,7 +39,7 @@ export class Tracing implements api.Tracing {
     });
   }
 
-  async stopChunk(options: { path: string }) {
+  async stopChunk(options: { path?: string } = {}) {
     await this._context._wrapApiCall(async (channel: channels.BrowserContextChannel) => {
       await this._doStopChunk(channel, options.path);
     });

packages/playwright-test/src/index.ts

@@ -290,9 +290,11 @@ export const test = _baseTest.extend<TestFixtures, WorkerAndFileFixtures>({
         if (!(context.tracing as any)[kTracingStarted]) {
           await context.tracing.start({ screenshots: true, snapshots: true });
           (context.tracing as any)[kTracingStarted] = true;
+        } else {
+          await context.tracing.startChunk();
         }
-        await context.tracing.startChunk();
       } else {
+        (context.tracing as any)[kTracingStarted] = false;
         await context.tracing.stop();
       }
       (context as any)._csi = {
@@ -376,6 +378,8 @@ export const test = _baseTest.extend<TestFixtures, WorkerAndFileFixtures>({
     await Promise.all(leftoverContexts.map(async context => {
       if (preserveTrace)
         await context.tracing.stopChunk({ path: addTraceAttachment() });
+      else if (captureTrace)
+        await context.tracing.stopChunk();
       if (captureScreenshots)
         await Promise.all(context.pages().map(page => page.screenshot({ timeout: 5000, path: addScreenshotAttachment() }).catch(() => {})));
     }));

packages/playwright-test/src/reporters/base.ts

@@ -98,7 +98,7 @@ export class BaseReporter implements Reporter  {
   }
 
   onError(error: TestError) {
-    console.log(formatError(error));
+    console.log(formatError(error).message);
   }
 
   async onEnd(result: FullResult) {
@@ -325,7 +325,7 @@ function formatTestHeader(config: FullConfig, test: TestCase, indent: string, in
   return pad(header, '=');
 }
 
-export function formatError(error: TestError, file?: string): ErrorDetails {
+function formatError(error: TestError, file?: string): ErrorDetails {
   const stack = error.stack;
   const tokens = [''];
   let positionInFile: PositionInFile | undefined;

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

@@ -317,3 +317,39 @@ test('should stop tracing with trace: on-first-retry, when not retrying', async
     'report.json',
   ]);
 });
+
+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++) {
+    files[`a${i}.spec.ts`] = `
+      import { test } from './helper';
+      test('flaky', async ({ myContext }, testInfo) => {
+        await new Promise(f => setTimeout(f, 200 + Math.round(Math.random() * 1000)));
+        expect(testInfo.retry).toBe(1);
+      });
+      test('passing', async ({ myContext }, testInfo) => {
+        await new Promise(f => setTimeout(f, 200 + Math.round(Math.random() * 1000)));
+      });
+    `;
+  }
+  const result = await runInlineTest({
+    ...files,
+    'playwright.config.ts': `
+      module.exports = { use: { trace: 'on-first-retry' } };
+    `,
+    'helper.ts': `
+      const { test: base } = pwt;
+      export const test = base.extend({
+        myContext: [async ({ browser }, use) => {
+          const c = await browser.newContext();
+          await use(c);
+          await c.close();
+        }, { scope: 'worker' }]
+      })
+    `,
+  }, { workers: 3, retries: 1 });
+
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(6);
+  expect(result.flaky).toBe(6);
+});

tests/tracing.spec.ts

@@ -237,6 +237,10 @@ test('should work with multiple chunks', async ({ context, page, server }, testI
   await page.hover('"Click"');
   await context.tracing.stopChunk({ path: testInfo.outputPath('trace2.zip') });
 
+  await context.tracing.startChunk();
+  await page.click('"Click"');
+  await context.tracing.stopChunk();  // Should stop without a path.
+
   const trace1 = await parseTrace(testInfo.outputPath('trace.zip'));
   expect(trace1.events[0].type).toBe('context-options');
   expect(trace1.events.find(e => e.metadata?.apiName === 'page.goto')).toBeFalsy();