Add tests for serialization with internal ids forscript.evaluate and script.callFunction.
#36041
Conversation
wpt-pr-bot
requested review from
AutomatedTester,
jgraham,
juliandescottes,
sadym-chromium,
shs96c and
whimboo
|
Chromium doesn't support internal IDs yet, and not sure when will. Can the tests be moved to something like |
| ["bar", {"type": type, "internalId": any_string}], | ||
| ] | ||
|
|
||
| recursive_compare({"type": result_type, "value": value}, result) |
There was a problem hiding this comment.
Do we really need to check the return type here? It looks like comparing the value will enough. Can simplify the test, and check only the internal IDs, without data types?
There was a problem hiding this comment.
True, the result_type is not really important here, I've removed it, but I think it is still good to check that the type is present in the value with internalId
There was a problem hiding this comment.
It can be removed from the parameters as well them
|
|
||
| if result_type == "array": | ||
| value = [ | ||
| {"type": type, "internalId": any_string}, |
There was a problem hiding this comment.
Do we want to check one of the items actually have the value?
There was a problem hiding this comment.
The value is actually not present here, since we serialize only with maxDepth=1 now
|
|
||
| @pytest.mark.asyncio | ||
| @pytest.mark.parametrize( | ||
| "return_structure, result_type", |
There was a problem hiding this comment.
result_type can be removed from here now
There was a problem hiding this comment.
but it's still used to build the value here: https://github.com/web-platform-tests/wpt/pull/36041/files#diff-609470bfe4f5bba1b3da43624062982a560723dbd8cf36af1b27cdab9a34194fR31
|
@sadym-chromium, thanks a lot for the quick review! |
|
FYI I also requested an additional test that actually checks that the generated |
The code is already reviewed in Mozilla repo.