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

Start to update the wasi crate in wasi tests #675

Merged

Conversation

alexcrichton
Copy link
Member

This commit starts an update where all the wasi-tests/src/bin/*.rs programs are updated to the newest snapshot of the wasi API, rather tan using the old snapshot 0.

Not all tests are updated, and there's a few changes here:

  • The test harness instantiates both snapshots of the wasi API now
  • Both 0.7.0 and 0.9.0 are available to tests. The 0.7.0 version is now known as wasi_old and the 0.9.0 version is now known as just normal wasi.
  • A few tests have been updated to use wasi rather than wasi_old
  • The open_scratch_directory function had to get a duplicate which uses the most recent snapshot's file descriptors.

The purpose of this PR is to start us down the road of updating these tests, allowing the update to happen in parallel for each test. Also to get some early feedback!

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know one could depend on two versions of the same crate at once, cool!

Copy link
Member

@kubkon kubkon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all, this looks great to me, awesome job, thanks @alexcrichton! Just to strengthen my understanding though, did I get it right that we instantiate both snapshots as two Wasm modules, and then a test picks the correct one at runtime?

crates/test-programs/wasi-tests/src/bin/clock_time_get.rs Outdated Show resolved Hide resolved
crates/test-programs/tests/wasm_tests/runtime.rs Outdated Show resolved Hide resolved
crates/test-programs/wasi-tests/src/lib.rs Show resolved Hide resolved
@alexcrichton
Copy link
Member Author

Just to strengthen my understanding though, did I get it right that we instantiate both snapshots as two Wasm modules, and then a test picks the correct one at runtime?

Indeed!

Basically both snapshots are available under their respective modules, and depending on the name imported the test gets the respective functionality.

@alexcrichton
Copy link
Member Author

Updated!

@kubkon
Copy link
Member

kubkon commented Dec 6, 2019

Just to strengthen my understanding though, did I get it right that we instantiate both snapshots as two Wasm modules, and then a test picks the correct one at runtime?

Indeed!

Basically both snapshots are available under their respective modules, and depending on the name imported the test gets the respective functionality.

Hmm, after careful re-read of your comments in the issue referenced, I'm thinking it would be worth finishing what you've started here and rewriting all of the remaining tests to indeed use wasi crate. I think I should be able to do this pretty easily (here's me hoping I don't underestimate the magnitude of the task!). What do you think?

@kubkon
Copy link
Member

kubkon commented Dec 6, 2019

Updated!

LGTM! I'm happy to merge it as-is!

@kubkon
Copy link
Member

kubkon commented Dec 9, 2019

I've rebased to current master and fast-forwarded to latest Wasmtime API changes, and will merge as-is.

@kubkon kubkon merged commit e13fabb into bytecodealliance:master Dec 9, 2019
@alexcrichton alexcrichton deleted the update-wasi-crate-in-tests branch January 7, 2020 15:08
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