Skip to content

Enable all features in wasm-shell assert failure tests#2254

Merged
aheejin merged 1 commit intoWebAssembly:masterfrom
aheejin:enable_feature
Jul 26, 2019
Merged

Enable all features in wasm-shell assert failure tests#2254
aheejin merged 1 commit intoWebAssembly:masterfrom
aheejin:enable_feature

Conversation

@aheejin
Copy link
Copy Markdown
Member

@aheejin aheejin commented Jul 24, 2019

If we don't enable features in assertion failure tests, new feature
tests fail not because they are malformed but because they have
unsupported features. It's hard to add tests because existing
assert_invalid tests were already failing because they have
unsupported features.

If we don't enable features in assertion failure tests, new feature
tests fail not because they are malformed but because they have
unsupported features. It's hard to add tests because existing
`assert_invalid` tests were already failing because they have
unsupported features.
@aheejin aheejin requested a review from kripken July 24, 2019 13:47
@kripken
Copy link
Copy Markdown
Member

kripken commented Jul 24, 2019

How does the spec test suite handle this? Do they also assume all features are always present (no tests for a feature not being present)?

If they also assume that then lgtm.

@aheejin
Copy link
Copy Markdown
Member Author

aheejin commented Jul 24, 2019

I searched and couldn't find any tests that tests for feature enabling/disabling. It is not even possible currently because wasm-shell does not take --enable-* or --disable-* set of feature options.

@aheejin
Copy link
Copy Markdown
Member Author

aheejin commented Jul 26, 2019

I guess it's ok to merge?

@aheejin aheejin merged commit 6ca3f4c into WebAssembly:master Jul 26, 2019
@aheejin aheejin deleted the enable_feature branch July 26, 2019 03:29
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