Update Wasm API test to use provider imports#498
Conversation
| Ok(path) | ||
| }); | ||
|
|
||
| fn download_trampoline(destination: &Path, version: &str) -> Result<()> { |
There was a problem hiding this comment.
Is there a reasonable alternative to hitting the network here (even if infrequently)? My main worry is that this might potentially make tests flaky. Have you considered committing the shopify-function-trampoline-<triple>.wasm?
There was a problem hiding this comment.
Mostly I was copying the approach we use for the shopify_function Rust crate here.
We could commit the trampolined file. The part that kind of sucks with rebuilding the Wasm file manually is getting the correct version of the trampoline since if you grab the wrong version, the trampolining will appear to work but running it will fail with a failed to instantiate error. I could also update the test such that we check if the trampoline file exists and don't bother downloading and running the trampoline if it does and then update the instructions for rebuilding the fixtures to say delete the trampolined version and run the tests to rebuild it. Or I could write a Makefile to handle building the fixtures and that could handle downloading and running the correct trampoline version. Thoughts?
There was a problem hiding this comment.
Oh, I wrongly assumed that you were introducing this approach in this PR without realizing that it was a pre-existing approach (in some of our other repos). In that case, I believe we can leave it.
While trying to add a test case for using a v2 of the Wasm API, I noticed the v1 Wasm API test case doesn't import the Wasm API and uses WASI's stdio directly. So I'm updating it here so I can use a similar test case for v2. I'm also opting to add a
test-utilscrate to perform the trampolining since we will need to use multiple trampoline versions to rebuild the test fixtures in the near future.