Tests for structuredClone

[surma]

Edit: This PR got side-tracked into refactoring the testing setup for algorithms that use structured clonging. The PR for adding tests for structuredClone (whatwg/html#3414) now lives in #11069.

---

This is in connection to whatwg/html#3414.

Do not merge. This still requires interest/agreement from other browser vendors.

[surma]

I’m not sure to what extend I need to test the functionality of structuredClone considering that HTML’s structuredSerialize and structuredDeserialize are probably tested through other APIs already.

What’s the procedure here?

[w3c-bots]

Build PASSED

Started: 2018-03-21 15:18:58
Finished: 2018-03-21 15:27:28

Failing Jobs

• build_css

View more information about this build on:

• WPT PR Status
• TravisCI

[annevk]

Ideally we find existing postMessage()/history state/etc. tests and generalize those to run on both methods. Note that even if methods share a primitive per the specification, you cannot trust implementations not to take shortcuts or do the wrong thing (they might also have more legitimate reasons to diverge for performance and such, and end up making a mistake).

[surma]

Note to self: Look at tests below and create set of tests that can be imported into other API tests

• html/browsers/history/the-history-interface
• html/infrastructure/safe-passing-of-structured-data
• workers/semantics/structured-clone

Corner-cases:

• postMessage(Symbol(), "*", [new ArrayBuffer(2)]) throwing and not detaching the buffer

[surma]

I’m back from vacation and continuing now to work on this.

The plan in the most recent commit is to create a battery of tests for the structured clone algorithm that can be reused by all APIs that use the algorithm.

Before I start porting a large number of tests and including this setup everywhere, I wanted to get a review if I am doing something stupid or re-inventing the wheel.

PTAL :)

[annevk]

You want to remove the css/tools/w3ctestlib changes. It seems that at some point this should handle transferables as well?

I wonder a bit what all the infrastructure is needed for, but I'm guessing you have a plan of sorts.

@jeremyroman @bakulf @wanderview might have some thoughts on a good framework here as well.

[jeremyroman]

If this setup function completes asynchronously, it is not guaranteed to happen before the page finishes loading. If promise_test doesn't run by then, I don't think the test will run correctly. (Not super experienced with WPT; let me know if there's some sneaky way this is safe.)

Instead, you could have each promise_test await the setup promise. (Redundantly awaiting the setup doesn't seem problematic.)

I'd suggest structuredCloneBatteryOfTests being a non-async function to reduce the risk of unexpectedly awaiting before test registration in general. The only interesting waiting that happens here seems to be between waiting for all tests to finish and tearing down.

[surma]

TIL!

I like your suggestion, will implement it that way.

[jeremyroman]

Since this appears to be the only interesting awaiting here, we could make the outer function non-async and just do:

Promise.all(allTests).then(() => runner.teardown());

[surma]

I wonder a bit what all the infrastructure is needed for, but I'm guessing you have a plan of sorts.

@annevk I might very well be over-engineering. But considering I have to accommodate for workers to be set up and would like this setup to be future proof as well, it seems like a decent middle ground. Definitely open to other suggestions.

[surma]

It seems that at some point this should handle transferables as well?

Yes, that will be added once I get there 😄 Wanna ensure this rig is acceptable to y’all first.

[surma]

I pulled in all structured clone-specific bits of @zcorpan’s worker test-suite and made it reusable.
This should test pretty much every kind of data type.

I still need to write tests for the transferable bit, tho.

[zcorpan]

@surma the original tests do assertions in the worker and then pass the object back and do assertions again. I think this skips the middle part, right? Is that what we want?

It seems your new-dedicated.html test runs much better than the original, so maybe we can iterate here instead of #10820... :)

[bakulf]

Maybe it can help to share what we have in gecko. For gecko tests (mochitests) I wrote this test:
https://searchfox.org/mozilla-central/source/dom/base/test/test_postMessages.html
maybe some parts of this can be taken and converted to a WPTs.

Here the idea is to have separate tests for the different 'contexts' (for instance BroadcastChannel, window-to-window, workers, etc) but with a common core where clonable an transferable objects are tested.

[zcorpan]

Thanks @bakulf. I think that's the case with @surma's setup as well, right?

@surma can you remove the structuredClone test (for now) and add shared worker test, and remove the old tests? Then we can create a new PR for structuredClone (and window-to-window etc).

I see that you fixed createImageBitmap to work when it's returning a promise, excellent 👍

[surma]

@surma can you remove the structuredClone test (for now) and add shared worker test,

Can do.

and remove the old tests?

What exactly do you mean?

Then we can create a new PR for structuredClone (and window-to-window etc).

My plan was to add a separate “battery of tests” that involve transferables. Should we punt on that for a separate PR as well?

(Also I’m currently busy with Google I/O, so won’t be able to actually do anything until after next week)

[zcorpan]

I mean removing workers/semantics/structured-clone/dedicated.html and workers/semantics/structured-clone/shared.html and their dependencies that are no longer used by your test.

I'm not in a rush, but would be nice to land something that fixes the old tests as they're currently disabled.

[zcorpan]

@surma friendly ping :)

[surma]

Sorry for the delay. Now back from I/O, planning on working on this tomorrow :)

[surma]

• Removed mock structuredClone() test
• Added test for SharedWorker
• Added test infrastructure for structured cloning tests with transferables
• Removed old DedicatedWorker and SharedWorker test

There’s only 2 very minimal tests for transferables, but I‘d prefer to finally land this and augment it in a separate PR (potentially with the help of others).

@zcorpan @annevk PTAL :)

[zcorpan]

Thanks!

[zcorpan]

Can you squash and rewrite the commit message to reflect the current state?

[surma]

@zcorpan done

[surma]

🎉