-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[WPT] Reland Upload a test for sending mouse events with key pressed #16475
Conversation
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.
Already reviewed downstream.
@KyleJu looks like we'll need to add some expectations for Safari here. I'll try pushing a commit to fix it. |
No, actually it's Chrome Dev on macOS not working. But it's not the new test that's failing, but an existing test. I've sent #16607 to see if the failure was pre-existing. |
#16607 shows that it wasn't a pre-existing error. Will have to look again. |
@LukeZielinski we've had a few changes get stuck in export like this, mostly around pointer actions where @NavidZ or @Summerlw have had a bad time. This seems like one of the least smooth parts of getting new testing capabilities added, because one has to understand the results of other browsers and manually update expectations. Do you think there's anything we can do to make this better? |
@LukeZielinski I tried and failed again, can't work out what the correct expectations are by just reading the logs of this. I see you're on the rotation next week, maybe you can help get this unstuck and identify what tooling we'd need to make this less painful? |
Yes, I plan to look into this next week. |
6b26e21
to
61ff185
Compare
So the latest failures seem to be in a different test and it's because ChromeDriver is complaining about a parameter mismatch in some action command. Trying to rebase and see if that helps. |
I actually suspect there is a bug here (either in this test, or in Chromedriver). There might be state being leaked between tests. @Summerlw are you able to try to reproduce my results below? Running tests on their own, each test passes: Running two tests together, the second test fails: Renaming |
@Hexcles does the importer still implicitly revert changes when they get old enough? This is 29 days now and I'm wondering if there's deadline for us to get it exported. (We had this situation a few years ago at least.) |
There was an issue in ChromeDriver which should be fixed by https://crrev.com/c/1626676 I've retriggered Taskcluster and hopefully it'll pass this time. |
Also thanks for the reminder, @foolip . This PR is indeed almost out of the history window (~9000 commits behind the tip of the tree, and we only keep 10,000 commits). |
Well, the continous builder hasn't caught up yet (currently at r661237 but the commit we want is r662708). I'll try again in an hour. |
Higher-level, we discussed that a TearDown function would be useful for tests to clean up after themselves, I filed #16979 to track that. |
Oh turns out we not only need to wait for ChromeDriver to build, but also Chrome Dev release (because of the version matching). Another few days then. |
More days have passed, I'll try rebasing again. |
When we have keyboard events and mouse events send together, we should set key modifies in mouse events. This test first presses some keys then send some mouse events. This test only works on Chrome. Bug: 606367 [email protected] Change-Id: Idad0bcca2c03f961f6c6af211d7f843f06e14fc1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1544170 Reviewed-by: Lan Wei <[email protected]> Commit-Queue: Lan Wei <[email protected]> Cr-Commit-Position: refs/heads/master@{#653672}
61ff185
to
81fc948
Compare
Woohoo! Looks like the tests finally passed with the newly released Chrome Dev https://tools.taskcluster.net/groups/MeIR9NGrRlqnzGZbOIkqaA/tasks/C-n9qbkmTP60Bkd2imSIuA/runs/0/logs/public%2Flogs%2Flive.log#L1296 I'll do a squash-merge. |
…16475) * [WPT] Reland Upload a test for sending mouse events with key pressed When we have keyboard events and mouse events send together, we should set key modifies in mouse events. This test first presses some keys then send some mouse events. This test only works on Chrome. Bug: 606367 [email protected] Change-Id: Idad0bcca2c03f961f6c6af211d7f843f06e14fc1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1544170 Reviewed-by: Lan Wei <[email protected]> Commit-Queue: Lan Wei <[email protected]> Cr-Commit-Position: refs/heads/master@{#653672} * Update Firefox expectations to match actual results * Expect Chrome to pass
When we have keyboard events and mouse events send together, we should
set key modifies in mouse events. This test first presses some keys
then send some mouse events. This test only works on Chrome.
Bug: 606367
TBR=[email protected]
Change-Id: Idad0bcca2c03f961f6c6af211d7f843f06e14fc1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1544170
Reviewed-by: Lan Wei <[email protected]>
Commit-Queue: Lan Wei <[email protected]>
Cr-Commit-Position: refs/heads/master@{#653672}