-
Notifications
You must be signed in to change notification settings - Fork 51
Remove duplicate (broken) definition of assert_ArrayBuffer
#235
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
base: main-legacy
Are you sure you want to change the base?
Conversation
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.
|
Alternatively to this targeted fix we could "just" rebase this repository, I guess... |
|
@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. |
|
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. |
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. |
|
So is anything blocking the |
|
My plan is to do this rename tomorrow after the threads portion of the CG meeting, unless the presentation reveals any unexpected complications. |
|
Done here #237 |
|
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 |
|
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. |
This was removed from the upstream test in
WebAssembly/spec@cc2d59b.
The
assert_ArrayBufferhelper is now defined in theassertions.jsfile. The version there does not have the problem withself.This problem was uncovered when importing the threads JS tests into V8, see https://crbug.com/449581914.