Skip to content

Conversation

@backes
Copy link
Member

@backes backes commented Oct 6, 2025

This was removed from the upstream test in
WebAssembly/spec@cc2d59b.

The assert_ArrayBuffer helper is now defined in the assertions.js file. The version there does not have the problem with self.

This problem was uncovered when importing the threads JS tests into V8, see https://crbug.com/449581914.

This was removed from the upstream test in
WebAssembly/spec@cc2d59b.

The `assert_ArrayBuffer` helper is now defined in the `assertions.js`
file. The version there does not have the problem with `self`.

This problem was uncovered when importing the threads JS tests into V8,
see https://crbug.com/449581914.
@backes
Copy link
Member Author

backes commented Oct 6, 2025

Alternatively to this targeted fix we could "just" rebase this repository, I guess...

@conrad-watt
Copy link
Collaborator

@backes is this fix already present in the https://github.com/WebAssembly/threads/tree/upstream-rebuild of this repository? That branch contains a rebase. If so, I should probably rename that branch to "main", which I've intended to do for a while but never got around to.

@backes
Copy link
Member Author

backes commented Oct 7, 2025

Ah, I wasn't aware of that branch. Yes, indeed, all tests pass on that branch. Great!

But that made me notice that the threads proposal does not seem to add any JS-API tests. Can that be true, or do I miss anything?

How I noticed: In V8 when we import tests from proposals, we skip tests that are either identical to the main spec test, or unchanged since the branch of the proposal (determined by looking at the git merge-base). With these checks, we didn't add any tests from the threads proposal.

@conrad-watt
Copy link
Collaborator

But that made me notice that the threads proposal does not seem to add any JS-API tests. Can that be true, or do I miss anything?

Currently these parts of the spec are mainly exercised by the extraction of the Wast tests to the JS harness, enabled by #229 in the upstream-rebuild branch. We should write some manual JS API tests.

@backes
Copy link
Member Author

backes commented Oct 9, 2025

Oh, nice, so after #229 we can even import the spec tests into V8! That would be awesome.

But yes, we should definitely add a few JS API tests to catch stuff like #234.

@backes
Copy link
Member Author

backes commented Oct 9, 2025

So is anything blocking the upstream-rebuild branch from becoming main?
If no, then I would wait for that to happen, and then we can import the updated tests into V8.
If yes, then we might consider switching to the rebuild-upstream branch for now, and importing the tests from there.

@conrad-watt
Copy link
Collaborator

My plan is to do this rename tomorrow after the threads portion of the CG meeting, unless the presentation reveals any unexpected complications.

@conrad-watt
Copy link
Collaborator

Done here #237

@backes
Copy link
Member Author

backes commented Oct 30, 2025

Thanks!

I tried importing that new main branch now, and as expected this does not import any JS tests (because the proposal does not define any).

Trying to import the core tests (after #229 added support for translating the thread construct to JS) leads to multiple errors, see #229 (comment). I am not sure if those tests were ever run in a JS shell. It's very possible that I am doing something wrong there.

@conrad-watt
Copy link
Collaborator

I think most of the testing was done in the browser rather than through a headless shell, so there may be some quirks to iron out. I'll keep most of the follow-ups in the other issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants