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 refactor #1380

Merged
merged 9 commits into from
Apr 14, 2020
Merged

Test refactor #1380

merged 9 commits into from
Apr 14, 2020

Conversation

syrusakbary
Copy link
Member

Description

This PR refactors the way we do testing in our infrastructure, simplifying test generation specially for spectests for each of the backends.

Review

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

@MarkMcCaskey
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request Apr 14, 2020
@@ -4,8 +4,108 @@

use generate_emscripten_tests;
use generate_wasi_tests;
use std::env;
Copy link
Contributor

Choose a reason for hiding this comment

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

General comment on this file: I'd like to keep as much logic as possible out of it! Because this is a top level file that people have to look at a lot or deal with making unrelated changes, it has the potential to get really messy if we start mixing all the different concerns in it.

I'm fairly happy with the way it delegated to the emtests and wasitests logic before, perhaps we need a spectest-generator as well

Copy link
Member Author

Choose a reason for hiding this comment

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

I will follow-up on another PR for that (the one that unifies emtests and wasitests) :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the change related? My intuition is that organizing code related to spectests belongs in a spectest PR and not a wasitest/emtest PR; that said I'm fine with following up on it

lib/runtime-core/src/import.rs Show resolved Hide resolved
unused_variables,
unused_unsafe,
unreachable_patterns
)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this temporary? Given how large this file is, I'd like to make sure we're guaranteeing it's high quality. That said, I think long-term we should move away from deny statements and check it in CI instead

edit: It seems that this file is no longer large, perhaps we don't need it

tests/wast/README.md Outdated Show resolved Hide resolved
build.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors bot commented Apr 14, 2020

try

Build failed:

@syrusakbary
Copy link
Member Author

bors try

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

bors bot commented Apr 14, 2020

try

Build failed:

@syrusakbary
Copy link
Member Author

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 14, 2020

Build succeeded:

  • wasmerio.wasmer

@bors bors bot merged commit 249d0b8 into master Apr 14, 2020
@bors bors bot deleted the test-refactor branch April 14, 2020 20:58
@syrusakbary syrusakbary mentioned this pull request Apr 16, 2020
13 tasks
bors bot added a commit that referenced this pull request Apr 16, 2020
1382: Test Refactor r=syrusakbary a=syrusakbary

# Description

This PR is the continuation of #1380.
It refactors our testing infrastructure based on various points:
* There is no longer a "default compiler" on tests
* Tests are automatically generated for: emscripten and wasi
* It accelerates testing into multiple threads when possible (with the exception of the llvm backend)
* It automatically detects the backends available in the host

Summary of changes:
* [x] Removed all extra logic for test creation in emscripten test generator
* [x] Removed all extra logic for test creation in the wasi test generator
* [x] Divided wasmer wast test in different files (under `tests/custom/`)
* [x] Refactored trap asserts using the wast format
* [x] Improved the build script by adding emscripten and wasi test generators (separated from build)
* [x] Improved WASI errors to be Rust error types (by deriving from `thiserror::Error`)
* [x] Fixed toolchain error creation
* [x] Removed leaking of testing logic from generators into build
* [x] Refactored import tests to be compatible with multiple backends
* [x] Improved wasmer binary to work even when no backends are set (useful for testing without any backend available).
* [x] Removed assumption that Cranelift will be the default in the `wasmer` binary
* [x] Migrated middleware

# Review

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


Co-authored-by: Syrus <me@syrusakbary.com>
Co-authored-by: Syrus Akbary <me@syrusakbary.com>
bors bot added a commit that referenced this pull request Apr 16, 2020
1382: Test Refactor r=syrusakbary a=syrusakbary

# Description

This PR is the continuation of #1380.
It refactors our testing infrastructure based on various points:
* There is no longer a "default compiler" on tests
* Tests are automatically generated for: emscripten and wasi
* It accelerates testing into multiple threads when possible (with the exception of the llvm backend)
* It automatically detects the backends available in the host

Summary of changes:
* [x] Removed all extra logic for test creation in emscripten test generator
* [x] Removed all extra logic for test creation in the wasi test generator
* [x] Divided wasmer wast test in different files (under `tests/custom/`)
* [x] Refactored trap asserts using the wast format
* [x] Improved the build script by adding emscripten and wasi test generators (separated from build)
* [x] Improved WASI errors to be Rust error types (by deriving from `thiserror::Error`)
* [x] Fixed toolchain error creation
* [x] Removed leaking of testing logic from generators into build
* [x] Refactored import tests to be compatible with multiple backends
* [x] Improved wasmer binary to work even when no backends are set (useful for testing without any backend available).
* [x] Removed assumption that Cranelift will be the default in the `wasmer` binary
* [x] Migrated middleware

# Review

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


Co-authored-by: Syrus <me@syrusakbary.com>
Co-authored-by: Syrus Akbary <me@syrusakbary.com>
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.

2 participants