search

bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
15.08k stars 1.26k forks source link

Tests for cranelift-wasm use wat files located outside the crate #2910

Closed olivierlemasle closed 3 years ago

olivierlemasle commented 3 years ago

cranelift-wasm's tests use wat files which are located outside the crate (in directory ../wasmtests).

This causes test failures when the tests are executed outside wasmtime's repository, which is the case when packaging cranelift's crates (I'm currently in the process of packaging wasmtime & cranelift for Fedora).

Which option do you prefer?

  1. Moving wasmtests in the crate cranelift-wasm. :heavy_plus_sign: That's the cleanest/easiest way to make the tests autonomous :heavy_minus_sign: The crate's size grows from 340 KB to 2.2 MB.
  2. Making wasmtests an independent crate, published on crates.io, and adding it to cranelift-wasm's dev-dependencies. :heavy_plus_sign: Crates that depend on cranelift-wasm do not have to fetch the test files. :heavy_minus_sign: We need to publish a crate with (wat) data only and no Rust...
  3. Skip the tests if ../wasmtests do not exist :heavy_plus_sign: No change in the packaging :heavy_minus_sign: The tests will not be executed outside the git repo

/cc @cfallin because you expressed interest on this Fedora packaging

cfallin commented 3 years ago

@olivierlemasle thanks for pointing this out!

I think I would favor Option 1 as the most canonical and the cleanest. Aside from the growth in size, it doesn't require us to publish a data-only crate, and it doesn't create a weird non-deterministic testing dependence on the filesystem outside the crate's root. We could eventually opt to split the tests out as a separate piece of infrastructure, however packaging them as part of the crate now doesn't prevent us from doing that later.

So: I'd be happy to review a PR that just moves the directory and alters the path in the testsuite driver. Thanks!

abrown commented 3 years ago

Fedora packaging would be nice; what repository this will be in, etc.?

olivierlemasle commented 3 years ago

@abrown It will be included in the main Fedora repository, starting with Fedora 34 normally. But there's still a long way to go. Here is my status: https://olem.fedorapeople.org/wasmtime/