feat(tracing) Adding groups to trace via pw-api

[Snooz82]

This PR implements #33047 .

It shall be possible to add groups to trace logs so that also users that do not use Playwright-Test, like Playwright Python, Robot Framework, Cucumber etc., can also group their traces. Similar to test.step visually.

[github-actions[bot]]

Test results for "tests 1"

16 failed :x: [installation tests] › playwright-cli-install-should-work.spec.ts:93:5 › install playwright-chromium should work @package-installations-macos-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-chromium should work @package-installations-macos-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-firefox should work @package-installations-macos-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-webkit should work @package-installations-macos-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:58:5 › playwright-core should work @package-installations-macos-latest :x: [installation tests] › connect-to-selenium.spec.ts:20:5 › connect to selenium @package-installations-ubuntu-latest :x: [installation tests] › playwright-cli-install-should-work.spec.ts:93:5 › install playwright-chromium should work @package-installations-ubuntu-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-chromium should work @package-installations-ubuntu-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-firefox should work @package-installations-ubuntu-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-webkit should work @package-installations-ubuntu-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:58:5 › playwright-core should work @package-installations-ubuntu-latest :x: [installation tests] › playwright-cli-install-should-work.spec.ts:93:5 › install playwright-chromium should work @package-installations-windows-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-chromium should work @package-installations-windows-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-firefox should work @package-installations-windows-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:22:7 › playwright-webkit should work @package-installations-windows-latest :x: [installation tests] › playwright-packages-install-behavior.spec.ts:58:5 › playwright-core should work @package-installations-windows-latest

4 flaky

:warning: [playwright-test] › ui-mode-test-setup.spec.ts:98:5 › should show errors in config @macos-latest-node18-1
:warning: [installation tests] › playwright-electron-should-work.spec.ts:31:5 › electron should work with special characters in path @package-installations-macos-latest
:warning: [chromium] › components/splitView.spec.tsx:35:5 › should render sidebar first @web-components-web
:warning: [webkit-library] › library/browsercontext-clearcookies.spec.ts:116:3 › should remove cookies by path @webkit-ubuntu-22.04-node18

27011 passed, 431 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[github-actions[bot]]

Test results for "tests 1"

35865 passed, 620 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[Snooz82]

@microsoft-github-policy-service agree

[github-actions[bot]]

Test results for "tests 1"

2 flaky

:warning: [playwright-test] › ui-mode-test-setup.spec.ts:22:5 › should run global setup and teardown @ubuntu-latest-node18-1
:warning: [webkit-library] › library/browsercontext-pages.spec.ts:82:3 › should click the button with offset with page scale @webkit-ubuntu-22.04-node18

34572 passed, 615 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[Snooz82]

@dgozman Hi Dmitry, Thanks for your feedback. Yes i will of course write some test cases. With the hints you provided, i should be able to do them.

Update: Test written

[github-actions[bot]]

Test results for "tests 1"

2 failed :x: [playwright-test] › runner.spec.ts:118:5 › should ignore subprocess creation error because of SIGINT @macos-latest-node18-1 :x: [playwright-test] › fixture-errors.spec.ts:471:5 › should not give enough time for second fixture teardown after timeout @windows-latest-node18-1

3 flaky

:warning: [firefox-page] › page/page-event-request.spec.ts:169:3 › should return response body when Cross-Origin-Opener-Policy is set @firefox-ubuntu-22.04-node18
:warning: [installation tests] › playwright-electron-should-work.spec.ts:31:5 › electron should work with special characters in path @package-installations-macos-latest
:warning: [chromium-library] › library/trace-viewer.spec.ts:674:1 › should handle src=blob @ubuntu-20.04-chromium-tip-of-tree

35866 passed, 620 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[github-actions[bot]]

Test results for "tests 1"

2 fatal errors, not part of any test 1 failed :x: [playwright-test] › playwright.ct-dev-server.spec.ts:21:5 › should run dev-server and use it for tests @macos-latest-node18-1

1 flaky

:warning: [webkit-library] › library/screenshot.spec.ts:42:14 › page screenshot › should work with a mobile viewport @webkit-ubuntu-22.04-node18

35944 passed, 621 skipped, 1 did not run :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[Snooz82]

PS: It seems to me that you also have some flaky tests?

Is it possible to give the users that do PRs the right, to re-run failed actions?

[Snooz82]

@pavelfeldman @dgozman Would be really appreciated if someone could check my questions and give feedback what is missing. This week i have still some time to do fixes or requested changes.

We would really love to see that in next Playwright Release, so that we could implement it in Robot Framework Browser as well.

Thank you.

[dgozman]

@Snooz82 Sorry, I didn't get to this PR today, but I will tomorrow.

• The Firefox bot will recover if you rebase your PR.
• Re: protocol - yes, I was only talking about the internal client-server protocol, not the public API.
• I still think that you'd need to follow my comment about location and _wrapApiCall, but I will be able to apply and try things myself tomorrow.

[github-actions[bot]]

Test results for "tests 1"

36001 passed, 624 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[Snooz82]

@dgozman Thanks for the feedback.

• [x] fixed internal protocol to have location directly and not typed, but inline definition
• [x] improved docs (I do not know how to link to test.step from api docs.)

You may review again. In my testing the metadata.location worked absolutely flawless but if you want to change to _wrapApiCall go ahead.

There is one limitation though. Traces generated by Playwright Test do not correctly show the groups. They interfere with test.step but also without, they are not correctly handled in the actions tree. If i remove the test.trace and rename the 0-trace.trace etc. so that it is a normal trace, all groups work perfectly. I added therefore a hint to the docs. Imho this limitation is mostly irrelevant, because Playwright Test users should use test.step anyway and Playwright Python, Java, C# and Robot Framework will not use Playwright Test at all.

Thanks for reviewing! We are pretty exited about this coming into the core! ❤️

[github-actions[bot]]

Test results for "tests 1"

2 flaky

:warning: [chromium] › components/splitView.spec.tsx:35:5 › should render sidebar first @web-components-web
:warning: [webkit-library] › library/screenshot.spec.ts:75:14 › page screenshot › should work with device scale factor @webkit-ubuntu-22.04-node18

34714 passed, 603 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[github-actions[bot]]

Test results for "tests 1"

2 fatal errors, not part of any test 1 failed :x: [webkit-library] › library/video.spec.ts:381:5 › screencast › should capture navigation @webkit-ubuntu-22.04-node18

1 flaky

:warning: [chromium] › components/splitView.spec.tsx:35:5 › should render sidebar first @web-components-web

35976 passed, 623 skipped, 24 did not run :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[github-actions[bot]]

Test results for "tests 1"

1 flaky

:warning: [webkit-library] › library/browsertype-connect.spec.ts:172:5 › run-server › should ignore page.pause when headed @webkit-ubuntu-22.04-node18

36000 passed, 624 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[Snooz82]

Hi @dgozman,

I am sorry that i might be annoying. But if there is anything that i need to do, i could do it at the weekend. Otherwise, if you want to take over and improve it to a mergeable state, that is also fine.

Thank you.

[github-actions[bot]]

Test results for "tests 1"

6 failed :x: [chromium-library] › library/trace-viewer.spec.ts:114:1 › should show groups as tree in trace viewer @chromium-ubuntu-22.04-node18 :x: [chromium-library] › library/trace-viewer.spec.ts:114:1 › should show groups as tree in trace viewer @chromium-ubuntu-22.04-node20 :x: [chromium-library] › library/trace-viewer.spec.ts:114:1 › should show groups as tree in trace viewer @chromium-ubuntu-22.04-node22 :x: [firefox-library] › library/trace-viewer.spec.ts:114:1 › should show groups as tree in trace viewer @firefox-ubuntu-22.04-node18 :x: [chromium-library] › library/trace-viewer.spec.ts:114:1 › should show groups as tree in trace viewer @ubuntu-20.04-chromium-tip-of-tree :x: [webkit-library] › library/trace-viewer.spec.ts:114:1 › should show groups as tree in trace viewer @webkit-ubuntu-22.04-node18

4 flaky

:warning: [chromium] › components/splitView.spec.tsx:77:5 › drag resize @web-components-web
:warning: [webkit-library] › library/browsercontext-pages.spec.ts:82:3 › should click the button with offset with page scale @webkit-ubuntu-22.04-node18
:warning: [webkit-library] › library/screenshot.spec.ts:94:14 › page screenshot › should work with device scale factor and scale:css @webkit-ubuntu-22.04-node18
:warning: [playwright-test] › ui-mode-test-watch.spec.ts:145:5 › should watch all @windows-latest-node18-1

36395 passed, 639 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[github-actions[bot]]

Test results for "tests 1"

3 flaky

:warning: [firefox-library] › library/inspector/cli-codegen-3.spec.ts:171:7 › cli codegen › should generate frame locators (4) @firefox-ubuntu-22.04-node18
:warning: [webkit-library] › library/screenshot.spec.ts:42:14 › page screenshot › should work with a mobile viewport @webkit-ubuntu-22.04-node18
:warning: [webkit-library] › library/screenshot.spec.ts:64:14 › page screenshot › should work with a mobile viewport and fullPage @webkit-ubuntu-22.04-node18

36402 passed, 639 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[Snooz82]

@dgozman

All requested changes are done.

Thanks

[github-actions[bot]]

Test results for "tests 1"

36447 passed, 639 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

[Skn0tt]

Hey folks! Chiming in a bit late here. I'm working on a change in a related area so this is interesting to see.

I'm wondering if it'd be better to call this "steps" instead of "groups"? As you're saying, this is pretty similar to steps in how it works and what it does. So we could reuse the term and make it less confusing to users. We could also make the API similar and use Tracing.step(body: Callback) instead of step and stepEnd.

@snooz82 What are your thoughts on this? Did you consider calling it "steps"? What term do you prefer, and why?

[Snooz82]

@Snooz82 What are your thoughts on this? Did you consider calling it "steps"? What term do you prefer, and why?

Hi @Skn0tt my initial idea was to call it tracing.startStep & tracing.stopStep but @pavelfeldman proposed group in the original issue #33047 and i was fine wirh that. my idea was to be more similar to start, startChunk and stopChunk. therefor the startStep would fit into that. i also think startGroup and stopGroup would make sense.

just step and stepEnd would imho make no sense, because it is neither similar to current methods of tracing, nor would it be same as console api in normal JS.

actually i think now that grouping is a more generic word than step. and i would now prefer:

tracing.startGroup() tracing.stopGroup()

but to be honest, i do not really care. 😉 whatever you decide i take it. Why? Because the Robot Framework users will anyway never use that feature it will be automatically activated by the Playwright based library they are using. therefor i do not have any strong feelings here.

i would just hope that this feature could be finalized sooner than later. 🙂

[Skn0tt]

Sounds good! Seeing the discussion in https://github.com/microsoft/playwright/issues/33047, it looks like the naming has already been considered well :) Taking back my point then.

[Snooz82]

@dgozman Hi Dmitry,

may i ask if i can do anything to accelerate this PR? i think it would be cool if at least Playwright Python would have some time to adapt this feature until the release?

best regards René

[Skn0tt]

Dmitry is currently out of office, so expect to wait a couple more days.

[Snooz82]

🥳