rustdoc: don't run doctests on private items (by default) #54438
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
QuietMisdreavus
force-pushed
the
private-doctests
branch
from
9e8da49
to
bc41a7a
Compare
|
Whoops, left a "todo" in there that was already addressed. I folded the change into the proper commit and force-pushed. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I think this will need a relatively long warning period or a edition (not 2018, it's too late) to make this change. Silently not running test cases anymore seems like a bad idea. |
|
I thought tests were not run on private items. This way of working seems more logical to me and seems to fit the expected behavior better. However, just like @Mark-Simulacrum said, it'd be nice to wait a bit before merging a behavior change. Once done, r=me. |
|
I agree with @Mark-Simulacrum, and would go even further and not do this change at all: Given that we are saying:
I would assume that doctests would also run for private items (even if the documentations is not visible in the end). |
|
I can appreciate that reasoning, and i'm willing to scrap this PR and close the linked issue as "won't fix", but there's one wrinkle that doctests have that unit tests do not: doctests are still always run on the public crate. It's impossible to actually test the item being documented properly, unless you want to include some demo code that doesn't touch it. There's no reason to actually write a doctest for a private item, and to leave such an idea open is a bit of an oversight. I've seen several comments requesting some kind of functionality to allow it because of that same parallel @TimNN mentions. |
|
Ping from triage! What is the status of this PR? I assume this needs some discussion from @rust-lang/rustdoc, so marking as such. |
TimNN
added
S-waiting-on-team
|
Please don't do this. It's a bad idea to leave some tests untested by default. |
|
Based on discussion in this thread, i'm going to close this PR, and motion to close the linked issue as well. Such a change is too controversial to introduce, especially after the current behavior has been present for so long. |
apiraino
removed
the
S-waiting-on-team
fixes #30094
Currently, rustdoc will scan the whole crate for doctests, regardless of whether the docs it finds are actually being shown in documentation, or actually useful docs. This PR changes the doctest collection scan to account for module visibility. Public re-exports of items will also pull in their doctests, if their original location was private. I added a check to make sure that items are not tested twice, to make sure that re-exporting an item into multiple places (say, if the original location is private, you export it into its "canonical" location, and you also include it in a prelude).
To continue running all tests, you can pass
--document-private-itemsto rustdoc, like this:While typing this up, i noticed that impls inside non-module scope also get skipped here, though running private tests will catch it. If desired, i can try and add it, though i'm not sure how severe it is to be missing.