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(ui): clear action selection when executing test #34678

Closed
wants to merge 1 commit into from

Conversation

agg23
Copy link
Contributor

@agg23 agg23 commented Feb 7, 2025

Fixes #34482. When executing a test with an existing selection in UI Mode, the action selection follows the test execution, up until completion or the first error, as it does on initial execution (without a selection).

This also fixes an issue where selections were preserved between tests, causing strange things to be highlighted.

@agg23 agg23 requested a review from pavelfeldman February 7, 2025 20:22
Copy link
Contributor

github-actions bot commented Feb 7, 2025

Test results for "tests 1"

6 failed
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:115:5 › should preserve action list selection upon live trace update @macos-latest-node18-1
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:115:5 › should preserve action list selection upon live trace update @ubuntu-latest-node18-1
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:115:5 › should preserve action list selection upon live trace update @ubuntu-latest-node20-1
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:115:5 › should preserve action list selection upon live trace update @ubuntu-latest-node22-1
❌ [webkit-library] › tests/library/inspector/title.spec.ts:35:5 › should update primary page URL when original primary closes @webkit-ubuntu-22.04-node18
❌ [playwright-test] › tests/ui-mode-test-progress.spec.ts:115:5 › should preserve action list selection upon live trace update @windows-latest-node18-1

15 flaky ⚠️ [chromium-page] › tests/page/page-event-request.spec.ts:151:3 › should report navigation requests and responses handled by service worker with routing @chromium-ubuntu-22.04-node20
⚠️ [firefox-page] › tests/page/page-evaluate.spec.ts:403:3 › should throw for too deep reference chain @firefox-ubuntu-22.04-node18
⚠️ [firefox-page] › tests/page/page-event-request.spec.ts:182:3 › should return response body when Cross-Origin-Opener-Policy is set @firefox-ubuntu-22.04-node18
⚠️ [installation tests] › tests/playwright-electron-should-work.spec.ts:21:5 › electron should work @package-installations-macos-latest
⚠️ [playwright-test] › tests/ui-mode-trace.spec.ts:341:5 › should work behind reverse proxy @ubuntu-latest-node20-1
⚠️ [webkit-library] › tests/library/inspector/cli-codegen-1.spec.ts:116:7 › cli codegen › should click after same-document navigation @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/inspector/cli-codegen-csharp.spec.ts:227:7 › should work with --save-har and --save-har-glob in nunit @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/proxy.spec.ts:93:11 › should proxy local network requests › by default › loopback address @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/trace-viewer.spec.ts:149:1 › should open simple trace viewer @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/trace-viewer.spec.ts:1551:1 › should not leak recorders @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/video.spec.ts:475:5 › screencast › should scale frames down to the requested size @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
⚠️ [webkit-page] › tests/page/page-leaks.spec.ts:161:5 › waitFor should not leak @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-screenshot.spec.ts:345:5 › page screenshot › should work while navigating @webkit-ubuntu-22.04-node18

37813 passed, 655 skipped
✔️✔️✔️

Merge workflow run.

setSelectedTime(undefined);
setRevealedError(undefined);
}, [model]);
}, [model, setSelectedAction]);
Copy link
Member

Choose a reason for hiding this comment

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

I think this will break the live mode entirely.

@agg23
Copy link
Contributor Author

agg23 commented Feb 19, 2025

Multiple team members have argued that it's intentional to preserve selection. Closing for now.

@agg23 agg23 closed this Feb 19, 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
Development

Successfully merging this pull request may close these issues.

[Feature]: When running a test again do not stop at previously selected action
2 participants