WebAssembly / wasi-testsuite

WASI Testsuite
Apache License 2.0
51 stars 20 forks source link

Migrate Rust tests from wasmtime #24

Open loganek opened 1 year ago

loganek commented 1 year ago

Looks like wasmtime already has a good coverage for WASI interface here. It'd be beneficial to move those tests here so other runtimes can easily access them

yamt commented 1 year ago

FYI, some of them are testing behaviors which i couldn't find the authoritive text in wasi. they might be implementation (wasmtime) specific.

sunfishcode commented 1 year ago

FYI, some of them are testing behaviors which i couldn't find the authoritive text in wasi. they might be implementation (wasmtime) specific.

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/lib/wasi.c#L640

            /* Note: wasmtime returns EINVAL for embedded NULs */

This error condition does not occur in POSIX so there is no obvious choice here. Do you have a suggestion for an error code that makes more sense than EINVAL here?

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/lib/wasi.c#L1382-L1383

         /*
  • Note: wasmtime directory_seek.rs test expects EBADF.
  • Why not EISDIR? */

No strong opinion here; EISDIR sounds reasonable; I'll look into changing Wasmtime.

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/lib/wasi.c#L1669-L1671

This was discussed here: https://github.com/WebAssembly/WASI/pull/193 and EINVAL was discussed in a WASI meeting.

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/lib/wasi.c#L2319-L2334

I think you're right; Wasmtime should follow POSIX here and allow readlink to return a truncated result.

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/test/wasmtime-wasi-tests-blacklist-Linux.txt#L1

That test is intending to test the POSIX rules. Which parts of it do you find incorrect?

yamt commented 1 year ago

FYI, some of them are testing behaviors which i couldn't find the authoritive text in wasi. they might be implementation (wasmtime) specific.

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/lib/wasi.c#L640
            /* Note: wasmtime returns EINVAL for embedded NULs */

This error condition does not occur in POSIX so there is no obvious choice here. Do you have a suggestion for an error code that makes more sense than EINVAL here?

is there anything which prohibits a WASI implementation from accepting a path with NULs?

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/lib/wasi.c#L1382-L1383
         /*
             * Note: wasmtime directory_seek.rs test expects EBADF.
             * Why not EISDIR?
             */

No strong opinion here; EISDIR sounds reasonable; I'll look into changing Wasmtime.

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/lib/wasi.c#L1669-L1671

This was discussed here: WebAssembly/WASI#193 and EINVAL was discussed in a WASI meeting.

ok. thank you.

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/lib/wasi.c#L2319-L2334

I think you're right; Wasmtime should follow POSIX here and allow readlink to return a truncated result.

* https://github.com/yamt/toywasm/blob/cf6f19d7b2921c7a244e95e38f36c5b6339a708d/test/wasmtime-wasi-tests-blacklist-Linux.txt#L1

That test is intending to test the POSIX rules. Which parts of it do you find incorrect?

path_rename_file_trailing_slashes fails on macOS because of errno mismatch.

4: thread 'main' panicked at 'expected errno NOTDIR; got NOENT', src/bin/path_rename_file_trailing_slashes.rs:18:5

path_symlink_trailing_slashes fails on Linux for a similar reason.

4: thread 'main' panicked at 'expected errno NOTDIR or NOENT; got EXIST', src/bin/path_symlink_trailing_slashes.rs:45:5

i'm not sure if it's WASI's responsibility to "fix" nuances of the underlying platforms/filesystems like this.