From 8761dafc7365a1340f247472ef9c2c7e36458a15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=EC=98=A4=EC=86=8C=ED=98=84?=
 <53892427+osohyun0224@users.noreply.github.com>
Date: Wed, 18 Sep 2024 00:11:21 +0900
Subject: [PATCH] feat(test runner): allow to pass arbitrary location to
 test.step (#32504)

Fixes https://github.com/microsoft/playwright/issues/30160

### Description:
This pull request introduces the ability to specify custom locations for
test steps in Playwright. By enabling the provision of arbitrary
locations to the test.step method, it resolves the limitation where
helper methods obfuscate the original call site, providing more accurate
and meaningful location data in test reports.

### Motivation:
To enhance the utility and clarity of test reports in Playwright.
Specifically, it addresses the need to trace test steps back to their
precise location in the code, which is especially important when steps
are abstracted in helper functions. This feature is crucial for
maintaining accurate documentation and facilitating debugging processes.

### Changes:
Added functionality to pass a custom location object to test.step.

### Expected Outcome:
This PR is expected to significantly improve the precision and
usefulness of diagnostic data in test reports by allowing specific
locations within helper functions to be accurately documented. It
facilitates better tracking of test executions and simplifies the
debugging process, making it easier for developers to understand and
address issues within complex tests.

### References:
Closes https://github.com/microsoft/playwright/issues/30160 -
"[Feature]: allow to pass arbitrary location to test.step"

**Code Check**
I conducted tests on this new feature by integrating it into some
existing test codes, and it worked well. I will attach the code used for
testing and a screenshot showing the successful outcome.

<details>
<summary>toggle dropdown</summary>
<div markdown="1">

```
import type { Location } from '../../../packages/playwright/types/testReporter'
...
test('should respect the back button', async ({ page }) => {
    await page.locator('.todo-list li .toggle').nth(1).check();
    await checkNumberOfCompletedTodosInLocalStorage(page, 1);
...
    await test.step('Showing active items', async () => {
      await page.getByRole('link', { name: 'Active' }).click();
    }, {location});
```

<img width="1109" alt="image"
src="https://github.com/user-attachments/assets/359feafa-0949-4c71-9426-46debef21bdd">
</div>
</details>
---
 docs/src/test-api/class-test.md            |  5 +++
 packages/playwright/src/common/testType.ts |  4 +--
 packages/playwright/types/test.d.ts        |  2 +-
 tests/playwright-test/test-step.spec.ts    | 39 ++++++++++++++++++++++
 utils/generate_types/overrides-test.d.ts   |  2 +-
 5 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/docs/src/test-api/class-test.md b/docs/src/test-api/class-test.md
index 9b56fbf2e6462..b6ae7d1522116 100644
--- a/docs/src/test-api/class-test.md
+++ b/docs/src/test-api/class-test.md
@@ -1710,6 +1710,11 @@ Step body.
 
 Whether to box the step in the report. Defaults to `false`. When the step is boxed, errors thrown from the step internals point to the step call site. See below for more details.
 
+### option: Test.step.location
+* since: v1.48
+- `location` <[Location]>
+Specifies a custom location for the step to be shown in test reports. By default, location of the [`method: Test.step`] call is shown.
+
 ## method: Test.use
 * since: v1.10
 
diff --git a/packages/playwright/src/common/testType.ts b/packages/playwright/src/common/testType.ts
index 5c7850a3df07f..f0882735dc7e2 100644
--- a/packages/playwright/src/common/testType.ts
+++ b/packages/playwright/src/common/testType.ts
@@ -259,11 +259,11 @@ export class TestTypeImpl {
     suite._use.push({ fixtures, location });
   }
 
-  async _step<T>(title: string, body: () => Promise<T>, options: { box?: boolean } = {}): Promise<T> {
+  async _step<T>(title: string, body: () => Promise<T>, options: {box?: boolean, location?: Location } = {}): Promise<T> {
     const testInfo = currentTestInfo();
     if (!testInfo)
       throw new Error(`test.step() can only be called from a test`);
-    const step = testInfo._addStep({ category: 'test.step', title, box: options.box });
+    const step = testInfo._addStep({ category: 'test.step', title, location: options.location, box: options.box });
     return await zones.run('stepZone', step, async () => {
       try {
         const result = await body();
diff --git a/packages/playwright/types/test.d.ts b/packages/playwright/types/test.d.ts
index e17a43843c55d..51a6720a2ecdd 100644
--- a/packages/playwright/types/test.d.ts
+++ b/packages/playwright/types/test.d.ts
@@ -4703,7 +4703,7 @@ export interface TestType<TestArgs extends KeyValue, WorkerArgs extends KeyValue
    * @param body Step body.
    * @param options
    */
-  step<T>(title: string, body: () => T | Promise<T>, options?: { box?: boolean }): Promise<T>;
+  step<T>(title: string, body: () => T | Promise<T>, options?: { box?: boolean, location?: Location }): Promise<T>;
   /**
    * `expect` function can be used to create test assertions. Read more about [test assertions](https://playwright.dev/docs/test-assertions).
    *
diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts
index d14bccd98b786..ad3478b112b12 100644
--- a/tests/playwright-test/test-step.spec.ts
+++ b/tests/playwright-test/test-step.spec.ts
@@ -1246,3 +1246,42 @@ fixture   |  fixture: page
 fixture   |  fixture: context
 `);
 });
+
+test('test location to test.step', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'reporter.ts': stepIndentReporter,
+    'helper.ts': `
+      import { test } from '@playwright/test';
+
+      export async function dummyStep(test, title, action, location) {
+        return await test.step(title, action, { location });
+      }
+
+      export function getCustomLocation() {
+        return { file: 'dummy-file.ts', line: 123, column: 45 };
+      }
+    `,
+    'playwright.config.ts': `
+      module.exports = {
+        reporter: './reporter',
+      };
+    `,
+    'a.test.ts': `
+      import { test } from '@playwright/test';
+      import { dummyStep, getCustomLocation } from './helper';
+
+      test('custom location test', async () => {
+        const location = getCustomLocation();
+        await dummyStep(test, 'Perform a dummy step', async () => {
+        }, location);
+      });
+    `
+  }, { reporter: '', workers: 1 });
+
+  expect(result.exitCode).toBe(0);
+  expect(stripAnsi(result.output)).toBe(`
+hook      |Before Hooks
+test.step |Perform a dummy step @ dummy-file.ts:123
+hook      |After Hooks
+`);
+});
\ No newline at end of file
diff --git a/utils/generate_types/overrides-test.d.ts b/utils/generate_types/overrides-test.d.ts
index 90ef7fa75a52c..be1fa7ee373d3 100644
--- a/utils/generate_types/overrides-test.d.ts
+++ b/utils/generate_types/overrides-test.d.ts
@@ -128,7 +128,7 @@ export interface TestType<TestArgs extends KeyValue, WorkerArgs extends KeyValue
   afterAll(inner: (args: TestArgs & WorkerArgs, testInfo: TestInfo) => Promise<any> | any): void;
   afterAll(title: string, inner: (args: TestArgs & WorkerArgs, testInfo: TestInfo) => Promise<any> | any): void;
   use(fixtures: Fixtures<{}, {}, TestArgs, WorkerArgs>): void;
-  step<T>(title: string, body: () => T | Promise<T>, options?: { box?: boolean }): Promise<T>;
+  step<T>(title: string, body: () => T | Promise<T>, options?: { box?: boolean, location?: Location }): Promise<T>;
   expect: Expect<{}>;
   extend<T extends KeyValue, W extends KeyValue = {}>(fixtures: Fixtures<T, W, TestArgs, WorkerArgs>): TestType<TestArgs & T, WorkerArgs & W>;
   info(): TestInfo;