-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
wasi-nn: adapt to new test infrastructure #7679
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abrown
force-pushed
the
nn-test-infrastructure
branch
6 times, most recently
from
December 15, 2023 01:06
8146ac4
to
7cc3488
Compare
This change adds new test programs for wasi-nn in a way fits in with the existing WASI test infrastructure. The code is not new, though: this reuses the wasi-nn `examples`, which are currently used by the `run-wasi-nn-example.sh` CI script. Eventually the examples will be removed in favor of these tests. Because wasi-nn's component model support is still in flight, this change also skips the generation of components for `nn_`-prefixed tests.
This testing-only module has code (i.e., `check_test!`) to check whether OpenVINO and some test artifacts are available. The test artifacts are downloaded and cached if not present, expecting `curl` to be present on the command line (as discussed in the previous version of this, bytecodealliance#6895).
Following the pattern of other WASI crates, this change adds the necessary infrastructure to run the `nn_*` files in `crates/test-programs` (built by `test-program-artifacts`). These tests are only run when two sets of conditions are true: - statically: we only run these tests where we expect OpenVINO to be easy to install and run (e.g., the `cfg_attr` parts) - dynamically: we also only run these tests when the OpenVINO libraries can be located and the model artifacts can be downloaded
abrown
force-pushed
the
nn-test-infrastructure
branch
from
December 15, 2023 01:34
7cc3488
to
c0c54d9
Compare
abrown
force-pushed
the
nn-test-infrastructure
branch
from
December 15, 2023 02:18
2815567
to
c1f8232
Compare
alexcrichton
approved these changes
Dec 15, 2023
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.
Thanks! This is replacing the preexisting wasi-nn test, right? Would it make sense to delete that from CI while here?
abrown
force-pushed
the
nn-test-infrastructure
branch
from
December 15, 2023 22:17
e8bca86
to
da86dc4
Compare
dhil
pushed a commit
to dhil/wasmtime
that referenced
this pull request
Dec 29, 2023
* wasi-nn: add test programs This change adds new test programs for wasi-nn in a way fits in with the existing WASI test infrastructure. The code is not new, though: this reuses the wasi-nn `examples`, which are currently used by the `run-wasi-nn-example.sh` CI script. Eventually the examples will be removed in favor of these tests. Because wasi-nn's component model support is still in flight, this change also skips the generation of components for `nn_`-prefixed tests. * wasi-nn: add `testing` module This testing-only module has code (i.e., `check_test!`) to check whether OpenVINO and some test artifacts are available. The test artifacts are downloaded and cached if not present, expecting `curl` to be present on the command line (as discussed in the previous version of this, bytecodealliance#6895). * wasi-nn: run `nn_*` test programs as integration tests Following the pattern of other WASI crates, this change adds the necessary infrastructure to run the `nn_*` files in `crates/test-programs` (built by `test-program-artifacts`). These tests are only run when two sets of conditions are true: - statically: we only run these tests where we expect OpenVINO to be easy to install and run (e.g., the `cfg_attr` parts) - dynamically: we also only run these tests when the OpenVINO libraries can be located and the model artifacts can be downloaded * ci: install OpenVINO for running wasi-nn tests prtest:full * vet: certify the `wasi-nn` crate * ci: remove wasi-nn test script
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a modern refactoring of #6895 to make wasi-nn use the test infrastructure that other WASI proposals are using (i.e.,
crates/test-programs
). This is only the beginning: in follow up PRs, wasi-nn could gain unit tests, the existing examples could be removed, more kinds of tests could be added, etc. This will be easier in the future, though, once this infrastructure is in place for thewasmtime-wasi-nn
crate.