-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Cargo check does not check doctests #6424
Comments
This is blocked on the lack of upstream (rustdoc) support for checking doc examples. I had a go looking, but I couldn't find an upstream issue tracking this. |
Rustdoc implemented |
Anyone tempted to have a crack at integrating it? |
I would like to work on it, if nobody beats me to it. Can look into it next week or so. |
There is a PR open to add this (#8859). I don't think |
@ehuss --check isn't the same either - that checks the crate itself, not any of its doctests. |
I wonder if it makes sense to add an option to forward options to rustc verbatim, so you could pass |
Ah, I misunderstood what |
It seems like Being able to pass arbitrary flags is certainly an option, but it introduces a compatibility hazard when you want to change the way rustdoc behaves in the future. For example, something like rust-lang/rust#50784 or rust-lang/rust#75341 could require fundamentally changing the way doctests work, and could introduce difficulties if users are passing flags that may not be compatible. It's certainly an option, but it would be nice if rustdoc could not leak its details too much. I would expect the combination of |
Per rust-lang/cargo#6424, `cargo check` doesn't actually run the doctests. So we need `cargo test` for this to work.
Per rust-lang/cargo#6424, `cargo check` doesn't actually run the doctests. So we need `cargo test` for this to work.
Of note, the issue requests $ cargo check --all-targets to check doctests. Except doctests aren't quite a target. This shows up in It would be interesting to find an alternative model that works for all of our needs but the trickiest need might be compatibility. The other angle to this is mental models. As I said, doctests aren't quite a target. At least for myself, I've so tightly coupled them in my mind to an "extra" that testing does that I would find it odd to see a |
cargo check —all ?
…On Mon, 30 Oct 2023 at 21:50, Ed Page ***@***.***> wrote:
Of note, the issue requests
$ cargo check --all-targets
to check doctests.
Except doctests aren't quite a target. This shows up in cargo test where cargo
test --all-targets doesn't test doctests, instead requiring a separate
--doc flag which is mutually exclusive with build target flags. Following
this model, we would at best support cargo check --doc which might still
have some usefulness.
It would be interesting to find an alternative model that works for all of
our needs but the trickiest need might be compatibility.
The other angle to this is mental models. As I said, doctests aren't quite
a target. At least for myself, I've so tightly coupled them in my mind to
an "extra" that testing does that I would find it odd to see a cargo
check --doc flag (what does that even mean?). I think I'd more associate
it with cargo doc but unsure what that would look like (turn the errors
into diagnostics that are controllable?).
—
Reply to this email directly, view it on GitHub
<#6424 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGEJCGCXVCQUARQ2DZQ2TLYCAHIXAVCNFSM4GKCNI7KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCNZYGYYDCOBQGUZQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Problem
The command
cargo check --all-targets
does not compile doctests.Steps
cargo check --all-targets
, which should "Check all targets" according to--help
Notes
I suspect this has been known for some time (#4592 (comment)). Combined with #5242, this means that there is no way to check the code in a doctest without running it (and using
cargo test
). From memory, I think that's because doctests are very different from all other code and tests, but I think it's surprising enough (I keep re-discovering this even now!) that it should probably be fixed.Output of
cargo version
:cargo 1.32.0-nightly (b3d0b2e54 2018-11-15)
The text was updated successfully, but these errors were encountered: