Add lifecycle event validation to WaitForLoadState
#300
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
imiric
force-pushed
the
feat/validate-lifecycleevent
branch
from
e188a4d to
62fa217
Compare
imiric
commented
Apr 25, 2022
imiric
commented
Apr 26, 2022
imiric
commented
Apr 26, 2022
imiric
pushed a commit
that referenced
this pull request
May 9, 2022
imiric
pushed a commit
that referenced
this pull request
May 9, 2022
* Add checkHitTarget fix to release notes * Update #264 details * Mention the rest of v0.3.0 changes * Update mention of JS API docs * Mention k6 update along with other dependencies * Mention #299 fix * Mention #300 improvement * Remove leftover closing paren * Add 264 as a feature (#265) * Update 283 comment (#265) * Add executablePath option (#265) * Rephrase code refactors (#265) * Rephrase waitForFunction mention Resolves #265 (comment) * Mention the project roadmap as an improvement * Mention that raf is the default waitForFunction polling option Co-authored-by: Ivan Mirić <ivan.miric@grafana.com>
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.
Previously if the user passed an invalid lifecycle event value in
page.waitForLoadState()options, we would silently default toload, which might not have been the intention. This PR validates the value inLifecycleEvent.UnmarshalText()and fails the test with an error if it's invalid. It also reuses the validation in other parts whereLifecycleEventis used, such as inFrame.Goto,Frame.SetContentandFrame.WaitForNavigationoptions, which all used their own validation returning a less informative error.Ideally lifecycle events should be exported as JS symbols to avoid typo situations, and so that we can have type definitons for them, but this is a stopgap solution until then.
Thanks to @wardbekker for reminding us about this!