From e65ac2bb9cbef4a94bed1f5b685ff7787b870b63 Mon Sep 17 00:00:00 2001
From: Yury Semikhatsky <yurys@chromium.org>
Date: Thu, 1 Aug 2024 11:23:30 -0700
Subject: [PATCH] fix(trace): do not place expect into unfinished api calls
 based on time

Fixes https://github.com/microsoft/playwright/issues/31959
---
 packages/playwright/src/worker/testInfo.ts    |  4 +-
 .../playwright-test/playwright.trace.spec.ts  | 66 +++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts
index 1e9e1f9c46..e55619e9c6 100644
--- a/packages/playwright/src/worker/testInfo.ts
+++ b/packages/playwright/src/worker/testInfo.ts
@@ -33,7 +33,7 @@ export interface TestStepInternal {
   complete(result: { error?: Error, attachments?: Attachment[] }): void;
   stepId: string;
   title: string;
-  category: 'hook' | 'fixture' | 'test.step' | 'expect' | string;
+  category: 'hook' | 'fixture' | 'test.step' | 'expect' | 'pw:api' | string;
   location?: Location;
   boxedStack?: StackFrame[];
   steps: TestStepInternal[];
@@ -252,7 +252,7 @@ export class TestInfoImpl implements TestInfo {
       parentStep = this._findLastStageStep();
     } else {
       parentStep = zones.zoneData<TestStepInternal>('stepZone');
-      if (!parentStep && data.category !== 'test.step') {
+      if (!parentStep && data.category === 'pw:api') {
         // API steps (but not test.step calls) can be nested by time, instead of by stack.
         // However, do not nest chains of route.continue by checking the title.
         parentStep = this._findLastNonFinishedStep(step => step.title !== data.title);
diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts
index 9a692245f0..f2eaec1742 100644
--- a/tests/playwright-test/playwright.trace.spec.ts
+++ b/tests/playwright-test/playwright.trace.spec.ts
@@ -1178,3 +1178,69 @@ test('should record trace for manually created context in a failed test', async
   // Check console events to make sure that library trace is recorded.
   expect(trace.events).toContainEqual(expect.objectContaining({ type: 'console', text: 'from the page' }));
 });
+
+test('expect during fetch', async ({ page, server }) => {
+  server.setRoute('/index', (req, res) => {
+    console.log('index');
+    res.writeHead(200, { 'Content-Type': 'text/html' });
+    res.end(`<script>fetch('/api')</script><div>Hello!</div>`);
+  });
+  server.setRoute('/hang', async (req, res) => {
+    console.log('Hanging request');
+  });
+  await page.route('**/api', async route => {
+    const response = await route.fetch({ url: server.PREFIX + '/hang' });
+    await route.fulfill({ response });
+  });
+  await page.goto(server.PREFIX + '/index');
+  await expect(page.getByText('Hello!')).toBeVisible();
+  await page.unrouteAll({ behavior: 'ignoreErrors' });
+});
+
+test('should not nest top level expect into unfinished api calls ', {
+  annotation: { type: 'issue', description: 'https://github.com/microsoft/playwright/issues/31959'}
+}, async ({ runInlineTest, server }) => {
+  server.setRoute('/index', (req, res) => {
+    res.writeHead(200, { 'Content-Type': 'text/html' });
+    res.end(`<script>fetch('/api')</script><div>Hello!</div>`);
+  });
+  server.setRoute('/hang', () => {});
+  const result = await runInlineTest({
+    'a.spec.ts': `
+      import { test, expect } from '@playwright/test';
+      test('pass', async ({ page }) => {
+        await page.route('**/api', async route => {
+          const response = await route.fetch({ url: '${server.PREFIX}/hang' });
+          await route.fulfill({ response });
+        });
+        await page.goto('${server.PREFIX}/index');
+        await expect(page.getByText('Hello!')).toBeVisible();
+        await page.unrouteAll({ behavior: 'ignoreErrors' });
+      });
+    `,
+  }, { trace: 'on' });
+  expect(result.exitCode).toBe(0);
+  expect(result.failed).toBe(0);
+
+  const tracePath = test.info().outputPath('test-results', 'a-pass', 'trace.zip');
+  const trace = await parseTrace(tracePath);
+  expect(trace.actionTree).toEqual([
+    'Before Hooks',
+    '  fixture: browser',
+    '    browserType.launch',
+    '  fixture: context',
+    '    browser.newContext',
+    '  fixture: page',
+    '    browserContext.newPage',
+    'page.route',
+    'page.goto',
+    '  route.fetch',
+    '    page.unrouteAll',
+    'expect.toBeVisible',
+    'After Hooks',
+    '  fixture: page',
+    '  fixture: context',
+  ]);
+});
+
+