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

Add the QEMU test invocation to make test #199

Merged
merged 5 commits into from
Jun 17, 2020
Merged

Conversation

jrvanwhy
Copy link
Collaborator

Prior to this PR, there was no easy way to run libtock_test in QEMU. It was run as part of the Travis CI checks, but not make test. This PR moves that functionality into the Makefile, so that make test runs the QEMU tests.

Main changes in this PR:

  1. Added a tock submodule pointing to the kernel version the tests are run against. This submodule is used to build Tock kernels for testing.
  2. Tweaked the README to clarify that the Tock submodule has its own license (although it matches libtock-rs' license).
  3. Added make setup-qemu, automatically run as part of make qemu, which compiles QEMU in a new build directory. This shouldn't touch the host system, although you need QEMU's dependencies for it to succeed.
  4. Tweaked test-runner to use the QEMU and Tock kernel built by make setup-qemu and make kernel-hifive.
  5. Added test-qemu-hifive to run libtock_test on an emulated hifive.
  6. Added test-qemu-hifive as a dependency of test so that make test runs both sets of tests
  7. Replaced make examples with a dependency on the examples, so that running parallel make jobs works correctly.

jrvanwhy added 3 commits June 12, 2020 14:43
Currently we use Alistair's fork of QEMU, as it has patches for HiFive support that upstream QEMU doesn't have. QEMU is installed into a `build/` directory that is in .gitignore so it doesn't touch the host system or dirty the repository.
…that kernel. Move QEMU tests to `make test` from the Travis config.

I also fixed an issue in test-runner where it breaks console echo. I don't fully understand why, but I needed to set QEMU's stdin to null rather than test-runner's stdin to avoid breaking console echo.

This should fix tock#198
Previously, QEMU's build in Travis was run with -j8. Now QEMU is built by `make setup`, so we should pass `-j8` to `make setup` instead.
Makefile Outdated Show resolved Hide resolved
I also switched to using `cargo clean` in the Makefile rather than manually removing `target/` and `Cargo.lock`, as that seems like a more correct and maintainable design.
@torfmaster
Copy link
Collaborator

I am a bit sceptical about adding test dependencies as git submodules. In my opinion git submoudules should be ultima ratio even for compile time dependencies. My favorite solution would be to publish binary releases of the kernel and retrieve them during the CI of tock. This would speed up the CI a lot.

@jrvanwhy
Copy link
Collaborator Author

I am a bit sceptical about adding test dependencies as git submodules. In my opinion git submoudules should be ultima ratio even for compile time dependencies. My favorite solution would be to publish binary releases of the kernel and retrieve them during the CI of tock. This would speed up the CI a lot.

Putting Tock into a submodule makes it easier to contribute. If someone needs to test libtock-rs with a newer Tock version, they can update the Tock submodule to that version very easily. If someone wants to modify Tock (e.g. to add a new syscall API) and test it with libtock-rs, they can easily do so by making those changes in the submodule. The submodule can be pointed at their fork of Tock.

On my Chromebook (which is far from the fastest machine around), it takes 17.4 seconds to build the Tock kernel. If I don't change the tock directory, subsequent builds take less than half a second. I don't think that amount of time is worth avoiding submodules, given the developer productivity benefits.

@torfmaster
Copy link
Collaborator

On my Chromebook (which is far from the fastest machine around), it takes 17.4 seconds to build the Tock kernel.

The issue here is not local build time, I am worried about the build times in CI which are much longer due to do limited resources. However, this particular PR does not change the fact that we check out and build the tock kernel in every build so my argument doesn't hold.

I am not sure to which extend we should prepare a "working environment" for a developer but if the benefits of having the kernel as a submodule outweighs the risks (complexity, coupling of the kernel to the userspace library) I am okay with doing so.

One little remark: I would probably give a hint to the users of the library that there are submodules to be initialized.

bors d+

@bors
Copy link
Contributor

bors bot commented Jun 17, 2020

✌️ jrvanwhy can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@jrvanwhy
Copy link
Collaborator Author

I changed the git clone command in the README to initialize submodules.

@jrvanwhy
Copy link
Collaborator Author

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 17, 2020

Build succeeded:

@bors bors bot merged commit 7bb207b into tock:master Jun 17, 2020
@jrvanwhy jrvanwhy deleted the qemu-tests branch June 17, 2020 17:59
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.

3 participants