Copy some of the WASI Rust tests from wasmtime #32
Conversation
loganek
force-pushed
the
loganek/rust-tests
branch
from
9df0046 to
bfd68ac
Compare
|
why not all of them? |
|
For now I only migrated tests starting from A to F; haven't done all of them to get early feedback before I spend more time on that. |
ok. it makes sense. i have concerns on some of test cases: #24 (comment) |
loganek
force-pushed
the
loganek/rust-tests
branch
from
bfd68ac to
d5ef781
Compare
Thanks, I've taken the ones related to this PR into account. |
tests/rust/src/bin/directory_seek.rs
Outdated
| @@ -27,7 +27,7 @@ unsafe fn test_directory_seek(dir_fd: wasi::Fd) { | |||
| wasi::fd_seek(fd, 0, wasi::WHENCE_CUR) | |||
| .expect_err("seek on a directory") | |||
| .raw_error(), | |||
| wasi::ERRNO_BADF | |||
| wasi::ERRNO_ISDIR | |||
There was a problem hiding this comment.
i guess it's more reasonable to accept both.
There was a problem hiding this comment.
I'd rather have tests very specific so implementers can use it as a guidance. IIRC @sunfishcode will look into changing that in wasmtime, do you know of any other runtime where the error code for this case is BADF?
There was a problem hiding this comment.
I'd rather have tests very specific so implementers can use it as a guidance.
without having it in the spec itself? why?
IIRC @sunfishcode will look into changing that in wasmtime, do you know of any other runtime where the error code for this case is
BADF?
toywasm. (because it followed wasmtime)
There was a problem hiding this comment.
without having it in the spec itself? why?
There's no error code mentioned in spec itself, so are you suggesting we should just check if there's any error, and ignore its value? I don't mind doing it either way, but I think even if the spec doesn't mention the value of the error code, we can still have a specific assertion here, and use it as a guidance for the future spec changes.
There was a problem hiding this comment.
my suggestion is
- accept both values here for now (not necessarily "any error". probably just BADF and ISDIR are enough.)
- start mentioning error codes in the spec
There was a problem hiding this comment.
ok, I updated it so it accepts BADF, ISDIR and NOTCAPABLE (this one is returned by WAMR).
There was a problem hiding this comment.
although NOTCAPABLE sounds very awkward to me, i guess it isn't a problem in this PR
…y cleaned up by the test runner
The return value is not documented so we allow for a few reasonable error codes here.
So we fail the script if one of the test doesn't compile
loganek
force-pushed
the
loganek/rust-tests
branch
from
d5ef781 to
0b39314
Compare
|
@sunfishcode / @yamt let me know if you have any more comments, otherwise I'll go ahead and merge that (we can always update the code if needed). |
|
Looks good to me; we can iterate from here. I also agree that |
No description provided.