Set cfg(test) when rustdoc is running with --test option

[GuillaumeGomez]

Following a discussion on twitter, I proposed this change. What do you think about it?

r? @QuietMisdreavus

cc @BurntSushi

[BurntSushi]

I'm not sure what the rustdoc team procedures are here, but you might want to consider this as a normal public API addition. That is, if you accept this change and it makes it to stable Rust, then removing this would be a breaking change AIUI. For example, one thing I think this would allow you to do is to write doc tests using code that is gated behind a cfg(test).

[ehuss]

There is some discussion about this in #45599.

[QuietMisdreavus]

(Sorry about closing this, i misclicked the button when typing my initial comment. 😓)

This looks like it's not the same as #45599 - this is about setting cfg(test) when collecting doctests, not when compiling them. This is probably fine, since there's been a slight pattern of having non-public doctests to test when something should not compile, or (in the case of the linked twitter thread) to check the doctests of the README without directly including it in the crate's docs. I originally learned about this when trying to block doctests on private items (#54438). It looks like this is a way to properly support that.

[ehuss]

Ah, sorry for misinterpreting.

[GuillaumeGomez]

So is it ok to merge it?

[QuietMisdreavus]

This seems fine to me, though i'm a little worried that there will be some subtle ramifications that we haven't thought about. I'd like someone from @rust-lang/docs or @rust-lang/rustdoc to see whether i'm overthinking this, but in the meantime there doesn't seem to be much harm from landing it into nightly for now - we have plenty of time to revert it if things go awry.

[QuietMisdreavus]

@bors r+

[bors]

📌 Commit 1ce17e6392a0bcb4d99cd2e8cd6249d015f43e6d has been approved by QuietMisdreavus

[ollie27]

I'm a bit worried that doctests on items marked #[cfg(not(test))] will no longer be run but I don't know how common that would be in the wild.

Also can we add a rustdoc-ui test like the following to test this behavior?

// compile-pass
// compile-flags:--test
// normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR"

/// this doctest will be ignored:
///
/// ```
/// assert!(false);
/// ```
#[cfg(not(test))]
pub struct Foo;

/// this doctest will be tested:
///
/// ```
/// assert!(true);
/// ```
#[cfg(test)]
pub struct Foo;

[GuillaumeGomez]

@ollie27: Hum good point, they won't be tested indeed. I don't see it as an issue since there are workarounds though. I'll add the test shortly.

@bors: r-

[GuillaumeGomez]

Updated!

[GuillaumeGomez]

r? @ollie27 (since @QuietMisdreavus was ok with the changes, I guess it's up to you to validate the tests :p)

[ollie27]

@bors r+

[bors]

📌 Commit 459d677 has been approved by ollie27

[Centril]

@bors retry

yielding to r0llup

[bors]

⌛ Testing commit 459d677 with merge 3ae1afdb0a6df45f29f9829968419ff7fde0cc3a...

[Centril]

@bors retry

[bors]

⌛ Testing commit 459d677 with merge 180edc2...

[ollie27]

I should have checked before merging this but there are doctests in this repository that are behind #[cfg(not(test))] which we haven't been testing since this was merged. For example the entire of core:

rust/src/libcore/lib.rs

Lines 50 to 51 in f492693

	// This cfg won't affect doc tests.
	#![cfg(not(test))]

I've submitted a revert: #61199