Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update testsuite #1738

Merged
merged 1 commit into from
Oct 18, 2021
Merged

Update testsuite #1738

merged 1 commit into from
Oct 18, 2021

Conversation

sbc100
Copy link
Member

@sbc100 sbc100 commented Oct 15, 2021 •
edited
Loading

bulk-memory-operations and reference-types were completely
removed from the upstream testsuite becuase there were
merged into the upstream spec:

WebAssembly/testsuite#44

In order to land this I had to disable several spec tests
under wasm2c because it lacks support for mutli-table and
reference types. I filed #1737 to track this.

@sbc100 sbc100 requested review from ngzhian and kripken October 15, 2021 01:28
ngzhian
ngzhian reviewed Oct 15, 2021
View reviewed changes
Copy link
Member

@ngzhian ngzhian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to mark a test as FAILING rather than deleting it? Seems like we might accidentally forget to add those tests back in.

kripken
kripken reviewed Oct 15, 2021
View reviewed changes
src/c-writer.cc
@@ -1100,6 +1100,9 @@ void CWriter::WriteElemInitializers() {
}
Index elem_segment_index = 0;
for (const ElemSegment* elem_segment : module_->elem_segments) {
if (elem_segment->kind == SegmentKind::Passive) {
continue;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it might conflict with #1721 (same file altered, diff is extremely close and might overlap) so we should coordinate with @shravanrn

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is very minor. It should be pretty trivial to replicate (or deal with any merge conflicts).

@sbc100
Copy link
Member Author

sbc100 commented Oct 15, 2021

Is there a way to mark a test as FAILING rather than deleting it? Seems like we might accidentally forget to add those tests back in.

Not that I know of no.

@sbc100
Copy link
Member Author

sbc100 commented Oct 15, 2021

Is there a way to mark a test as FAILING rather than deleting it? Seems like we might accidentally forget to add those tests back in.

Not that I know of no.

Thats why I open a bug and listed all the tests I was forced to delete.

ngzhian
ngzhian approved these changes Oct 15, 2021
View reviewed changes
Copy link
Member

@ngzhian ngzhian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM since we have that bug, and also the conflict is minor. (btw any plans to keep up to date with multi table and ref types?)

@ngzhian
Copy link
Member

ngzhian commented Oct 15, 2021

Is there a way to mark a test as FAILING rather than deleting it? Seems like we might accidentally forget to add those tests back in.

Not that I know of no.

Thats why I open a bug and listed all the tests I was forced to delete.

Dk if this feature will be useful, but yea that tracking bug works, thanks!

@sbc100
Copy link
Member Author

sbc100 commented Oct 15, 2021

Is there a way to mark a test as FAILING rather than deleting it? Seems like we might accidentally forget to add those tests back in.

Not that I know of no.

Thats why I open a bug and listed all the tests I was forced to delete.

Dk if this feature will be useful, but yea that tracking bug works, thanks!

Actually I forgot to run ./test/update-spec-tests.py during this change so its not ready to land yet.

Also ./test/update-spec-tests.py recreates all the removed tests so its pretty clear which ones are missing and easy to re-create them once we add these features to wasm2c.

@sbc100 sbc100 changed the title Update testsuite [WIP] Update testsuite Oct 18, 2021
@sbc100
Update testsuite
c306630 
bulk-memory-operations and reference-types were completely
removed from the upstream testsuite becuase there were
merged into the upstream spec:

WebAssembly/testsuite#44

In order to land this I had to disable several spec tests
under wasm2c because it lacks support for mutli-table and
reference types.  I files
#1737 to track this.
@sbc100 sbc100 changed the title [WIP] Update testsuite Update testsuite Oct 18, 2021
@sbc100 sbc100 merged commit 61be173 into main Oct 18, 2021
@sbc100 sbc100 deleted the update_testsuite branch October 18, 2021 18:55
@keithw keithw mentioned this pull request Sep 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kripken kripken kripken left review comments

@ngzhian ngzhian ngzhian approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sbc100 @ngzhian @kripken