-
Notifications
You must be signed in to change notification settings - Fork 4.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
E2E testing: Use JSON serialization for getAllBlocks #31199
Conversation
Size Change: -4 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
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.
Looks good! Thanks!
The PR is scoped down to only affect I wonder what others think, should we add this to |
I'd agree with your intuition here. I only see
I can also imagine others running into the same problem in the future, so I'd vote for making the change to |
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.
The current changes look good to me too.
|
I think we can restrict this to We should not forget that both are public APIs and are used outside of GB as well. Since there are no created issues for either of them, IMO it's okay to land the only needed change for |
That makes sense 👍 . In that case, could we leave more explicit documentation somewhere? It seems easy to overlook the fact that |
Agreed! We can do this in a follow up. I'll merge this though now, as is needed for #30804 Thanks for all the work here! |
Description
page.evaluate
used inwpDataSelect
, returnsundefined
when we return a non-serializable value from the evaluation. Non-serializable values arefunction
,DOM element
etc.Reference:
https://pptr.dev/#?product=Puppeteer&version=v9.0.0&show=api-pageevaluatepagefunction-args
For a real world example, we store
validationIssues
in theblock-editor
store for each block. When we have a validation issue, we store the ref and owner (DOM element handles). Those are non-serializable so the evaluation returns withundefined
. Which is causing the tests to fail.As a fix, we turn the result into a JSON string, then we parse it. JSON replaces all unserializable values with
null
, so the evaluation can return safely.Reference:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify
This PR ensures we are returning an object that is serializable by Puppeteer.
How has this been tested?
Tests are passing
Types of changes
E2E tests consistency
Checklist:
*.native.js
files for terms that need renaming or removal).