Stream: git-wasmtime

Topic: wasmtime / PR #6385 Refactor test-programs to build modul...

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 03:44):

pchickey opened PR #6385 from bytecodealliance:pch/test_programs to bytecodealliance:main:

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

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
-->

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 03:45):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 03:46):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 03:49):

pchickey edited PR #6385:

This is task 2 in https://github.com/bytecodealliance/wasmtime/issues/6370

On the tails of https://github.com/bytecodealliance/wasmtime/pull/6374, this adds support for test-programs to build components out of preview 1 modules.

But thats really just the side story for this PR, which rewrites the entire way that test-programs works. Over in preview2-prototyping, I got tired of doing a bunch of code generation of all the #[test] functions, which makes it tests to read and hard to modify.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 03:54):

pchickey edited PR #6385:

This is task 2 in https://github.com/bytecodealliance/wasmtime/issues/6370

On the tails of https://github.com/bytecodealliance/wasmtime/pull/6374, this adds support for test-programs to build components out of preview 1 modules.

But thats really just the side story for this PR, which rewrites the entire way that test-programs works. Over in preview2-prototyping, I got tired of doing a bunch of code generation of all the #[test] functions, which makes it tests to read and hard to modify.

So, I trimmed code generation in test-programs/build.rs down to the bare minimum: it calls out to cargo to build the test program crates, and then we have a function module_rs() which writes a source file {test-crate}_modules.rs into OUT_DIR. This source file defines a function get_module(&str) -> Module which gives the user the module corresponding to the binary name.

For test program crates which can be built into components, you can invoke component_rs() which creates {test-crate}_components.rs in OUT_DIR, and you use get_component(&str) -> wasmtime::component::Component. One extra argument is required: a built adapter, which is the command build of wasi-preview1-component-adapter from this tree.

As part of this rewrite, we generate the components for the test crates, but we don't actually execute them yet, because we need a host implementation of preview 2 to do so. Sit tight - that is coming in the very next PR!

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 17:00):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 17:06):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 18:42):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:31):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:31):

pchickey has marked PR #6385 as ready for review.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:31):

pchickey requested jameysharp for a review on PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:31):

pchickey requested wasmtime-core-reviewers for a review on PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:31):

pchickey requested wasmtime-default-reviewers for a review on PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:33):

pchickey requested alexcrichton for a review on PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:37):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:44):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:44):

alexcrichton created PR review comment:

Oh? I've not seen this dependency before, what does it do?

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:44):

alexcrichton created PR review comment:

Could this move to a { workspace = true } dep in the top-level Cargo.toml since it's probably gonna show up in a few places? (also good to have in one place to update when all of wasm-tools is bumped)

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:44):

alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:44):

alexcrichton created PR review comment:

Like wit-component, could this move to a workspace dependency?

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:44):

alexcrichton created PR review comment:

One thing that this pattern is losing is that if a test is added there's no guarantee that a test is added here. Could that be fixed perhaps with the build script generating something like:

use self::big_random_buf as _;

in the generated code? That'll trigger a compile error if there's a test without a corresponding #[test]

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:44):

alexcrichton created PR review comment:

Could you check to see if 0.2.13, which is exempted, is a small or big diff from 0.3.5? If small mind adding an audit for that? If big though it's a bit of a bummer but we can probably live with an exemption.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:44):

alexcrichton created PR review comment:

Could this diff be audited to avoid adding a new exemption? (assuming that this diff is a relatively small one hopefully)

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:53):

pchickey created PR review comment:

It is a wrapper over #[test] (or e.g. #[tokio::test] if you pass it in as an argument) which takes care of installing tracing-subscriber with an env filter before starting your test. It gets rid of a Once cell we used to do that manually.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:53):

pchickey edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:54):

pchickey created PR review comment:

I didn't want to do any sort of audit on this because it is the redox syscall layer, an OS I don't know anything about and don't think we care about in the context of this project. It just happens to be a transitive dep in some crates.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 19:56):

pchickey created PR review comment:

I could, I just figured that if we have an exemption I'd keep bumping it. I've been thinking about our exemptions of the tokio/hyper family (which this is a part of) is a wildcard audit for carl and sean.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 20:08):

alexcrichton created PR review comment:

Ok sounds reasonable :+1:

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 20:08):

alexcrichton created PR review comment:

Ok sounds reasonable :+1:

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 20:09):

pchickey requested abrown for a review on PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 20:09):

pchickey requested wasmtime-compiler-reviewers for a review on PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 20:09):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 20:10):

pchickey created PR review comment:

Great idea, I will do that!

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 20:32):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 20:32):

pchickey has enabled auto merge for PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 21:40):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 22:07):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 22:17):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 23:16):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 23:21):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 16 2023 at 23:50):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 17 2023 at 00:05):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 17 2023 at 02:57):

pchickey has disabled auto merge for PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 17 2023 at 03:08):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 17 2023 at 17:43):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 17 2023 at 17:46):

pchickey updated PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 17 2023 at 17:51):

pchickey has enabled auto merge for PR #6385.

view this post on Zulip Wasmtime GitHub notifications bot (May 17 2023 at 18:57):

pchickey merged PR #6385.

Last updated: Dec 23 2024 at 13:07 UTC