-
Notifications
You must be signed in to change notification settings - Fork 21
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
doc: document how a test executor works #74
Conversation
This describes the JSON specification for others who might want to write their own test executors.
The primary purpose of this change is to link to `doc/specification.md` but while I was here I made some minor text cleanups.
- `env`: construct an environment from each key-value pair in `env`; the environment should only | ||
contain these keys and otherwise should be empty (note that this environment is the WASI | ||
environment, whether the engine inherits the shell environment or explicitly configures it via | ||
`--env` parameters) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the --env
option is runtime-specific i guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the parenthetical is giving an example.
|
||
To execute the tests, the test executor is expected to: | ||
- `env`: construct an environment from each key-value pair in `env`; the environment should only | ||
contain these keys and otherwise should be empty (note that this environment is the WASI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this something from the wasi spec?
note that some of runtimes have some environment variables set by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one of the tests sets three keys and then checks that the environment has exactly three keys, so I don't know what those other runtimes will do. The implication of that test seems to be that the environment is empty otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess the test should be relaxed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(out of curiosity) what are the runtimes that already add additional variables? Do they also provide a mechanism to remove those variables?
If WASI spec doesn't define that the environment should be empty, than I agree we need to update the test and the documentation. I've created an issue to not forget about it: #77
### Checking | ||
|
||
With the execution results in hand, the test executor is expected to: | ||
- `exit_code`: check that the WASI exit code matches `exit_code`, or `0` if none was provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right now we don't distinguish wasi exit code and other errors.
cf. #39
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is a different issue. What I noticed running the wasi-threads tests was that they failed if the exit code did not line up with the JSON specification, which is what I'm documenting here. Unfortunately, there are some problems with this but the issue to reference for that is WebAssembly/wasi-cli#11.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that is a different issue. What I noticed running the wasi-threads tests was that they failed if the exit code did not line up with the JSON specification, which is what I'm documenting here. Unfortunately, there are some problems with this but the issue to reference for that is WebAssembly/wasi-cli#11.
how is it a different issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest continuing the discussion in #39, I think the documentation here describes the existing behavior.
This change describes the JSON specification — and how a test executor is supposed to work — for others who might want to write their own test executors. Over in the
wasmtime
project, the expectation is that most tests are Rust-based, so I created a Rust test executor (see bytecodealliance/wasmtime#6341, e.g.) that follows this specification. I propose we keep this document up to date as this repository progresses to make it clear that the suite itself is independent from the Python test executor.The primary purpose of this change is to create
doc/specification.md
but I also made some minor text cleanups to the README. I can back any of those out if desired!