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

examples: move tests to be integration tests #1154

Merged
merged 1 commit into from
Jun 15, 2020
Merged

examples: move tests to be integration tests #1154

merged 1 commit into from
Jun 15, 2020

Conversation

daviddrysdale
Copy link
Contributor

Most of the tests for example Oak Applications just use the external
gRPC service definition for testing, and so can be external integration
tests rather than unit tests.

This does require an "rlib" target for the modules though, because of
rust-lang/cargo#6659

The trusted_information_retrieval example's tests included both unit
tests and integration tests, so is split. Along the way, make the
floating comparisons in the unit tests a bit more robust.

Fixes #1054.

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.

@daviddrysdale daviddrysdale marked this pull request as ready for review June 15, 2020 12:46
@daviddrysdale daviddrysdale requested a review from rbehjati June 15, 2020 12:46
Copy link
Contributor

@rbehjati rbehjati left a comment

Choose a reason for hiding this comment

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

Nice!

@@ -6,7 +6,7 @@ edition = "2018"
license = "Apache-2.0"

[lib]
crate-type = ["cdylib"]
crate-type = ["cdylib", "rlib"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is rlib added to make the modules available to the integration tests? How is oak/server/rust/oak_runtime/tests/integration_test.rs working without it? What is its magic?

Not so important, but perhaps oak/server/rust/oak_runtime/tests/integration_test.rs could be renamed to integration_tests.rs as well, to be consistent :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh! Just saw your description!

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 think the magic is that rlib is the default (and so not mentioned in oak_runtime/Cargo.toml; however, the examples need to be built for the Wasm target and that (I think) needs cdylib crate type.

Also, it looks like integration_test.rs is more canonical so I'll move to that in the examples.

Most of the tests for example Oak Applications just use the external
gRPC service definition for testing, and so can be external integration
tests rather than unit tests.

This does require an "rlib" target for the modules though, because of
rust-lang/cargo#6659

The trusted_information_retrieval example's tests included both unit
tests and integration tests, so is split.  Along the way, make the
floating comparisons in the unit tests a bit more robust.

Fixes #1054.
@daviddrysdale daviddrysdale merged commit 4c5c0f3 into project-oak:master Jun 15, 2020
@daviddrysdale daviddrysdale deleted the integration-test branch June 15, 2020 14:53
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.

Move Node tests to be integration tests not unit tests
3 participants