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

Move integration tests to tests dir in workspace root #1349

Merged
merged 9 commits into from
Apr 7, 2020

Conversation

MarkMcCaskey
Copy link
Contributor

Reorganizing some of our tests.

Still tests left to migrate and a few bugs to fix.

Review

  • Add a short description of the the change to the CHANGELOG.md file

@MarkMcCaskey MarkMcCaskey added 📦 lib-compiler-cranelift About wasmer-compiler-cranelift 📦 lib-emscripten About wasmer-emscripten 📦 lib-deprecated About the deprecated crates 📦 lib-runtime 🧪 tests I love tests 📦 lib-compiler-llvm About wasmer-compiler-llvm 📦 lib-wasi About wasmer-wasi ⏱ metering labels Apr 2, 2020
@syrusakbary
Copy link
Member

syrusakbary commented Apr 2, 2020

Some feedback:

  • Can we rename tests/wasitests to tests/wasi?
  • Can generate-wasi-tests be just an .rs file rather than a crate? (that way, we can reuse the dependencies, via dev-dependencies in the toplevel cargo)

@MarkMcCaskey
Copy link
Contributor Author

Some feedback:

* Can we rename `tests/wasitests` to `tests/wasi`?

* Can `generate-wasi-tests` be just an `.rs` file rather than a crate? (that way, we can reuse the dependencies, via `dev-dependencies` in the toplevel cargo)

Renaming is easy! I had trouble getting this to work properly without having them be crates: the current design is designed around calling code from a top level build.rs and I don't know how we'd import it otherwise.

It's gone through quite a few iterations already to get here, but I'm sure there's ways to improve it

@syrusakbary
Copy link
Member

I had trouble getting this to work properly without having them be crates: the current design is designed around calling code from a top level build.rs and I don't know how we'd import it otherwise.

Ok, fair point!

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Apr 3, 2020
@bors
Copy link
Contributor

bors bot commented Apr 3, 2020

try

Build failed

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Apr 3, 2020
@MarkMcCaskey
Copy link
Contributor Author

bors try

@bors
Copy link
Contributor

bors bot commented Apr 3, 2020

try

Already running a review

@MarkMcCaskey
Copy link
Contributor Author

bors try-

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Apr 3, 2020
@bors
Copy link
Contributor

bors bot commented Apr 4, 2020

try

Build failed

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Apr 4, 2020
@bors
Copy link
Contributor

bors bot commented Apr 4, 2020

try

Build failed

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Apr 6, 2020
@MarkMcCaskey
Copy link
Contributor Author

bors try-

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Apr 6, 2020
@bors
Copy link
Contributor

bors bot commented Apr 6, 2020

try

Build failed

@MarkMcCaskey
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Apr 6, 2020
@bors
Copy link
Contributor

bors bot commented Apr 6, 2020

try

Build succeeded

@MarkMcCaskey MarkMcCaskey marked this pull request as ready for review April 6, 2020 23:47
@MarkMcCaskey MarkMcCaskey requested a review from nlewycky as a code owner April 6, 2020 23:47
@MarkMcCaskey
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Apr 6, 2020
1349: Move integration tests to `tests` dir in workspace root r=MarkMcCaskey a=MarkMcCaskey

Reorganizing some of our tests.

Still tests left to migrate and a few bugs to fix.

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <[email protected]>
@bors
Copy link
Contributor

bors bot commented Apr 7, 2020

Canceled

@MarkMcCaskey
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Apr 7, 2020
1349: Move integration tests to `tests` dir in workspace root r=MarkMcCaskey a=MarkMcCaskey

Reorganizing some of our tests.

Still tests left to migrate and a few bugs to fix.

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


1357: Refactored bin commands into separate files r=syrusakbary a=syrusakbary

<!-- 
Prior to submitting a PR, review the CONTRIBUTING.md document for recommendations on how to test:
https://github.com/wasmerio/wasmer/blob/master/CONTRIBUTING.md#pull-requests

-->

# Description

Refactored bin commands into separate files.
This PR does not do any sustancial changes other than refactoring into different files for better readability.

<!-- 
Provide details regarding the change including motivation,
links to related issues, and the context of the PR.
-->

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Syrus <[email protected]>
Co-authored-by: Syrus Akbary <[email protected]>
Cargo.toml Outdated
crate-type = ["bin"]

[[example]]
name = "callback"
crate-type = ["bin"]
crate-type = ["bin"]
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have the trailing newline back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, usually when these get removed it's due to some tool doing it. I don't know if rustfmt affects cargo toml files (not sure what else it could be) 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or maybe I accidentally deleted it 😆

use wasmer_dev_utils::stdio::StdioCapturer;

let wasm_bytes = include_bytes!($file);
let backend = $crate::emtests::_common::get_backend().expect("Please set one of `WASMER_TEST_CLIF`, `WASMER_TEST_LLVM`, or `WASMER_TEST_SINGELPASS` to `1`.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this error string may make a suggestion that won't help (WASM_TEST_LLVM=1 when llvm isn't compiled in).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fine with that for now, spectests has its own version of this code that's more robust. Ideally we'll combine them together but that should be done in a PR where it's possible to read and understand the diff


static BANNER: &str =
"// Rust test file autogenerated with cargo build (generate-emscripten-tests).
// Please do NOT modify it by hand, as it will be reseted on next build.\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Please do NOT modify it by hand, as it will be reseted on next build.\n";
// Please do NOT modify it by hand, as it will be reset on next build.\n";

Copy link
Member

Choose a reason for hiding this comment

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

That's probably my english 😂

@MarkMcCaskey
Copy link
Contributor Author

bors r-

@bors
Copy link
Contributor

bors bot commented Apr 7, 2020

Canceled

@MarkMcCaskey MarkMcCaskey force-pushed the feature/reorganized-tests branch from 515e49c to ade38aa Compare April 7, 2020 00:22
@MarkMcCaskey
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Apr 7, 2020
1349: Move integration tests to `tests` dir in workspace root r=MarkMcCaskey a=MarkMcCaskey

Reorganizing some of our tests.

Still tests left to migrate and a few bugs to fix.

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


1357: Refactored bin commands into separate files r=syrusakbary a=syrusakbary

<!-- 
Prior to submitting a PR, review the CONTRIBUTING.md document for recommendations on how to test:
https://github.com/wasmerio/wasmer/blob/master/CONTRIBUTING.md#pull-requests

-->

# Description

Refactored bin commands into separate files.
This PR does not do any sustancial changes other than refactoring into different files for better readability.

<!-- 
Provide details regarding the change including motivation,
links to related issues, and the context of the PR.
-->

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Syrus <[email protected]>
Co-authored-by: Syrus Akbary <[email protected]>
@bors
Copy link
Contributor

bors bot commented Apr 7, 2020

Build failed (retrying...)

  • wasmerio.wasmer

@syrusakbary
Copy link
Member

Merging manually

@syrusakbary syrusakbary merged commit bc75790 into master Apr 7, 2020
@bors bors bot deleted the feature/reorganized-tests branch April 7, 2020 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 lib-compiler-cranelift About wasmer-compiler-cranelift 📦 lib-compiler-llvm About wasmer-compiler-llvm 📦 lib-deprecated About the deprecated crates 📦 lib-emscripten About wasmer-emscripten 📦 lib-wasi About wasmer-wasi ⏱ metering 🧪 tests I love tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants