[Feature]: allow to pass arbitrary location to test.step

[vitalets]

trafficstars

🚀 Feature Request

Currently test.step method binds location in file to the place of it's call. When test.step is wrapped with helper methods, it produces the same location for different steps. Suggestion is to allow to pass arbitrary location to test.step. See example below.

Example

Imagine I have a helper.ts where I define step helper for my tests:

export async function clickButton(text: string) {
  return test.step(`User clicks button "${text}"`, async () => {
    // ...
  }); 
}

Now I run the following test from ./e2e/test.ts:

test('test', async ({}) => {
  await clickButton('foo');
  await clickButton('bar');
  await clickButton('baz');
});

In the report I get all steps with the same location:

It would be great to see more meaningful locations in the report, e.g.:

User clicks button "foo"— e2e/test.ts:2
User clicks button "bar"— e2e/test.ts:3
User clicks button "baz"— e2e/test.ts:4

If I were able to provide a custom location to test.step, I would achieve it. Example:

import { Location } from '@playwright/test/reporter';
import errorStackParser from 'error-stack-parser';

export async function clickButton(text: string) {
  const location = getStepLocation();
  return test.step(`User clicks button "${text}"`, async () => {
    // ...
  }, { location }); 
}

function getStepLocation(): Location {
  const frame = errorStackParser.parse(new Error()).find((frame) => frame.fileName.endsWith('.test.ts'));
  return {
      file: frame.getFileName(),
      line: frame.getLineNumber(),
      column: frame.getColumnNumber(),
  };
}

Motivation

In general this feature provides more opportunities for writing high order step helpers.

Personally, I need it for managing playwright-bdd project. I'm wrapping test.step with Given / When / Then helpers to have correct step titles in the report:

const Given = (text: string) => test.step(`Given ${text}`, () => { ... });
const When = (text: string) => test.step(`When ${text}`, () => { ... });
const Then = (text: string) => test.step(`Then ${text}`, () => { ... });

Before Playwright 1.43 I've used private testInfo._runAsStep method to run step with a custom location. Since 1.43 testInfo._runAsStep was replaced with a more complex logic and now I need more tricky hacks to achieve the same.

[pavelfeldman]

If you use boxing in the test.step it'll give you the right locations. https://playwright.dev/docs/release-notes#hide-implementation-details-box-test-steps

[vitalets]

If you use boxing in the test.step it'll give you the right locations. https://playwright.dev/docs/release-notes#hide-implementation-details-box-test-steps

Yes, but it boxes only 1 stack frame up (I think because of this line). If I wrap my clickButton into another function, locations become incorrect:

export async function clickButton2(text: string) {
  return clickButton(text);
}

export async function clickButton(text: string) {
  return test.step(`User clicks button "${text}"`, async () => {
    // ...
  }, { box: true }); 
}

Running:

test('test', async ({ }) => {
  await clickButton2('foo');
  await clickButton2('bar');
  await clickButton2('baz');
});

Output:

[pavelfeldman]

That's because you did not box clickButton2. Any time you want to pass location to step, box it instead.

[vitalets]

That's because you did not box clickButton2. Any time you want to pass location to step, box it instead.

So I've re-written it this way:

export async function clickButton2(text: string) {
  return test.step('dummy step', () => clickButton(text), { box: true });
}

Now it works. But I need to pass this dummy step every time I need to wrap helper. I've also tried to pass empty string as a step title, but the report shows these empty steps as well:

And it creates an unnesessary step structure:

Imagine I have a bunch of helpers that re-uses each other, the report structure in that case would reflect all this nesting. Although I just want flat steps with custom locations.

I think just passing location is a more convenient option (if there no technical blockers with that solution).

[pavelfeldman]

That's something we can fix, open to making the step title optional. Marking as open to a pr.

[vitalets]

Imagine I have a bunch of helpers that re-uses each other, the report structure in that case would reflect all this nesting. Although I just want flat steps with custom locations.

@pavelfeldman will these empty-title steps keep nesting structure? Because I'm caring about this:

Imagine I have a bunch of helpers that re-uses each other, the report structure in that case would reflect all this nesting. Although I just want flat steps with custom locations.

[jan-molak]

Being able to pass a custom location would be great for Serenity/JS integration with Playwright Test, too.

@pavelfeldman - would you be open to extending test.step to accept a custom location? For example:

test.step('my step', () => { ... }, { location: { ... } })

Alternatively, a separate stepInternal or similar would also work.

[WestonThayer]

We had this implemented us the non-public TestInfo._runAsStep API, but are now working on replacing it with test.step and box: true since v1.43 removed _runAsStep. Similar to others, we'd be fine with a custom location as well, we'd already built it for the previous implementation.

[osohyun0224]

Hello! :) @vitalets I made https://github.com/microsoft/playwright/pull/32504 PR PTAL when you have time ... !

[vitalets]

Hello! :) @vitalets I made #32504 PR PTAL when you have time ... !

Thank you! Let's wait for Playwright team members to check. @pavelfeldman

[dgozman]

@vitalets @osohyun0224 @WestonThayer @jan-molak Are you generating your playwright tests? We have discussed this with the team, and feel like for generated tests the best solution would be a source map. It is an established technology that gives you plenty of control over source locations. Let me know what you think.

[vitalets]

Hi @dgozman I generate Playwright tests from BDD feature files in playwright-bdd. But I point step location to the generated files, not to the source files:

So I don't need source maps, as it's a regular Playwright tests execution.

Pointing to feature files is a nice option, but it adds complexity to the code generation process. Currently, I just output correct JavaScript code.

I think the general use case is following: There are helper functions that I use in tests and I'm sure their locations are not helpful for the report viewer. That's why I want to display the initial location in the test file. Wrapping each call into anonymous step (https://github.com/microsoft/playwright/issues/30160#issuecomment-2027362160) looks a bit awkward from the DX perspective. I'm not sure source maps can also help here, as there is no code generation involved.

Passing custom location as a step property is so straightforward. Why do you think we need another solution?

[jan-molak]

Hi @dgozman and thanks for looking into this!

Serenity/JS doesn't generate Playwright tests, so source maps would not be appropriate in our case. It would be ideal if we could specify the location instead.

To give you some more context, Serenity/JS integrates with Playwright Test and uses test.step API to make the descriptions of Serenity/JS tasks appear in Playwright Test reports, like this one:

This integration also offers a nice developer experience in VSCode with the Playwright extension:

Since, at the moment, the public test.step API does not allow for the caller to override the location, Serenity/JS Playwright Test integration has to monkey-patch the testInfo._addStep method.

The need to monkey-patch is not ideal. However, this approach allows us to inject the location where the Serenity/JS task is instantiated in the test file (or in a user-land test library built on top of Serenity/JS) instead of where the test.step is invoked somewhere inside the Serenity/JS framework. Of course, the user-land location is what developers writing Serenity/JS scenarios care about.

As per my previous suggestion, we'd love for the test.step API to allow the caller to specify the location. For example:

test.step('my step', () => { ... }, { location: { ... } })

If your team prefers to keep the test.step method as is, Playwright Test could introduce a separate integration-specific API that achieves the same objective, e.g. by allowing framework authors to indicate to Playwright Test when the "step" starts and finishes, the step title, and its location. Allowing for the location to be passed to test.step seems simpler, though.

I'd happily answer any other questions your team has and collaborate on a solution.

[dgozman]

@vitalets @jan-molak Thank you for the information. We decided to proceed with adding location to test.step, so I am reviewing #32504.

[osohyun0224]

@dgozman Sorry for the late reply. I also think it is right to implement it by adding it as an option to the existing functions of @vitalets and @jan-molak . If there are more ways to utilize your Source Map scalability method, then consider it again. First of all, I think what @vitalets and @jan-molak suggested is correct in order to implement it the way we want.

I understood @vitalets and @jan-molak opinions well, and it's really cool! 👏🏻 I will be happy to answer any other questions the Playwright team may have and work with them to find a solution.

[WestonThayer]

@dgozman apologies for the delay as well. We also are not generating tests, we have a custom fixture similar to Page optimized for accessibility testing (it has access to a screen reader, etc). It has methods analogous to Page.goto() that make sense to show up as test steps in the report, and we have similar issues where it's not always possible to keep all test.step()s at the top level so boxing works.

[vitalets]

Thank you!