chore: do not add to the internal action logs (#28365)

Fixes #28319
microsoft · Nov 28, 2023 · 022b363 · 022b363
1 parent cea28b2
commit 022b363
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 6 deletions.
diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts
@@ -373,6 +373,8 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
   }
 
   onCallLog(sdkObject: SdkObject, metadata: CallMetadata, logName: string, message: string) {
+    if (metadata.isServerSide || metadata.internal)
+      return;
     if (logName !== 'api')
       return;
     const event = createActionLogTraceEvent(metadata, message);

diff --git a/packages/trace-viewer/src/traceModel.ts b/packages/trace-viewer/src/traceModel.ts
@@ -190,7 +190,10 @@ export class TraceModel {
       }
       case 'log': {
         const existing = actionMap.get(event.callId);
-        existing!.log.push({
+        // We have some corrupted traces out there, tolerate them.
+        if (!existing)
+          return;
+        existing.log.push({
           time: event.time,
           message: event.message,
         });

diff --git a/tests/playwright-test/ui-mode-trace.spec.ts b/tests/playwright-test/ui-mode-trace.spec.ts
@@ -19,7 +19,7 @@ import { test, expect, retries } from './ui-mode-fixtures';
 
 test.describe.configure({ mode: 'parallel', retries });
 
-test('should merge trace events', async ({ runUITest, server }) => {
+test('should merge trace events', async ({ runUITest }) => {
   const { page } = await runUITest({
     'a.test.ts': `
       import { test, expect } from '@playwright/test';
@@ -99,7 +99,7 @@ test('should merge screenshot assertions', async ({  runUITest }, testInfo) => {
   ]);
 });
 
-test('should locate sync assertions in source', async ({ runUITest, server }) => {
+test('should locate sync assertions in source', async ({ runUITest }) => {
   const { page } = await runUITest({
     'a.test.ts': `
       import { test, expect } from '@playwright/test';
@@ -118,7 +118,7 @@ test('should locate sync assertions in source', async ({ runUITest, server }) =>
   ).toHaveText('4        expect(1).toBe(1);');
 });
 
-test('should show snapshots for sync assertions', async ({ runUITest, server }) => {
+test('should show snapshots for sync assertions', async ({ runUITest }) => {
   const { page } = await runUITest({
     'a.test.ts': `
       import { test, expect } from '@playwright/test';
@@ -150,7 +150,7 @@ test('should show snapshots for sync assertions', async ({ runUITest, server })
   ).toHaveText('Submit');
 });
 
-test('should show image diff', async ({ runUITest, server }) => {
+test('should show image diff', async ({ runUITest }) => {
   const { page } = await runUITest({
     'playwright.config.js': `
       module.exports = {
@@ -175,7 +175,7 @@ test('should show image diff', async ({ runUITest, server }) => {
   await expect(page.locator('.image-diff-view .image-wrapper img')).toBeVisible();
 });
 
-test('should show screenshot', async ({ runUITest, server }) => {
+test('should show screenshot', async ({ runUITest }) => {
   const { page } = await runUITest({
     'playwright.config.js': `
       module.exports = {
@@ -197,3 +197,32 @@ test('should show screenshot', async ({ runUITest, server }) => {
   await expect(page.getByText('Screenshots', { exact: true })).toBeVisible();
   await expect(page.locator('.attachment-item img')).toHaveCount(1);
 });
+
+test('should not fail on internal page logs', async ({ runUITest, server }) => {
+  const { page } = await runUITest({
+    'a.test.ts': `
+      import { test, expect } from '@playwright/test';
+      test('pass', async ({ browser }, testInfo) => {
+        const context = await browser.newContext({ storageState: { cookies: [], origins: [] } });
+        const page = await context.newPage();
+        await page.goto("${server.EMPTY_PAGE}");
+        await page.context().storageState({ path: testInfo.outputPath('storage.json') });
+      });
+    `,
+  });
+
+  await page.getByText('pass').dblclick();
+  const listItem = page.getByTestId('actions-tree').getByRole('listitem');
+
+  await expect(
+      listItem,
+      'action list'
+  ).toHaveText([
+    /Before Hooks[\d.]+m?s/,
+    /browser.newContext[\d.]+m?s/,
+    /browserContext.newPage[\d.]+m?s/,
+    /page.goto/,
+    /browserContext.storageState[\d.]+m?s/,
+    /After Hooks/,
+  ]);
+});