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

Migrate Rust tests from wasmtime #24

Open
loganek opened this issue Nov 25, 2022 · 3 comments
Open

Migrate Rust tests from wasmtime #24

loganek opened this issue Nov 25, 2022 · 3 comments

Comments

@loganek
Copy link
Collaborator

loganek commented Nov 25, 2022

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

@sunfishcode
Copy link
Member

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: WebAssembly/WASI#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
Copy link
Contributor

yamt commented Dec 16, 2022

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.

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

No branches or pull requests

3 participants