-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: Enable snapshots for most remaining public commands #34072
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but make sure you fix those tests before landing/
As this closes the issue for count(), having a test that checks that count() has gotten a snapshot would be nice as well. |
20e90e5
to
0ac3a8b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Test results for "tests others"1 flaky21465 passed, 503 skipped Merge workflow run. |
Test results for "tests 1"9 flaky37548 passed, 648 skipped Merge workflow run. |
Test results for "tests 2"3 fatal errors, not part of any test 115 flaky265324 passed, 10082 skipped Merge workflow run. |
Enable capturing snapshots on commands that have any visual component or interaction with the DOM. This is an interim solution to #33934. In the future we will display stale snapshots on trace entries that do not contain a snapshot.