Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(std/wasi): build testdata resources offline #6695

Closed

Conversation

caspervonb
Copy link
Contributor

@caspervonb caspervonb commented Jul 10, 2020

This changes up how the std/wasi test suite is built a little bit.

  • The build script is extrapolated to a standalone Python script. Went with Python here just because it's what I would do if the testdata directory was a standalone repository (an eventual side goal). Less friction when running the tests against other runtimes and it's what WebAssembly uses to build its own spec tests.

  • Tests are built ahead of time and checked in, this is because the Rust the toolchain used to build tests for a specific ABI snapshot is not necessarily the same toolchain that Deno uses so the CI isn't a suitable environment to build them on in the long run.

The motivation for this is that I'm hitting the ceiling of Rust being helpful in writing integration tests, while the assertions provided are nice and all it's difficult to get at certain syscalls so some parts just don't have any coverage. We can't even call UNIX extensions like read_at because WASI is not UNIX so I want to introduce an additional layer of tests written in C. Once we introduce C tests we hit the toolchain issue again, the toolchain installed on CI is not the one that's required for libc-wasi so it gets convoluted to set up the CI for both the native target and wasm32-wasi and overall it is just easier and cleaner to pre-bake the testdata and check it in.

A nice side effect is that a test run now takes 2 seconds, instead of the 47 seconds it takes to build and run the tests (on my machine, CI is probably significantly faster overall).

@caspervonb caspervonb force-pushed the test-std-wasi-prebuild-testdata branch from 0c6e384 to 1c3f601 Compare July 10, 2020 12:41
@caspervonb caspervonb marked this pull request as ready for review July 10, 2020 12:46
@caspervonb caspervonb force-pushed the test-std-wasi-prebuild-testdata branch from 1c3f601 to 692ffc9 Compare July 10, 2020 13:15
@caspervonb caspervonb force-pushed the test-std-wasi-prebuild-testdata branch from 692ffc9 to 6545540 Compare July 10, 2020 13:50
@ry
Copy link
Member

ry commented Jul 10, 2020

I don't like the introduced python dependency. Why not use Deno?

We're actively working on removing our python2 tooling dependency. Getting closer week by week. I definitely do not want to add a python3 dependency on that.

@caspervonb
Copy link
Contributor Author

caspervonb commented Jul 10, 2020

I don't like the introduced python dependency. Why not use Deno?

Moving towards a standalone test suite repository so that I can re-use it in web-wasi and pool together efforts from other vendors and maintainers, Python is the middle-ground in existing spec tests.

But we can maintain our own TypeScript port of the build script I suppose.

@ry
Copy link
Member

ry commented Jul 10, 2020

Another idea is to structure the testdata as a rust crate with many bins rather than scripting the compilation. Tho I see you want to have non-rust tests.

But we can maintain our own TypeScript port of the build script I suppose.

That would be preferable than python for us.

@caspervonb caspervonb marked this pull request as draft July 10, 2020 15:53
@caspervonb
Copy link
Contributor Author

caspervonb commented Jul 15, 2020

I marked this as a draft again as it got quite a bit of sync with what's been going on.

So I'm moving towards a standalone test suite which from the looks of it will become the conformance test suite for the WASI spec. There's a discussion with a bit more context over at webassembly/wasi.

I'm not sure what the final build system and prerequisites will be but it will be different from Deno's toolchain as it requires a WASI enabled sysroot.

I'm closing this without a merge, I will revisit this once the details of where the test suite is going to live are worked out (e.g, if we provide a repository with binaries checked in then that's dead simple to consume and pull from).

@caspervonb caspervonb closed this Jul 15, 2020
@caspervonb
Copy link
Contributor Author

This was resolved in #7042

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants