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.

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

WASI testsuite #9

Open
wanderer opened this issue Apr 7, 2019 · 52 comments
Open

WASI testsuite #9

wanderer opened this issue Apr 7, 2019 · 52 comments
Labels
proposal A discussion about a specific actionable item tests

Comments

@wanderer
Copy link

wanderer commented Apr 7, 2019

It would be nice to have a standard testsuite for the WASI API

@mohanson
Copy link

Is there an official testsuit currently available? Even very simple.

@tschneidereit
Copy link
Member

There's a testsuite for the wasi-sdk, which is perhaps the closest we have right now.

I'm not sure it'd make sense to do something more directly for WASI itself (as opposed to the libc abstraction on top of it, which wasi-sdk uses) right now: once WASI is defined in terms of Interface Types[1], I think the tests would probably have to look somewhat different.

[1]: see the last paragraph in the design principle doc's section on security.

@RReverser
Copy link
Member

I'm not sure it'd make sense to do something more directly for WASI itself (as opposed to the libc abstraction on top of it, which wasi-sdk uses)

I didn't realise these are separate; but yeah, the testsuite of wasi-sdk looks pretty comprehensive. Do you think it's something that could be extracted into a separate submodule that other implementations could rely on / contribute to independently from wasi-sdk implementation?

@RReverser
Copy link
Member

RReverser commented Jun 29, 2020

Maybe I rushed with "comprehensive" bit; looking through the suite, it doesn't seem to test most of filesystem operations, for example, or randomness, or timers.

https://github.com/devsnek/node-wasi/tree/master/test/c by @devsnek, referenced from one of related issues above, seems better in this regard.

@devsnek
Copy link
Member

devsnek commented Jun 29, 2020

We've got more tests now: https://github.com/nodejs/node/tree/master/test/wasi/c

@mohanson
Copy link

This is exactly what I want to express, there are many wasm vm implementations in the world, and they all have their own wasi test cases, which seems inconsistent.

I think we can write a unified testsuit, these testcases are not so platform-dependent, and which can ensure that the data structure, function signature and function logic are correct.

Some existing test cases are too complicated, I mean "not so much like unit tests". For example, when I want to testing fd_readdir, the case imports too many other wasi functions like:

  (import "wasi_unstable" "proc_exit" (func $__wasi_proc_exit (type 2)))
  (import "wasi_unstable" "fd_readdir" (func $__wasi_fd_readdir (type 3)))
  (import "wasi_unstable" "fd_close" (func $__wasi_fd_close (type 4)))
  (import "wasi_unstable" "fd_fdstat_get" (func $__wasi_fd_fdstat_get (type 5)))
  (import "wasi_unstable" "path_open" (func $__wasi_path_open (type 6)))
  (import "wasi_unstable" "args_sizes_get" (func $__wasi_args_sizes_get (type 5)))
  (import "wasi_unstable" "args_get" (func $__wasi_args_get (type 5)))
  (import "wasi_unstable" "fd_prestat_get" (func $__wasi_fd_prestat_get (type 5)))
  (import "wasi_unstable" "fd_prestat_dir_name" (func $__wasi_fd_prestat_dir_name (type 0)))
  (import "wasi_unstable" "fd_seek" (func $__wasi_fd_seek (type 7)))
  (import "wasi_unstable" "fd_write" (func $__wasi_fd_write (type 8)))

@caspervonb
Copy link
Contributor

caspervonb commented Jun 29, 2020

I have some tests for Deno's std/wasi implementation here and there are some legacy tests for libc in the pre-merge repository I haven't brought over; overall fairly similar coverage to @devsnek's tests, looks like I go a bit overboard on explicitly testing stdio 🤔 .

Ultimately I'd like to have a bunch of tests in the WebAssembly text format (for sharing between toolchains) that test the snapshots directly rather than relying fully on integration tests.

@sunfishcode
Copy link
Member

Tests written in wat are great because they easily work in many engines and don't require users to have various compilers for various languages installed in order to run them. So if people have tests of this form and/or are interesting in writing such tests, it'd be great to collect them.

That said though, tests written in wat directly, and targeting WASI APIs directly, are often very verbose and difficult to maintain, so it'd be good to collect compiled tests as well. Ideally we'd have the original Rust/C/etc. source files in the repository, so that people with the appropriate toolchains installed can edit and debug, and also the compiled output in the repository, in either wat or wasm format, so that it's possible for others to run the tests without having the toolchains installed.

@sbc100
Copy link
Member

sbc100 commented Jun 29, 2020

I agree with @sunfishcode .. I don't think we should require that tests be authored in wat. Writing them in C or rust and providing supplying the compiled output in wat/wasm format should be fine.

@RReverser
Copy link
Member

Sounds like merging Node & Deno tests and precompiling to WAT alongside existing C files would be a good start then. @devsnek & @caspervonb do you want to uh talk to each other? :)

@binji
Copy link
Member

binji commented Jun 29, 2020

That said though, tests written in wat directly, and targeting WASI APIs directly, are often very verbose and difficult to maintain, so it'd be good to collect compiled tests as well.

Another alternative could be to generate wat files from another language, JS/Python. We've done that for a few proposals now, see https://github.com/WebAssembly/bulk-memory-operations/tree/master/test/meta and https://github.com/WebAssembly/simd/tree/master/test/core/simd/meta as examples.

@devsnek
Copy link
Member

devsnek commented Jun 29, 2020

I was working on a test generator (written in JS) to that effect, and I came to the conclusion that it's simpler to just write the tests in rust/c/wat/etc and keep the compiled versions in the source tree.

@binji
Copy link
Member

binji commented Jun 29, 2020

@devsnek that makes sense. It's a bit sad because those examples go from ~300 byte C files to ~30k wasm files. But I can definitely understand that the burden of writing in wat with or without a generator may outweigh this slight debugging cost.

@sbc100
Copy link
Member

sbc100 commented Jun 29, 2020

Sounds like merging Node & Deno tests and precompiling to WAT alongside existing C files would be a good start then. @devsnek & @caspervonb do you want to uh talk to each other? :)

Don't forget the wasi-sdk tests (https://github.com/WebAssembly/wasi-sdk/tree/master/tests).. (i've not looked at how they compare to the node or deno tests but just wanted to make sure they don't get fogotten).

@RReverser
Copy link
Member

Don't forget the wasi-sdk tests (WebAssembly/wasi-sdk:tests@master).. (i've not looked at how they compare to the node or deno tests but just wanted to make sure they don't get fogotten).

I've mentioned them above:

Maybe I rushed with "comprehensive" bit; looking through the suite, it doesn't seem to test most of filesystem operations, for example, or randomness, or timers.

There are couple of useful tests, but mostly things seem to be better covered by other, more comprehensive, testsuites.

@caspervonb
Copy link
Contributor

Started extracting Deno's test suite here

@sbc100
Copy link
Member

sbc100 commented Jul 10, 2020

Personally I would rather see the testsuite written in C, since the syscall API being tested is so low level. It seems like a better match to me. I also imagine the resulting wat/wasm files from the tests would be smaller and closer the source code that way (maybe I'm wrong about that?). But I'm also probably showing my bias not being a rust person myself.

Do you have some reason to prefer rust over C for this task?

@devsnek
Copy link
Member

devsnek commented Jul 10, 2020

I think we need a mix of raw wat/wasm files and output from popular compilers, to verify both the theoretical conformance of an implementation and its ability to handle real world programs.

@sbc100
Copy link
Member

sbc100 commented Jul 10, 2020

Looking at the tests, it also looks like they are not actually written against the wasi API at all, but rather against some higher liker std API which kind of obscures exactly the syscall usage. Its seems to me that it would make sense to write the tests directly against the syscall layer that they are testing, no? These tests also seem to be testing the whole of the rust libc layer.

@sbc100
Copy link
Member

sbc100 commented Jul 10, 2020

@devsnek, sure, I guess that makes sense.

I do think its important to have a set of low level unit tests though that are written directly against the wasi API. The C language seems like the sweet spot for authoring those.

If we have higher level test in addition that is fine with me.

@caspervonb
Copy link
Contributor

caspervonb commented Jul 10, 2020

Personally I would rather see the testsuite written in C, since the syscall API being tested is so low level. It seems like a better match to me. I also imagine the resulting wat/wasm files from the tests would be smaller and closer the source code that way (maybe I'm wrong about that?). But I'm also probably showing my bias not being a rust person myself.

Just happened to be the way we went forward given Deno's CI setup, it is fairly tedious to get at certain syscalls, the libc crate does not even expose errno 🤔

Plan now is to clean up the old C tests from deno-wasi and merge them in, the extra rust coverage doesn't hurt, at-least we know that rustc and rust's std works as expected and they run in a second (1.5s to be exact on my machine). Same goes for C++ new std::filesystem APIs, doesn't hurt to have some coverage.

Directory structure is meant to reflect this approach, rs for rust, c for c, etc.

Looking at the tests, it also looks like they are not actually written against the wasi API at all, but rather against some higher liker std API which kind of obscures exactly the syscall usage. Its seems to me that it would make sense to write the tests directly against the syscall layer that they are testing, no? These tests also seem to be testing the whole of the rust libc layer.

Most are still fairly 1:1 in implementation but yeah need more coverage in general.
But actually rust std calls into wasi directly, at-least in the places i've looked.

@sbc100
Copy link
Member

sbc100 commented Jul 10, 2020

Having just looked at the wasi-sdk tests again I realize they are also written against wasi-libc higher level APIs, not directly against wasi APIs :(

@RReverser
Copy link
Member

RReverser commented Jul 10, 2020

Personally I would rather see the testsuite written in C, since the syscall API being tested is so low level.

FWIW Rust is equally low-level as a language, and there's nothing you can express in C but not in Rust. I do agree though that testing syscalls directly would be better than stdlib APIs (regardless of the language tests are written in).

When using low-level APIs directly, the resulting binary sizes should be ~same across languages as well.

@devsnek
Copy link
Member

devsnek commented Jul 10, 2020

one could theoretically target wasm32-unknown-unknown with this kind of code:

extern "C" {
  fn wasi_whatever() {}
}
fn main() {
  assert!(wasi_whatever(invalid) == wasi_some_error)
}

I just want to make sure we're both testing both that the individual wasi functions behave correctly (across ranges of valid and invalid inputs, etc., basically unit testing) and that we have test cases of them being used together (output from rust/c++ std, etc., basically integration testing)

@RReverser
Copy link
Member

@devsnek Yeah, that's basically what I meant (note that you'll need unsafe on main though for FFI bindings). I think we can even use wasi crate which already provides autogenerated low-level bindings for those APIs.

I don't have anything against them written in C either, just wanted to respond to the comment above and clarify that language doesn't really matter here, as long as we avoid stdlib/libc and go for low-level APIs.

@caspervonb
Copy link
Contributor

caspervonb commented Jul 12, 2020

Merged most of the C tests from https://github.com/caspervonb/deno-wasi into https://github.com/caspervonb/wasi-test and threw together a quick test runner.

Most pass but runtimes are in disagreement about a couple of tests.

Screenshot 2020-07-12 at 8 17 36 AM

Adding Deno and Node to this runner later, Wasmtime and Wasmer just happened to have nearly identical command line interfaces so they got in first.

Any other major runtimes that makes sense to be included in this kind of compatibility check? Still expecting runtimes to bring their own test runner but will eventually generate some sort of compat table on the CI from this.

@sbc100
Copy link
Member

sbc100 commented Jul 14, 2020

Where do we think the final version of this testsuite should live? I'm guessing in sub-directory within the wasi repo itself, or in a new wasi-testsuite repo?

@caspervonb
Copy link
Contributor

caspervonb commented Jul 14, 2020

Where do we think the final version of this testsuite should live? I'm guessing in sub-directory within the wasi repo itself, or in a new wasi-testsuite repo?

Should be as frictionless as possible to keep it as a submodule or subtree so I'd vote for a standalone repository.

@binji
Copy link
Member

binji commented Jul 14, 2020

Should be as frictionless as possible to keep it as a submodule or subtree so I'd vote for a standalone repository.

That's nice for integrating into other projects, but there's also value in having it synchronized with spec changes. You could always keep a separate repo that is copied from the main repo too (like we do with WebAssembly/testsuite). I don't have a preference really, just thought I'd mention it.

@sbc100
Copy link
Member

sbc100 commented Jul 14, 2020

Right, we need to weigh the balance of those two use cases:

  • On one side how much impact would
    getting the whole wasi repo be for a user of the testsuite (curreently not much at all its tiny).
  • On the other side, how hard would it be to make a spec changes that also include changes to the testsuite if they live in different repos?

@pchickey
Copy link
Contributor

@caspervonb thank you for leading the way on creating a test suite! I agree it is badly needed and the discussion here has been excellent.

The WebAssembly/WASI repository itself is pretty lightweight and doesn't have a ton of churn, so I'd support having the test suite live in this repository as well.

@RReverser
Copy link
Member

I like the @binji's idea here the most - if we can easily automate a mirror in a separate repo, that seems like best of both worlds.

@devsnek
Copy link
Member

devsnek commented Jul 15, 2020

@caspervonb would you mind reporting bugs you found at https://github.com/nodejs/node/issues/?

how hard would it be to make a spec changes that also include changes to the testsuite if they live in different repos?

For ECMAScript, we make spec changes in https://github.com/tc39/ecma262, and test changes in https://github.com/tc39/test262, and it hasn't been a big issue afaik.

@caspervonb
Copy link
Contributor

Thank you for leading the way on creating a test suite! I agree it is badly needed and the discussion here has been excellent.

That reminds me, should track lucet compatibility as-well 😉

That's nice for integrating into other projects, but there's also value in having it synchronized with spec changes. You could always keep a separate repo that is copied from the main repo too (like we do with WebAssembly/testsuite). I don't have a preference really, just thought I'd mention it.

So, for WebAssembly this makes a lot of sense since there's a reference implementation.
.
For an ABI like this I'm not sure it matters because snapshots are immutable?
Integration tests should just always work and unit tests are tied to a specific immutable ABI.

Actually when I think about it I'm a bit unsure about what the new proposals scheme and modularisation mean for future versions of the ABI, are we still going to be snapshotting?

Would you mind reporting bugs you found at https://github.com/nodejs/node/issues/?

Yep working down my way down the list of current failures...

@pchickey
Copy link
Contributor

pchickey commented Jul 16, 2020

Lucet and Wasmtime use the same wasi-common under the hood so just wasmtime is sufficient to track.

Snapshots are intended to be immutable. However, the way in which we specify them does change sometimes - we have some fuzzy plans to change the way the witx language works as part of the evolution towards interface types, and will change the witx documents for the snapshots accordingly.

I've reconsidered and I believe it is likely that the test suite will need to contain tools that refer to the witx documents in the spec repo (even if transitively) - e.g. the rust wasi crate both refers to the spec via a submodule relationship. So, we could avoid having a circular dependency on the spec repo by putting spec tests in a separate repo.

@caspervonb
Copy link
Contributor

caspervonb commented Aug 2, 2020

Been a bit under the weather so progress has been a bit slow but still working on this.

I've reconsidered and I believe it is likely that the test suite will need to contain tools that refer to the witx documents in the spec repo (even if transitively) - e.g. the rust wasi crate both refers to the spec via a submodule relationship. So, we could avoid having a circular dependency on the spec repo by putting spec tests in a separate repo.

So had a bit of churn and things have changed, I've embraced Cargo for this instead of an ad-hoc build script and brought in the wasi crate as a dependency for testing against the ABI (landing soon'ish).

So yes, as of a few commits ago this would be an indirect circular dependency if it went into the spec repo.

@caspervonb
Copy link
Contributor

caspervonb commented Aug 6, 2020

Landed a bunch of miscellaneous tests against the ABI yesterday, it's by no means exhaustive or complete coverage but it's a start on testing WASI directly.

I'd also like to welcome pull requests at this point 😉

@caspervonb
Copy link
Contributor

caspervonb commented Aug 28, 2020

So a little update, the test suite has moved here for the time being so it's not on my personal account and contributors can be added (hint hint).

We've merged the prebuilt test-suite into Deno which is working out great and I just published a little compatibility table over at https://khronosproject.github.io/wasi-compat-table.

I saw @sunfishcode mentioned this to the agenda of yesterday's meeting as-well. Anything I should be in the loop about?

@RReverser
Copy link
Member

@caspervonb Is the plan still to have .wasm files committed to the repo? I'm not sure everyone would want to rebuild the .rs files.

@caspervonb
Copy link
Contributor

Yep @RReverser!

Prebuilt is at https://github.com/khronosproject/wasi-test-suite which is what Deno pulls in as a submodule.

@RReverser
Copy link
Member

Ah great, thanks! To double-check - does this testsuite already include Node.js tests as well or only yours? So far I've been using the ones copied from Node.js in my WASI implementation, I wonder if I can just switch to this new repo instead, or need to combine both.

@caspervonb
Copy link
Contributor

To double-check - does this testsuite already include Node.js tests as well or only yours?

Node's C tests are not included at the moment

I did have my initial batch of C tests for a while but diverged in favor of doing direct tests against wasi syscalls which is still a work in progress.

For libc i've been thinking its better to just bring in, or adapt libc-test than rolling our ad-hoc set of tests for it.
If it's good enough for musl it should be good enough for us.

TLDR; for a little while it may be beneficial to combine both; because deno/wasi is a sandbox within deno's sandbox conditions that would result in ENOTCAPABLE tests in particular are a bit lacking.

@RReverser
Copy link
Member

Ah, right, that makes sense. Perhaps we could at least make sure that equivalent syscalls / arguments are covered in the new repo, then the Node's tests could migrate fairly easily?

@sunfishcode
Copy link
Member

(Catching up here, appologies for being slow).

I have an experimental adaptation of musl's libc-test that I've tested WASI libc with here, in case it's useful: https://github.com/CraneStation/wasi-libc-test. I wrote my own harness instead of using the provided Makefile, which may or may not be the best approach, but it made it easier to customize things. It hasn't moved into the WebAssembly organization yet because libc-test contains code with a variety of licenses, which I expect would require approval.

@caspervonb
Copy link
Contributor

caspervonb commented Sep 10, 2020

Perhaps we could at least make sure that equivalent syscalls / arguments are covered in the new repo, then the Node's tests could migrate fairly easily?

Went through Node's tests and currently wasi-test-suite is lacking equivalents of these tests:

  • cant_dotdot.c
  • freopen.c
  • poll.c
  • escape_symlink.c

With the exception of poll they more or less equate to one assertion so I'll try to get those taken care of today; if not today, really soon (Deno release tomorrow and I have things pending merge 😉).

@caspervonb
Copy link
Contributor

caspervonb commented Sep 10, 2020

I have an experimental adaptation of musl's libc-test that I've tested WASI libc with here, in case it's useful: https://github.com/CraneStation/wasi-libc-test. I wrote my own harness instead of using the provided Makefile, which may or may not be the best approach, but it made it easier to customize things. It hasn't moved into the WebAssembly organization yet because libc-test contains code with a variety of licenses, which I expect would require approval.

Good to know; I'll take a proper look once I've got full coverage for direct wasi syscall tests 👍

The way I'm envisioning this is that straight ABI tests should be the source of truth; where-as libc, libc++ and libstd (as in rust libstd) tests are nice to have optional smoke tests.

@sbc100
Copy link
Member

sbc100 commented Nov 5, 2020

I started work integrating https://github.com/khronosproject/wasi-test-suite into emscripten: emscripten-core/emscripten#12704

Once most tests are passing (not yet) I will update https://github.com/khronosproject/wasi-test 's compat.py to include an emscripten test function!

@caspervonb
Copy link
Contributor

caspervonb commented Jan 14, 2021

A little update:

I've been meaning to split the tests into sub repositories for a while and finally got around to it a couple of days ago, then I wasn't happy with the harness so I did it again.

The wasi-test repository is going the way of the dodo, it was trying to be a crate but it just made it convoluted to work with.

There are currently three source repositories:

These get built and merged into (https://github.com/caspervonb/wasi-testsuite); work in progress so coverage there is actually a bit less.

I've dropped the json preludes from the source files and instead gone with a simple file matching approach very similar to what is in wasi-sdk; meaning that each binary may have any of the following auxiliary files next to it:

  • <basename>.arg
  • <basename>.env
  • <basename>.stdin
  • <basename>.stdout
  • <basename>.stderr
  • <basename>.status

Intend to expand it with a couple of more auxiliary files and directories as I rewrite the old tests including but not limited to:

  • .dir
  • .invoke

A preliminary test runner used in the source repositories can be found at https://github.com/caspervonb/wasi-test-tools.

Heads up, it's bare bones, just enough to run the tests in CI.

At this point I'm pretty much accidentally converging with what we discussed earlier so going forward so I don't see much point in continuing this work outside of the webassembly/wasi-* repositories as I'm just starting on the rewrite so I'll start topping up @sunfishcode's review queue.

However, it seems however that the restructure still is not complete but I suppose that doesn't really interfere with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal A discussion about a specific actionable item tests
Projects
None yet
Development

No branches or pull requests