-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Fix the "must work properly when selecting undo by keyboard" integration test #19873
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 the "must work properly when selecting undo by keyboard" integration test #19873
Conversation
/botio integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/ddeaea8ebfad9c0/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/cd691b464aceb48/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/cd691b464aceb48/output.txt Total script time: 12.66 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/ddeaea8ebfad9c0/output.txt Total script time: 31.33 mins
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/6128590aacb6c7c/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/36d71479ebaafda/output.txt |
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.
r=me, since this would be the correct thing to do even if there were no intermittent failures; thank you.
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/6128590aacb6c7c/output.txt Total script time: 13.39 mins
|
77b0b27
to
1853015
Compare
/botio-linux integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/c6b8a124fd72c96/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/c6b8a124fd72c96/output.txt Total script time: 12.68 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/36d71479ebaafda/output.txt Total script time: 31.90 mins
|
/botio-linux integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/71c80f779bf762d/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/71c80f779bf762d/output.txt Total script time: 13.12 mins
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/3b77295b3636250/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/4c51f88eb843aab/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/3b77295b3636250/output.txt Total script time: 12.70 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/4c51f88eb843aab/output.txt Total script time: 31.80 mins
|
1853015
to
69129cc
Compare
/botio integrationtest |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/25603350ed021bc/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/8772fb37b4f6392/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/8772fb37b4f6392/output.txt Total script time: 12.80 mins
|
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.
r=me, thank you.
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/25603350ed021bc/output.txt Total script time: 33.40 mins
|
…ion test This integration test fails intermittently because the undo button can only be activated if focus can be put on it, and that in turn can only happen if it's visible. The test tried to make sure that the undo bar is visible, but checking for the absence of the `hidden` attribute is unfortunately not enough to assert visibility according to Puppeteer documentation [1]. Moreover, the undo button wasn't checked at all. To fix the issue we let Puppeteer do the visibility detection for the undo bar by providing the `visible: true` option to `waitForSelector` [2]. This is consistent with the other tests that already do this, and also with the existing code that detects if the undo bar is hidden (which uses the `hidden: true` option of `waitForSelector`). Moreover, we wait for the undo button to be present before putting focus on it. For consistency, and to avoid intermittent failures elsewhere, we mirror this solution to the other undo bar/button tests of the various editors. [1] https://pptr.dev/api/puppeteer.elementhandle.isvisible [2] https://pptr.dev/api/puppeteer.waitforselectoroptions
69129cc
to
967e340
Compare
/botio-linux integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/68e3ab1b26983ea/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/68e3ab1b26983ea/output.txt Total script time: 13.08 mins
|
This integration test fails intermittently because the undo button can only be activated if focus can be put on it, and that in turn can only happen if it's visible. The test tried to make sure that the undo bar is visible, but checking for the absence of the
hidden
attribute is unfortunately not enough to assert visibility according to Puppeteer documentation [1]. Moreover, the undo button wasn't checked at all.To fix the issue we let Puppeteer do the visibility detection for the undo bar by providing the
visible: true
option towaitForSelector
[2]. This is consistent with the other tests that already do this, and also with the existing code that detects if the undo bar is hidden (which uses thehidden: true
option ofwaitForSelector
). Moreover, we wait for the undo button to be present before putting focus on it.For consistency, and to avoid intermittent failures elsewhere, we mirror this solution to the other undo bar/button tests of the various editors.
[1] https://pptr.dev/api/puppeteer.elementhandle.isvisible
[2] https://pptr.dev/api/puppeteer.waitforselectoroptions
Fixes #19872.