WebAssembly
RFC: Remove tests of WASI preview 1 rights API as required
Summary
I propose that if there are failing tests related to the WASI preview 1 rights API, that we modify or delete those tests to remove those assertions.
Problem
Wasmtime fails some tests currently; #101. I am working on adding wasip3 tests, and I want to make sure I keep old tests working, and it's annoying to have some pre-existing failures, and I need to figure out what to do with them.
Background
In WASI preview 1, the wasi-filesystem interface had a concept of "rights", which tried to build an alternative capability-adjacent system that could work alongside the needed facilities to support compilation of POSIX programs to WASI. Ultimately this was deemed too complicated and was removed in WASI preview 2; see https://github.com/WebAssembly/wasi-filesystem/issues/31.
wasi-testsuite was written with two kinds of tests: stdlib tests, and wasi tests.
A stdlib test, like fdopendir-with-access.c, is coded against a language's standard library (POSIX in this case); it's the standard library from the toolchain that uses WASI, not the end user. It's important to keep this kind of test working, and here I don't propose any changes.
But a wasi test like fd_fdstat_set_rights.rs is written directly against WASI interfaces. Unlike, say, POSIX, there were no users of the rights API before wasip1, and there have not been any since then, as it was removed in wasip2. This test fails on wasmtime currently.
We could ask wasmtime and other wasip1 implementations to ensure that the rights API continues to work, but this imposes costs on systems as they evolve to wasip2 and beyond, given that the rights system is no longer present in those versions.
Proposal
I propose that if a wasi test of an API that was removed after wasip1 fails on any WASI runtime, that we remove that test or remove the failing assertion, unless that API is used by some language's standard toolchain.
Concretely, following @alexcrichton's triage in https://github.com/WebAssembly/wasi-testsuite/issues/101#issuecomment-3211783679, I would start with making the needed changes to the rust truncation_rights, path_link, and fd_fdstat_set_rights tests so that they pass on wasmtime.
Thanks for raising this RFC. I'm not sure if deleting the tests only because they fail is the right approach. For failing tests, we should verify if the test is correct (it's possible that there are bugs in the tests) and if so, update it. If the test is valid, but the runtime still fails on it, I think the runtime should either fix the issue on their side, or they should be marked as WASIp1 noncompliant (or partially compliant) - which might be acceptable for some of the runtimes.
As alternative, we might consider updating WASIp1 standard, but 1) this standard IIUC is frozen, and 2) I think that'd have significant implications that would be difficult to control.
there were no users of the rights API before wasip1, and there have not been any since then, as it was removed in wasip2
I don't think we can say it with high confidence. Even if we review all possible toolchains, there's still a chance that some users hand-write WAT files and rely on this behavior.
Overall my view is that this repository should first of all provide a conformance tests for the standard. If the runtime chooses to not support any of the usecases, their free to do so, and best we can do to support it is to provide option in the test runner to disable certain tests.
For what it is worth, here are the current results on these tests:
$ for i in pywasm-runtime wasm-micro-runtime wasmedge wasmtime wazero; do for t in truncation_rights path_link fd_fdstat_set_rights; do echo -en "$i/$t:\t\tpass: "; git cat-file -p origin/prod/daily-test-results:results/$i/latest.json | jq "(.results[].tests[] | select(.name == \"$t\")).failures == []"; done; done
pywasm-runtime/truncation_rights: pass: true
pywasm-runtime/path_link: pass: true
pywasm-runtime/fd_fdstat_set_rights: pass: true
wasm-micro-runtime/truncation_rights: pass: true
wasm-micro-runtime/path_link: pass: true
wasm-micro-runtime/fd_fdstat_set_rights: pass: true
wasmedge/truncation_rights: pass: true
wasmedge/path_link: pass: true
wasmedge/fd_fdstat_set_rights: pass: true
wasmtime/truncation_rights: pass: false
wasmtime/path_link: pass: false
wasmtime/fd_fdstat_set_rights: pass: false
wazero/truncation_rights: pass: false
wazero/path_link: pass: false
wazero/fd_fdstat_set_rights: pass: false
For completeness:
for i in pywasm-runtime wasm-micro-runtime wasmedge wasmtime wazero; do echo; echo "$i"; git cat-file -p origin/prod/daily-test-results:results/$i/latest.json | jq "(.results[].tests[] | select(.failures != [])).name"; done
pywasm-runtime
wasm-micro-runtime
"pwrite-with-append"
wasmedge
wasmtime
"path_filestat"
"remove_directory_trailing_slashes"
"fd_fdstat_set_rights"
"truncation_rights"
"renumber"
"file_allocate"
"fd_advise"
"path_link"
"pwrite-with-append"
wazero
"path_filestat"
"path_open_read_write"
"poll_oneoff_stdio"
"dir_fd_op_failures"
"readlink"
"fd_fdstat_set_rights"
"truncation_rights"
"renumber"
"symlink_filestat"
"stdio"
"path_link"
"overwrite_preopen"
"pwrite-with-append"
"sock_shutdown-not_sock"
I've proposed https://github.com/WebAssembly/wasi-testsuite/pull/124 to skip some tests if rights aren't supported. Rights aren't fully specified in WASIp1 so what implementations do is sort of up to them. I'd propose consolidating around a consensus of "whatever this testsuite currently tests" or "don't support rights" which is similar to many other checks throughout this testsuite for what the host supports and what it doesn't.
