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

Decide (and document) whether uefi-test-runner should work outside the QEMU env #553

Closed
nicholasbishop opened this issue Nov 10, 2022 · 4 comments
Assignees

Comments

@nicholasbishop
Copy link
Contributor

Currently the uefi-test-runner kinda sorta runs on real hardware. I just tried it on a recent Thinkpad, and it drew the usual blue/orange colors to the screen, some output flashed by, and it panicked on something related to serial.

The test runner currently has as lot of conditionals around things like "is this protocol supported?" and will often continue gracefully if something is missing. This has the advantage of making the runner more likely to work on real hardware, but it has the major disadvantage of making it easier to miss an unexpected error when running in the usual QEMU setup. And the more minor disadvantage of making the code a little more complicated.

This comes up from time to time in code review (recent example), so it would be good to make a decision one way or the other and document it.

@nicholasbishop
Copy link
Contributor Author

My current feeling is that we should probably just make the test runner expect that it is running in our special QEMU setup (cargo xtask run), since even if we wanted to support real hardware it's easy to make mistakes without having a regular test on real hardware. Interested to hear others' thoughts :)

@nicholasbishop nicholasbishop mentioned this issue Nov 10, 2022
2 tasks
@phip1611
Copy link
Contributor

The test runner currently has as lot of conditionals around things like "is this protocol supported?" and will often continue gracefully if something is missing. This has the advantage of making the runner more likely to work on real hardware, but it has the major disadvantage of making it easier to miss an unexpected error when running in the usual QEMU setup. And the more minor disadvantage of making the code a little more complicated.

Thanks for the initiative. I'm in favor of tighten the bond between the test runner and QEMU/OVMF. This will help us find bugs and errors.

@timrobertsdev
Copy link
Contributor

Something to potentially consider: OvmfPkg in edk2 is missing some protocols that we would be unable to test if we're using the stock OVMF image for QEMU. IIRC I ran into this while implementing DebugSupport or a supporting protocol.

@nicholasbishop nicholasbishop self-assigned this Nov 19, 2022
@nicholasbishop
Copy link
Contributor Author

Something to potentially consider: OvmfPkg in edk2 is missing some protocols that we would be unable to test if we're using the stock OVMF image for QEMU.

For things that OVMF doesn't yet support we probably just don't have a realistic way to test it in CI at all. However, we could add separate programs to https://github.com/rust-osdev/uefi-rs/tree/main/uefi-test-runner/examples that can be manually tested on real hardware or some other env.

nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this issue Jan 14, 2023
Per rust-osdev#553, we've made the test runner
always expect to be running in the special QEMU env, so we don't need to have a
separate "works with any env" disk test from the more detailed "known disk"
test. So, move `known_disk.rs` -> `mod.rs`.
phip1611 pushed a commit to nicholasbishop/uefi-rs that referenced this issue Jan 22, 2023
Per rust-osdev#553, we've made the test runner
always expect to be running in the special QEMU env, so we don't need to have a
separate "works with any env" disk test from the more detailed "known disk"
test. So, move `known_disk.rs` -> `mod.rs`.
nicholasbishop added a commit that referenced this issue Jan 22, 2023
Per #553, we've made the test runner
always expect to be running in the special QEMU env, so we don't need to have a
separate "works with any env" disk test from the more detailed "known disk"
test. So, move `known_disk.rs` -> `mod.rs`.
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

No branches or pull requests

3 participants