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

fix: match client step and server action by explicit stepId #30641

Merged
merged 3 commits into from May 14, 2024

Conversation

yury-s
Copy link
Member

@yury-s yury-s commented May 3, 2024

Matching bu apiName@wallTime fails when two actions start at the same time, e.g. two parallel api requests. Moreover, it results in trace actions that have parent set to themselves, which in turn causes infinite loop in the trace viewer. To avoid this problems we write stepId explicitly to the library trace and use those step ids to find corresponding test runner steps.

The stepId is passed via zone in case of expect, because the protocol step is quite deep in the call chain after or explicitly in case of API call steps created by the test runner instrumentation.

This comment has been minimized.

@yury-s yury-s marked this pull request as ready for review May 3, 2024 18:36

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@pavelfeldman
Copy link
Member

pavelfeldman commented May 3, 2024

Is there a user report for this? If you want to move off wallTime as an action identifier in the trace, we can do that, but that'll be a bigger change with some wallTime cleanups. Let's not land this.

@yury-s yury-s force-pushed the step-id-in-trace branch 2 times, most recently from f7b32db to 391a3e6 Compare May 11, 2024 02:07

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

Test results for "tests 1"

1 flaky ⚠️ [playwright-test] › ui-mode-test-progress.spec.ts:22:5 › should update trace live

27337 passed, 662 skipped
✔️✔️✔️

Merge workflow run.

@yury-s yury-s merged commit fb319e6 into microsoft:main May 14, 2024
30 checks passed
@yury-s yury-s deleted the step-id-in-trace branch May 14, 2024 19:10
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.

None yet

3 participants