Stream: git-wasmtime

Topic: wasmtime / PR #6341 test: add cases from `wasi-testsuite`

Wasmtime GitHub notifications bot (May 04 2023 at 18:05):

abrown opened PR #6341 from abrown:more-wasi-tests to bytecodealliance:main:

This change alters the wasi_testsuite test to run all of the available
test cases in [wasi-testsuite]. This involved making the test runner a
bit more robust to the various shapes of JSON specifications in that
project. Unfortunately, the wasi_testsuite test fails some of the
cases, so I added a WASI_COMMON_IGNORE_LIST to avoid these
temporarily. (This may remind some of the Wasm testsuite ignore lists in
Cranelift; those relied on build.rs to create a #[test] for each
test case, which I felt is not yet needed here).

It's unclear to me why some tests are failing. It could be because:

• wasi-common has a bug
• wasi-testsuite overspecifies (or incorrectly specifies) a test
• the test runner incorrectly configures Wasmtime's CLI execution.

But this change makes it easier to resolve this. Remove the file from
WASI_COMMON_IGNORE_LIST and run cargo test wasi_testsuite -- --nocapture. The printed output will show the expected result, the
actual result, and a command to replicate the failure from the command
line.

[wasi-testsuite]: https://github.com/WebAssembly/wasi-testsuite

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (May 04 2023 at 18:06):

abrown has marked PR #6341 as ready for review.

Wasmtime GitHub notifications bot (May 04 2023 at 18:06):

abrown requested elliottt for a review on PR #6341.

Wasmtime GitHub notifications bot (May 04 2023 at 18:06):

abrown requested wasmtime-compiler-reviewers for a review on PR #6341.

Wasmtime GitHub notifications bot (May 04 2023 at 18:06):

abrown requested itsrainy for a review on PR #6341.

Wasmtime GitHub notifications bot (May 04 2023 at 18:06):

abrown requested wasmtime-core-reviewers for a review on PR #6341.

Wasmtime GitHub notifications bot (May 04 2023 at 18:06):

abrown requested wasmtime-default-reviewers for a review on PR #6341.

Wasmtime GitHub notifications bot (May 04 2023 at 18:35):

elliottt created PR review comment:

Is it worth pulling this call outside the loop in run_all and passing the result in? It looks like the result should be the same across iterations of that loop.

Wasmtime GitHub notifications bot (May 04 2023 at 18:38):

elliottt created PR review comment:

Should we add a comment about whether or not this list is expected to grow?

Wasmtime GitHub notifications bot (May 04 2023 at 18:41):

abrown submitted PR review.

Wasmtime GitHub notifications bot (May 04 2023 at 18:41):

abrown created PR review comment:

Oh, I think that might make sense. We would still have to clone the Command because we mutate it but that is not a big deal.

Wasmtime GitHub notifications bot (May 04 2023 at 18:43):

abrown submitted PR review.

Wasmtime GitHub notifications bot (May 04 2023 at 18:43):

abrown created PR review comment:

I would expect this list to shrink — hopefully! Yeah, I can add something like that.

Wasmtime GitHub notifications bot (May 04 2023 at 19:06):

abrown submitted PR review.

Wasmtime GitHub notifications bot (May 04 2023 at 19:06):

abrown created PR review comment:

Hm, there is no Command::clone (learn something new every day!) so I propose we don't do this.

Wasmtime GitHub notifications bot (May 04 2023 at 19:07):

abrown updated PR #6341.

Wasmtime GitHub notifications bot (May 04 2023 at 19:50):

elliottt submitted PR review:

Looks good to me!

Wasmtime GitHub notifications bot (May 04 2023 at 20:16):

alexcrichton submitted PR review:

Looks reasonable to me too! Just one question about the dependency upgrade here.

Additionally cc @pchickey as you're probably interested in this as well for upcoming preview2 stuff

Wasmtime GitHub notifications bot (May 04 2023 at 20:16):

alexcrichton submitted PR review:

Looks reasonable to me too! Just one question about the dependency upgrade here.

Additionally cc @pchickey as you're probably interested in this as well for upcoming preview2 stuff

Wasmtime GitHub notifications bot (May 04 2023 at 20:16):

alexcrichton created PR review comment:

Could this stick to using 2.3.2 for now, or otherwise would you be ok auditing the 2.3.2 to 2.3.3 bump?

Wasmtime GitHub notifications bot (May 04 2023 at 20:37):

abrown created PR review comment:

Sure: https://github.com/bytecodealliance/wasmtime/pull/6342.

Wasmtime GitHub notifications bot (May 04 2023 at 20:51):

abrown merged PR #6341.

Wasmtime GitHub notifications bot (May 04 2023 at 20:53):

pchickey submitted PR review:

Thanks. I will have to spend some time looking at these failures soon.

Last updated: Nov 22 2024 at 17:03 UTC