-
Notifications
You must be signed in to change notification settings - Fork 68
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 support for doctests #52
Conversation
Freax13
commented
Apr 17, 2020
- detect doctests as tests
Thanks for the PR! Could you clarify how you're using doc tests with bootimage? As far as I know, doc tests don't work in combination with custom test frameworks. |
first of all I should note that this isn't working rn because of a bug in cargo (rust-lang/cargo#8094) i'm using something similar to doctests don't work with custom test frameworks. rustdoc basically just individually wraps the code for each test in a main method (unless you provide your own) and compiles it. example from my code: /// acquire the lock
/// ```
/// # #![cfg_attr(target_os = "none", no_std, no_main)]
/// # lemon_std::entry_point!(test);
/// # async fn test() {
/// # use lemon_std::sync::SpinMutex;
/// let lock = SpinMutex::new("Foo");
/// lock.lock();
/// # lemon_std::arch::exit(true);
/// # }
/// # main();
/// ```
pub fn lock(&self) -> SpinMutexGuard<'_, D> { |
Thanks for the clarification! I wasn't aware of the
So they're basically integration tests with disabled test harness (as described in https://os.phil-opp.com/testing/#no-harness-tests)?
I think it might make sense to delay merging this PR until this cargo bug is resolved and we can actually try whether this PR works. It might be also a good idea to add an automated test for this then to check the behavior on CI and ensure that we don't break it on future changes. Would that be okay with you? |
pretty much, yeah
rust 1.43 just got released, so the latest cargo on nightly finally contains the fix. do you want me to write an example for doctests (probably something similar to test-basic-boot)? |
? |
Sorry for the delay! Yes, I think some example that is tested on CI would be great. |
There are a few warnings for the new doctest test, otherwise this looks very good. |
that should have got rid of the warning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks a lot!
Published as v0.7.10 |