Skip to content

Commit

Permalink
feat(test runner): allow to pass arbitrary location to test.step (#32504
Browse files Browse the repository at this point in the history
)

Fixes #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 #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>
  • Loading branch information
osohyun0224 authored Sep 17, 2024
1 parent 507e515 commit 8761daf
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 4 deletions.
5 changes: 5 additions & 0 deletions docs/src/test-api/class-test.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions packages/playwright/src/common/testType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright/types/test.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
*
Expand Down
39 changes: 39 additions & 0 deletions tests/playwright-test/test-step.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
`);
});
2 changes: 1 addition & 1 deletion utils/generate_types/overrides-test.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 8761daf

Please sign in to comment.