Fix typing logic in the scripting integration tests #18862
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The default `page.type()` API from Puppeteer works for text fields that only dispatch a sandbox event on e.g. focus loss (i.e. after all characters have been inserted), and for those we can also use the default typing delay from Puppeteer instead of defining our own value. However, it doesn't work correctly for text fields where every character insertion dispatches a sandbox event. This is because processing the sandbox event takes some time and Puppeteer must wait for that before it can (safely) insert the next character. This commit therefore introduces a helper function to type a given value correctly in such text fields. Not only does this fix intermittent failures if our delay was too low for sandbox processing to complete, but it also speeds up the tests by eliminating our delays in places where they were (much) higher than necessary. In total the runtime of the scripting integration test suite goes from 137 seconds before this patch to 100 seconds after this patch.
|
/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/f104e54e1feb7b8/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/075f6a3bf2f24a9/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/f104e54e1feb7b8/output.txt Total script time: 9.04 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/075f6a3bf2f24a9/output.txt Total script time: 19.08 mins
|
|
For extra confirmation, the total script time above from after this patch can be compared with the total script time from before this patch at e.g. #18860 (comment) to see the reduction in runtime. |
calixteman
approved these changes
Oct 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The default
page.type()API from Puppeteer works for text fields that only dispatch a sandbox event on e.g. focus loss (i.e. after all characters have been inserted), and for those we can also use the default typing delay from Puppeteer instead of defining our own value.However, it doesn't work correctly for text fields where every character insertion dispatches a sandbox event. This is because processing the sandbox event takes some time and Puppeteer must wait for that before it can (safely) insert the next character. This commit therefore introduces a helper function to type a given value correctly in such text fields.
Not only does this fix intermittent failures if our delay was too low for sandbox processing to complete, but it also speeds up the tests by eliminating our delays in places where they were (much) higher than necessary. In total the runtime of the scripting integration test suite goes from 137 seconds before this patch to 100 seconds after this patch.
Fixes a part of #18773 (most scripting integration tests work now, except for two which have a different issue).
Fixes a part of #18396 (the final one from the list works now, and in general removing hardcoded delays can't hurt).
I have determined for each call individually if
page.type()ortypeAndWaitForSandbox()must be used. I did that by opening the PDF file from the test in a modified viewer that logs each sandbox invocation, then entering characters in the corresponding text field and checking if sandbox events were triggered after each character insertion or not.