-
Notifications
You must be signed in to change notification settings - Fork 35
wasi:[email protected]: Add tests for flags/type APIs #131
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
Conversation
5646b22 to
462f43f
Compare
5cb924f to
298070d
Compare
|
Hi, I have 20 or so tests that are of this general form, so if it works for y'all @pchickey and @alexcrichton would you mind taking a look that there's nothing in this PR that you wouldn't want repeated 20 times ? All the branches share the commit that adds a |
alexcrichton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me!
pchickey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one suggestion from me, maybe you already thought of this and there is a good reason it doesn't work.
| "wasi:cli/[email protected]#run", | ||
| ], | ||
| generate_all | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use wit-bindgen somewhere common (e.g. src/lib.rs) rather than having invocations in each test? That will make it easier to update version numbers, and also hopefully save compile time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into this and it got very annoying 😅
- If the mod is in
src/liblike in wasmtime's test-programs then I have to have the package name at all the use sites; annoying - The macrology needed to support multiple worlds is a bit annoying
- In general I was hoping for short & sweet & minimal
wit_bindgenworlds in each test; if we got it down to a couple lines, I think that would be a nice happy medium
All that said, I am sympathetic to the question of updating versions in umpteen places and of compile time. Let's revisit this. Happy to learn about tasteful ways to make things terse with Rust, I am still quite the newb!
298070d to
a9bb5a4
Compare
No description provided.