Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(playwright-test): test.step supports passing step params to trace view #32704

Closed
wants to merge 1 commit into from

Conversation

jan-molak
Copy link
Contributor

Extending the functionality requested in #30160 and #32680 and building on #32504 and #32687, this pull request enables the test.step API to accept an optional params object, to be included in the trace viewer.

This enables test automation frameworks that integrate with Playwright Test, such as Serenity/JS, to add extra context to Playwright reports.

Example usage:

// helper.ts
import { test, TestType } from '@playwright/test';

export async function sayHello(name: string) {
  await test.step('says hello', () => {}, { params: { name } });
}
// playwright.config.ts
module.exports = {
  reporter: './reporter',
  use: {
    trace: 'on',
  }
};
// a.test.ts
import { test } from '@playwright/test';
import { sayHello } from './helper';

test('params', async () => {
  await sayHello('World');
});

Example result (using Serenity/JS Playwright example):
Screenshot 2024-09-19 at 11 27 51

CC: @dgozman, @mxschmitt, @vitalets, @osohyun0224, @WestonThayer

@jan-molak
Copy link
Contributor Author

@microsoft-github-policy-service agree

Copy link
Contributor

Test results for "tests 1"

2 flaky ⚠️ [firefox-library] › library/inspector/cli-codegen-2.spec.ts:407:7 › cli codegen › click should emit events in order
⚠️ [playwright-test] › ui-mode-test-setup.spec.ts:22:5 › should run global setup and teardown

35525 passed, 661 skipped
✔️✔️✔️

Merge workflow run.

@pavelfeldman
Copy link
Member

I'm not sure I like the change. It fits the steps infrastructure nicely with the params being passed directly into the trace, but at the same time it is unclear why it is bypassing the reporters API. Could you give me a better idea on a couple of typical use cases for this?

@jan-molak
Copy link
Contributor Author

jan-molak commented Oct 7, 2024

Hi @pavelfeldman and thanks for reviewing the PR.

Could you give me a better idea on a couple of typical use cases for this?

Sure thing. Trace viewer is very useful in debugging complex test scenarios as it provides information about all the low-level Playwright API calls, network requests, and so on. However, I am not aware of a way to make it report the higher-level abstractions alongside the low-level calls.

For example, consider a test scenario that requires setting up some test data before making Playwright interact with the UI. Let's say we're testing a product search functionality and we need to inject product-related data into the system:

const products = [
  { name: 'apples', price: '£2.50', .... },
  { name: 'oranges', .. }, 
  // etc.
]

Let's also assume that setting up each product is itself quite involved and requires several REST API calls.

We could model a function to set up a test product using test.step, which allows us to abstract the lower-level API calls:

describe('Search', () => {

  it('find products based on name', ({ page, request }) => {

    for (const product of products) {
      await createTestProduct(product, request)
    }

    await page.goto('...')
    // enter search term, find the product, and so on
  
  })
})

async function createTestProduct(product: Product, request): Promise<void> {
  await test.step('create test product', async () => {
    await request... // create product
    await request... // update product details
    await request... // add product to search categories
    // ...
  });
}

The limitation here is that even though the report will now show several calls to 'create test product', it won't show us the exact details of the product object in a single location (yes, we could deduce the injected data from the details of lower-level requests, but that makes it more inconvenient than having it in one place).

It would be great if instead we could tell Playwright what the "high-level" parameter was, for example:

async function createTestProduct(product: Product, request): Promise<void> {
  await test.step('create test product', async () => {
    await request... // create product
    await request... // update product details
    await request... // add product to search categories
    // ...
  }, { params: { product } });
}

While this example is pretty basic, I'm sure you can see the challenge if, instead of apples and oranges, we wanted to set up several test customers in a banking system, each customer with one or more accounts, each account with an initial balance, overdraft limits and so on.

Being able to make Playwright aware of those higher-level parameters would make the report even more useful in such complex scenarios.

I hope this helps. Please let me know if you have any concerns or if there's a different way available in Playwright to support this use case.

@pavelfeldman
Copy link
Member

We could invest in improving the rendering of attachments:

import { test } from '@playwright/test';

const products = [
  { name: 'apples', price: '£2.50' },
  { name: 'oranges', price: '$2' },
];

test.describe('Search', () => {
  test('find products based on name', async ({}) => {
    for (const product of products)
      await createTestProduct(product);
  });
});

async function createTestProduct(product): Promise<void> {
  await test.step('create test product', async () => {
    // ...
    await test.info().attach('product', { body: JSON.stringify(product, undefined, 2), contentType: 'text/plain' });
  });
}

that way information would get both into the report and the trace.

@vitalets
Copy link
Contributor

vitalets commented Oct 8, 2024

Linking a similar issue for attributing steps with attachments data: #32748

Some points from previous discussions, that are important for attachments + steps:

  1. Attachments can't be reliably mapped to parallel steps (see #29323):

    We lack expressiveness of the API to reliably map attachments to steps. If steps run concurrently, we don't know which step to insert the attachment into without changing the step API and passing the attachment sink in it.

  2. Attachments are not available in onStepEnd in merge-reports mode.

@pavelfeldman
Copy link
Member

These are good points, but to me they speak in favor of prioritizing the step attachment fixes! Closing this PR, please file an issue where we could track it. I'm supportive of fixing attachments for steps on the API level.

@jan-molak
Copy link
Contributor Author

jan-molak commented Oct 8, 2024

@pavelfeldman @vitalets - would injecting the testInfo object into test.step address the issue with parallel steps?

I think an interface like this could be quite neat (and avoid the problem with the global object returned by test.info()):

async function createTestProduct(product): Promise<void> {
  await test.step('create test product', async ({ testInfo }) => {
    // ...
    await testInfo.attach('product', { body: JSON.stringify(product, undefined, 2), contentType: 'text/plain' });
  });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants