Stream: git-wasmtime

Topic: wasmtime / PR #8147 Move all tests out of `cranelift-wasm`


view this post on Zulip Wasmtime GitHub notifications bot (Mar 15 2024 at 16:30):

alexcrichton opened PR #8147 from alexcrichton:remove-all-cranelift-wasm-tests to bytecodealliance:main:

This commit moves all remaining tests out of the cranelift-wasm crate into other various test suites. My end goal is to eventually remove the Dummy* trait for wasm translation so we don't have to keep updating that over time, but for now these tests I think reside in a more appropriate location nowadays.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 15 2024 at 16:30):

alexcrichton requested abrown for a review on PR #8147.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 15 2024 at 16:30):

alexcrichton requested fitzgen for a review on PR #8147.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 15 2024 at 16:30):

alexcrichton requested wasmtime-compiler-reviewers for a review on PR #8147.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 15 2024 at 16:30):

alexcrichton requested wasmtime-core-reviewers for a review on PR #8147.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 15 2024 at 23:40):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 16 2024 at 00:06):

abrown merged PR #8147.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 16 2024 at 00:32):

jameysharp commented on PR #8147:

I was too slow to make these changes and you got to them first :laughing:

I had done a bit more historical research about the issue-1306 test case which I'll throw here in case anyone wonders later:

This test case was extracted from a fuzz bug in 2019, before Cranelift was merged into the Wasmtime repo:
https://github.com/bytecodealliance/cranelift/issues/1306

That test is interesting, but running it here only tests code in DummyEnvironment, not anything reachable from Wasmtime. There was a report of exactly the same bug in Wasmtime two years later (#3509), and the fix included tests in tests/misc_testsuite/empty.wast to cover this case.

The empty.wast test is better than this one because it has minimal and well-commented binary modules which I've verified still trigger the same bug if the fix is reverted. By contrast, since this older test was produced by a fuzzer, it is larger than necessary and the end of its name section is malformed and unparseable.

Also, I had concluded that the reachability tests were pretty well covered by tests/disas/*reach*.wat, but your addition of tests/disas/dead-code.wat certainly can't hurt.


Last updated: Oct 23 2024 at 20:03 UTC