Update script result_ownership tests to handle id is only on root #35966
Conversation
wpt-pr-bot
requested review from
AutomatedTester,
jgraham,
sadym-chromium,
shs96c and
whimboo
|
The question is do we want to restrict the protocol extensions. And having the nested handlers can be considered as such an extension. |
|
|
||
| # Regardless of the result_ownership, check that the nested object has no | ||
| # handle property. | ||
| assert result["value"] == [["a", {"type": "object"}]] |
There was a problem hiding this comment.
I wonder if we should better update assert_handle to recursively check that the handle is only part of the root object?
|
CC'ing @jgraham as well. |
I imagine this a question about the spec rather than about the test modification? Should we file another issue for that? Otherwise I'm not sure when we need to consider extensions when writing tests? |
@sadym-chromium I will file a separate issue for this topic, could we land this change? |
|
|
||
| # Regardless of the result_ownership, check that the nested object has no | ||
| # handle property. | ||
| assert result["value"] == [["a", {"type": "object"}]] |
There was a problem hiding this comment.
I assume that we want to use assert_handle() here as well.
juliandescottes
force-pushed
the
result-ownership-nested
branch
from
ea62471
to
6ecf77d
Compare
|
Note: I just rebased for now, will apply comments afterwards. |
juliandescottes
force-pushed
the
result-ownership-nested
branch
from
6ecf77d
to
9c8053d
Compare
|
@whimboo I applied the comments, can you take a look? |
There was a problem hiding this comment.
That looks much better! @sadym-chromium do you want to have another look again given it has been refactored?
|
Thanks for the review! I can see I asked for the task to be re executed. |
This task is always failing those days and maybe it needs to be removed if no-one from Google could get it fixed. @sadym-chromium, @foolip any idea what's wrong with it? |
|
@jgraham could you please merge this PR? Thanks. |
Per serialization spec, child ownership is always set to "none" before serializing values. This means that only the root can ever have a handle id set.
Our current tests for resultOwnership="root" assert that a handle is set on the root remote value, but not that nested remote values do not have a handle id.