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

Features and tests #405

Closed
m-mueller678 opened this issue Nov 21, 2023 · 1 comment · Fixed by #407
Closed

Features and tests #405

m-mueller678 opened this issue Nov 21, 2023 · 1 comment · Fixed by #407

Comments

@m-mueller678
Copy link

It looks like the tests currently don't do anything. I added a panic inside the #[cfg(feature = "bios")] block inside run_test_kernel_internal and it still passes even with the bios feature explicitly enabled. I think this is because features do not get passed on to the test runner crate.
I fixed this by adding a feature dependency, but I'm not entirely sure if that is the proper way to do it. The tests take considerably longer now, so I think that's good.

[features]
default = ["bios", "uefi"]
bios = ["dep:mbrman","bootloader_test_runner/bios"]
uefi = ["dep:gpt","bootloader_test_runner/uefi"]
@phil-opp
Copy link
Member

Thanks a lot for reporting! Looks I made the mistake to disable the features in #351. I needed to remove the features because of a bug in cargo that led to errors at publish time: rust-lang/cargo#12225 . However, I should have enabled these features unconditionally instead of disabling them. I'll prepare a fix!

phil-opp added a commit that referenced this issue Dec 27, 2023
The test runner was accidentally disabled in #351, in an attempt to fix the publish errors introduced by #304 (caused by a bug in cargo: rust-lang/cargo#12225). As a result, the test runner became a no-op as neither the bios nor the uefi features were enabled.

This commit fixes the issue by enabling both features by default. Once the cargo bug is fixed, we might want to switch back to the feature configuration added of #304.

Fixes #405
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 a pull request may close this issue.

2 participants