Workarena/Base: Merge init scripts before evaluation.#83
Open
conte91 wants to merge 3 commits intoServiceNow:mainfrom
Open
Workarena/Base: Merge init scripts before evaluation.#83conte91 wants to merge 3 commits intoServiceNow:mainfrom
conte91 wants to merge 3 commits intoServiceNow:mainfrom
Conversation
younik
reviewed
Jul 5, 2025
Comment on lines
155
to
157
| page.evaluate(init_script_js) | ||
| for f in page.frames: | ||
| f.evaluate(init_script_js) |
There was a problem hiding this comment.
I think the evaluate lines should be removed
conte91
changed the title
Author
|
Self-note: this could be an issue in case names defined across scripts shadow each other? Something like: I think we should be fine though, because even if two scripts' private functions will share the same name, the last one defined will win. |
remove evaluate
younik
approved these changes
Jul 16, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes #57 and duplicates without the need for downgrading the playwright version.
Playwright's init scripts are not executed sequentially, as per PW's docs:
However, some benchmarks depend on all installed init scripts to be executed in order, which is why the page hangs if they don't (in WorkArena's case, this was because of the function being waited on being undefined - since the script defining it hadn't been evaluated yet).
It shouldn't cause any problem to just concatenate all scripts together, so that we can guarantee FIFO execution order.