Skip to content

Add a new style of test suite for the witx tooling similar to *.wast #392

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

Merged
merged 7 commits into from
Feb 11, 2021

Conversation

alexcrichton
Copy link
Contributor

@alexcrichton alexcrichton commented Feb 10, 2021

These commits add a new *.witxt style of test suite to be similar to the *.wast style of test suites. (supposed to be shorthand for "witx test")

Most existing tests have been converted to the *.witxt style of test as an example. There's a few goals from this translation:

  • In general adding a few lines to a *.witxt file is quite easy and low-overhead. The intention is to make the barrier-to-entry on adding test extremely low.
  • Avoid API churn. By having a separate textual format for tests it helps us reduce churn on the tests themselves if the APIs of the witx crate change.
  • As the functionality of witx grows and expands this is intended to make it easier to add new classes of test suites. We'll in theory already have a lot of witx documents in the test suite already and are used to writing inline witx documents, so adding future functionality should be much easier to test by either reusing existing test directives or adding a small handful of new directives.

For example one thing I'd like to add stricter tests for on #391 is the elaboration of parsed types to the variant/record types. By using assert_representation I could reuse those definitions and existing type checking to assert this by adding a new text file here. Additionally I'd like to start codifying more of the translation of a witx signature to the expected ABI, and I believe that will be much easier with a test suite like this.

Another thought on this style of test suite is that as *.witx usage grows beyond simply WASI we'll want the ability to test more and more pieces of functionality that aren't necessarily used by WASI itself, which means it'll get more important to have strict and exhaustive tests outside of just the *.witx files in this repository for WASI itself.

@pchickey
Copy link
Contributor

pchickey commented Feb 10, 2021

I love this!

Copy link
Contributor

@pchickey pchickey left a comment

Choose a reason for hiding this comment

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

Lets release this as witx 0.9?

As the functionality of `witx` grows this will hopefully make adding
tests for new functionality as well as new kinds of tests easier. The
goal is to make it very easy to drop a test file with various directives
to exercise the functionality of `witx` and its internals.

For now the testsuite is quite simple, simply asserting whether
documents are either valid or invalid. My hope, though, is that this can
be expanded over time with more styles of assertions directives.
@alexcrichton
Copy link
Contributor Author

Ok with #391 merged I've rebased this on master to get all the tests passing again with adjusted syntax. This actually helped uncover a few bugs here and there!

Unless there's an anything urgent I'd prefer to not have a new release just yet. My hope is to next start implementing conveniences for code generators (e.g. witx-bindgen in Rust, wiggle in Wasmtime, the C header generator in wasi-libc, ...) to largely not have to worry about these changes. This is still a work in progress but it'd be nice if they didn't have to update twice or land in the middle by accident.

@alexcrichton
Copy link
Contributor Author

I think this should be good to merge now though!

@pchickey pchickey merged commit 9d79e58 into WebAssembly:main Feb 11, 2021
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