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

Remove hard coded Wasm paths in abitest & hello_world #578

Merged
merged 2 commits into from
Feb 7, 2020

Conversation

blaxill
Copy link
Contributor

@blaxill blaxill commented Feb 7, 2020

Removes hard coded Wasm paths in abitest & hello_world, and builds the Wasm modules during test invocation.

The Wasm files are outputted to a temporary directory and cargo doesn't seem to use artefacts from the host target dir so this does currently cause the tests to do a full rebuild of their Wasm files.

Checklist

  • Pull request affects core Oak functionality (e.g. runtime, SDK, ABI)
    • I have written tests that cover the code changes.
    • I have checked that these tests are run by Cloudbuild
    • I have updated documentation accordingly.
    • I have raised an issue to
      cover any TODOs and/or unfinished work.
  • Pull request includes prototype/experimental work that is under
    construction.

#541

@@ -440,7 +440,7 @@ fn test_say_hello() {
simple_logger::init().unwrap();

let configuration = oak_tests::test_configuration(
&[(MODULE_CONFIG_NAME, WASM_PATH)],
build_wasm().expect("failed to build wasm modules"),
LOG_CONFIG_NAME,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also update the text around l.469-470 which is out of sync?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

const BACKEND_WASM_NAME: &str = "abitest_1_backend.wasm";

fn build_wasm() -> std::io::Result<Vec<(String, Vec<u8>)>> {
let mut frontend = oak_tests::compile_rust_to_wasm(FRONTEND_MANIFEST)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add an extra module_name parameter to compile_rust_to_wasm and do the full path creation inside it? (So this would be oak_tests::compile_rust_wasm(FRONTEND_MANIFEST, FRONTEND_WASM_NAME) with no .pushing needed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@blaxill blaxill merged commit 408e06a into project-oak:master Feb 7, 2020
@blaxill blaxill mentioned this pull request Feb 12, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants