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(tracing): do not duplicate network traces #34527

Closed
wants to merge 2 commits into from

Conversation

yury-s
Copy link
Member

@yury-s yury-s commented Jan 28, 2025

Fixes #34404

@yury-s yury-s requested review from dgozman and pavelfeldman and removed request for dgozman January 28, 2025 23:32

This comment has been minimized.

@mxschmitt mxschmitt changed the title fix(tracing): do not duplicate netwrok traces fix(tracing): do not duplicate network traces Jan 29, 2025
Copy link
Contributor

Test results for "tests 1"

1 failed
❌ [webkit-page] › tests/page/page-leaks.spec.ts:161:5 › waitFor should not leak @webkit-ubuntu-22.04-node18

10 flaky ⚠️ [firefox-page] › tests/page/page-evaluate.spec.ts:403:3 › should throw for too deep reference chain @firefox-ubuntu-22.04-node18
⚠️ [playwright-test] › tests/ui-mode-trace.spec.ts:341:5 › should work behind reverse proxy @macos-latest-node18-1
⚠️ [webkit-library] › tests/library/popup.spec.ts💯3 › should inherit touch support from browser context @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:44:14 › page screenshot › should work with a mobile viewport @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:55:14 › page screenshot › should work with a mobile viewport and clip @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:66:14 › page screenshot › should work with a mobile viewport and fullPage @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:278:14 › element screenshot › should restore viewport after page screenshot and exception @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:291:14 › element screenshot › should restore viewport after page screenshot and timeout @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-leaks.spec.ts:82:5 › click should not leak @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-leaks.spec.ts:107:5 › fill should not leak @webkit-ubuntu-22.04-node18

37676 passed, 650 skipped
✔️✔️✔️

Merge workflow run.

Copy link
Member

@mxschmitt mxschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mind adding this regression test as well? #33106 (comment)

@yury-s
Copy link
Member Author

yury-s commented Feb 4, 2025

Discussed the approach and didn't like the complexity, especially because the file naming have to be consistent in at least 3 layers - server side, client side local utils, trace viewer. Will do #34616 instead to address immediate problem.

@yury-s yury-s closed this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants