-
Notifications
You must be signed in to change notification settings - Fork 162
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
"Production-Ready" Template Project #770
Comments
@blitz If you have time, I'd also love to hear your thoughts - I think you might also have experience with that in the lanzaboote project. |
For example, the bootloader crate also refrains from writing unit tests. Probably for the reason described above. https://github.com/rust-osdev/bootloader/blob/main/uefi/src/main.rs |
We can chat about it this week in person. :) |
My opinion is that unit tests should be self-contained and independent of the target system. So for target-specific crates such as UEFI applications, I would move all the platform-independent stuff to a separate crate, which can then be tested as usual without requiring any conditional compilation. For target-specific integration tests, you typically want to test them on an (emulated) target system directly. Rust doesn't really provide much support for this yet, unfortunately. I think the best solution is to build a custom test framework, e.g. using a crate such as I'm not a fan of using cfg-gates for testing since this makes it less obvious what you're really testing. For example, I think your example test at https://github.com/phip1611/rust-uefi-template/blob/2fde9ebfd9f1d43c5a90f7d0054efa050d1d55a8/src/lib.rs#L26-L29 effectively only verifies that there is a global allocator in Rust's standard library. |
I think you are correct that it's hard. I think there are two directions to attack the problem from:
And these are not mutually exclusive of course. I think the idea of a standalone template UEFI app repo is good, and better than our current template subdirectory in uefi-rs. Keeping it in a separate repo makes it easier to include things like basic github CI and potentially a simple workspace. I agree with phil-opp that using separate packages is good for testing; keep the testable-on-the-host stuff in one package and the depends-on-uefi stuff in another package. And I'm a proponent of the xtask pattern rather than a Makefile; Makefiles and shell scripts get hard to read very quickly IMO, and easy to make non-obvious mistakes when writing too. From the direction of improving the ecosystem, perhaps we should look at librarification of some of the code in uefi-rs used to run QEMU. Pretty much every UEFI project has some local code for finding OVMF (often just a hardcoded path, but we can do better), figuring out the args for QEMU, etc. uefi-run exists, but it's fairly restrictive even in the library. A new library with flexible code for launching QEMU could be very helpful for getting a new project up and running. |
Compiling a minimal hello-word UEFI application is easy. Setting up a productive development environment with a "run in QEMU" command and unit-tests is hard, however. Especially the last part is very challenging, as one has to mix the UEFI target with a standard target for test execution - rust runtime problems (panic handlers, global allocators), different compilation targets... I worked on something (https://github.com/phip1611/rust-uefi-template) that supports the simultaneous building and unit testing inside the same Cargo project.
(Either I'm totally dumb or this is indeed very hard).
Can you (@nicholasbishop, @GabrielMajeri but also the community) please tell me what you think? And if yes, could or should we integrate the template into the rust-osdev namespace? The README and the code comments should describe the relevant problems that I'm solving there..
Summary of the Problems
libtest
is pre-compiled and working on this target. Unit tests can't be compiled and executed for thex86_64-unknown-uefi
target - that would not work.x86_64-unknown-uefi
target#[panic_handler]
. This won't work for unit tests in a cargo test setup#[global_allocator]
As a consequence, it takes a few quirks to cope with that - and they are not very intuitive, I think.
https://github.com/phip1611/rust-uefi-template
PS: I just found that there is an open issue in cargo because of this problem: rust-lang/cargo#6784
The text was updated successfully, but these errors were encountered: