-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[Mouse] Remove all pointers at the end of tests #102694
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
[Mouse] Remove all pointers at the end of tests #102694
Conversation
gspencergoog
left a comment
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.
This will allow code which adds a pointer as a side effect to pass when it isn't explicitly removed. I don't think we currently have any code that does that, so it's probably OK, but we should be aware that we're trading test coding convenience for this possibility.
@goderbauer what do you think?
|
Once added, mouse devices are typically not removed until the end of the app, so "a test scenario that adds a pointer and never removes it" is actually close enough to the real case. |
|
This pull request is not suitable for automatic merging in its current state.
|
|
This pull request is not suitable for automatic merging in its current state.
|
c62f92a to
47ac713
Compare
|
This pull request is not suitable for automatic merging in its current state.
|
47ac713 to
992c2a0
Compare

This PR removes an assertion at the end of unit tests that ensures all mouse pointers have been removed. Instead, the
MouseTrackeris directly renewed without checking so.Moreover, this PR deleted all statements that remove mouse pointers at the of a test or schedule so. These changes are not really necessary, but are made nonetheless to verify that the change is safe.
This assertion (and subsequent pointer removal) was added in #32535 to solve an issue where mouse connection was leaked between tests. However this assertion doesn't feel needed for two reasons:
MouseTrackershould be enough to reset states, which has already been done.And it turns out that removing the code does not break any tests!
Moreover, this problem is also the cause for b/227634353: The developers have some tests for a web app that can be run in a real browser for debugging. And when they do, although these tests to not refer to mouse pointers at all, any movement of the real mouse will be captured by the tests, causing the test to throw for unremoved mouse pointers.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.