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

cfg(test) is not set during doctests #45599

Closed
sunjay opened this issue Oct 28, 2017 · 15 comments
Closed

cfg(test) is not set during doctests #45599

sunjay opened this issue Oct 28, 2017 · 15 comments
Labels
A-doctests Area: Documentation tests, run by rustdoc C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@sunjay
Copy link
Member

sunjay commented Oct 28, 2017

From @sunjay on October 27, 2017 21:23 (Moved from rust-lang/cargo)

It might be that I'm just not understanding how something works, so please let me know if that is the case!

Since doctests are tests, I expect that #[cfg(test)] would work and cfg!(test) would return true. This works during unit tests, but during doctests, both assumptions are wrong.

The reason I need this is because I have some code (not in doctests) that manages windows (like GUI windows) that I need to not run during any tests. I think this is a cargo problem because the attributes work consistently, I believe it's just cargo that doesn't set the attribute. If there is a workaround I can use, I would really appreciate it. :) (EDIT: My workaround is at the bottom.)

Here's some code that reproduces the problem: (Run with cargo test)

#[test]
fn cfg_test() {
    // This will pass, as it should
    assert!(cfg!(test))
}

/// ```rust
/// # fn main() {
/// // This will fail, even though we *are* in a test
/// assert!(cfg!(test), "cfg!(test) was false");
/// # }
/// ```
pub fn cfg_test2() {}

/// ```rust
/// # fn main() {
/// #[cfg(test)]
/// fn foo() {}
/// #[cfg(not(test))]
/// fn foo() {panic!("cfg(test) was not set")}
/// foo();
/// # }
/// ```
pub fn cfg_test3() {}

This produces the following output:

$ cargo test
    Finished dev [unoptimized + debuginfo] target(s) in 0.0 secs
     Running target/debug/deps/foo-3890a4e1c4133892

running 1 test
test cfg_test ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

   Doc-tests foo

running 2 tests
test src/lib.rs - cfg_test3 (line 15) ... FAILED
test src/lib.rs - cfg_test2 (line 7) ... FAILED

failures:

---- src/lib.rs - cfg_test3 (line 15) stdout ----
	thread 'rustc' panicked at 'test executable failed:

thread 'main' panicked at 'cfg(test) was not set', src/lib.rs:6:10
note: Run with `RUST_BACKTRACE=1` for a backtrace.

', /checkout/src/librustdoc/test.rs:315:16
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- src/lib.rs - cfg_test2 (line 7) stdout ----
	thread 'rustc' panicked at 'test executable failed:

thread 'main' panicked at 'cfg!(test) was false', src/lib.rs:3:0
note: Run with `RUST_BACKTRACE=1` for a backtrace.

', /checkout/src/librustdoc/test.rs:315:16


failures:
    src/lib.rs - cfg_test2 (line 7)
    src/lib.rs - cfg_test3 (line 15)

test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out

error: test failed, to rerun pass '--doc'

In both cases, since cfg(test) is not set, the test fails.


This is a temporary workaround you can use if you are in need of this functionality:

In Cargo.toml, add the following:

[features]
# The reason we do this is because doctests don't get cfg(test)
# See: https://github.com/rust-lang/cargo/issues/4669
test = []

Then run with cargo test --features "test".

This adds a feature called "test" to tests when they run. This works for both unit tests and doctests.

Then, to test for it, use the following:

#[cfg(any(feature = "test", test))]

You could also just go with the following, since the feature is set for all kinds of tests.

#[cfg(feature = "test")]

Copied from original issue: rust-lang/cargo#4669

@Mark-Simulacrum Mark-Simulacrum added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Oct 30, 2017
@jonhoo
Copy link
Contributor

jonhoo commented Nov 30, 2017

I may be mistaken here, but I believe doctests are treated as integration tests (i.e., not unit tests), and consider the crate under test external. This is supported by the fact that you must manually add extern crate <cratename>; if you extern any other crates in the doctest. Doctests (like integration tests) also cannot access private functions, or otherwise reach into the internals of the crate. The test attribute is only set for the compilation unit that holds the test, which in this case is not the crate under test, but rather the test harness itself.

Now, I do believe there is a bug here, and that is that test should technically be set inside the doctest code itself, since the doctest is running in test code. However, the crate under test should not be compiled with test, as the original post above requests.

That all said, I have also wanted the ability to have the crate under test behave differently in some way when it is run in "test mode", even when used as an extern crate, so I'm sympathetic with @sunjay's complaint. It's pretty common to work around this by adding a special test configuration parameter or feature to the crate under test, but this feels like a hacky workaround, as this toggle can then also be used by other consumers of the library. It is more okay for integration tests and doctests to reach into the crate under test than it is for some random other crate.

@sunjay
Copy link
Member Author

sunjay commented Nov 30, 2017

Maybe the answer is to have a specific doctest attribute so that we can choose what to include in either. My current hacky solution (adding a "test" feature), is only inconvenient because you have to manually pass it in when you run tests. Otherwise it works great and accomplishes exactly what I want. If there was a cfg flag I could use specifically to check for doctests, there would be no problem.

With that, unit tests could remain unit tests, and we could do more specialized behavior in doctests when we need to. Adding an attribute like that isn't a breaking change either so that's great too.

Note: if we add this, we should probably have something for tests that aren't unit tests or doctests. The tests in the separate tests directory could probably benefit from their own cfg flag.

@fschutt
Copy link
Contributor

fschutt commented Mar 25, 2018

It would be great if a #[cfg(doctest)] was added or #[cfg(test)] would work for doc tests. The workaround is simply annoying. I stumbled on this because I wanted to create a mock rendering function that is only available in doc-tests.

@dvdplm
Copy link

dvdplm commented Jul 3, 2018

+1 for a #[cfg(doctest)] – the workarounds are very unsatisfying.

@D4nte
Copy link

D4nte commented Aug 16, 2018

I am facing similar situation for the tests folder. I have code that provide me with helpers to the tests folder. I would like this code to only compile under #[cfg(test)] as it's only meant for test.

@briansmith
Copy link
Contributor

Doctests are integration tests so any solution for this problem should work for integration tests too.

I agree that we need a way to enable features that are available (only) to integration tests.

@paulvt
Copy link

paulvt commented Feb 5, 2019

Agreed. I am writing a crate that manages some devices via libusb. I would prefer if it wasn't necessary for me to connect a device to be able to perform doctests; the same actually also holds for the integration tests.

(If somebody known solutions for these kinds of issues and common ways to mitigate them, please let me know in private. Thanks in advance!)

@asomers
Copy link
Contributor

asomers commented Nov 16, 2019

My problem is quite similar. I maintain https://github.com/asomers/mockall , which is normally only used as a dev-dependency. So some of my examples won't really do anything unless cfg(test) is set. Ideally I'd be able to run the doc tests both ways: both with and without cfg(test).

bgianfo pushed a commit to bgianfo/rust-run-down that referenced this issue Dec 2, 2019
cfg(test) is no longer set during doctests
See: rust-lang/rust#45599
@ggriffiniii
Copy link

I would also point out that this causes the most prominent use of the doc-comment crate to seemingly not work for users. Each of the top 5 most popular crates (lazy_static, regex, aho-corasick, byteorder, num_cpus) are all using doc-comment behind a #[cfg(test)] conditional. Meaning that they intend to check their README.md files, but in fact are not. These crates are maintained by some of the best members of the rust community indicating me me that the current behavior is not what even experienced rust engineers expect.

I only discovered this issue after trying to use doc_comment to test my README.md file, trying to figure out why it wasn't working. Comparing my code to the examples in those crates above, failing to see what I was doing wrong. Eventually compiling those crates only to discover they also are not testing their README.md files, bisecting older versions of the rust compiler and doc_comment crate to discovery that this seemingly never worked.

@sunjay
Copy link
Member Author

sunjay commented Dec 13, 2019

I should note that my usecase will be pretty much addressed by the stabilization of #[cfg(doctest)]. This issue can probably be closed once that is released on stable Rust. (see next comment)

@sunjay
Copy link
Member Author

sunjay commented Dec 13, 2019

Ah actually it turns out I misunderstood what was added. The cfg(doctest) is only used when collecting doctests. It isn't used when compiling the code (:cry:).

This does not resolve this issue then. We would need cfg(doctest) to be added when compiling doctests for me to fully remove the --features "test" hack I've been using so far.

@GuillaumeGomez
Copy link
Member

@sunjay I think we actually should set it when compiling as well. I opened #67295 to keep track of it.

@sunjay
Copy link
Member Author

sunjay commented Dec 14, 2019

Thank you! I agree. I was initially very confused about the behaviour.

@erayerdin
Copy link

erayerdin commented Jan 7, 2020

I just would like to point out this thread from official user forum since it's settled that this is an issue affecting integration tests as well. Just in case.

ubnt-intrepid added a commit to ubnt-intrepid/rye that referenced this issue Apr 8, 2020
@jyn514 jyn514 added the A-doctests Area: Documentation tests, run by rustdoc label Nov 4, 2020
gustavgransbo pushed a commit to gustavgransbo/networkg that referenced this issue Dec 30, 2020
The bindings module does not play nicely with
cargo test. Fortunatley, it's functionality
is tested with pytest, and the module is not
required when testing the core module.

The test suite can now be ran with:
`cargo test --features "test"`

Workaround for: PyO3/pyo3#340 which allows for doctests.
Source of workaround: rust-lang/rust#45599
@jyn514
Copy link
Member

jyn514 commented Jul 2, 2021

Closing in favor of #67295, this is covered by cfg(doctest). If you need doctest to behave the same as test, you can use cfg(any(test, doctest)).

@jyn514 jyn514 closed this as completed Jul 2, 2021
joergroedel pushed a commit to coconut-svsm/svsm that referenced this issue Mar 13, 2023
In 'cargo test' environments, the SvsmAllocator must not be made the
global_allocator. However, the conditional setting of this attribute depends
on the 'test' configuration predicate being set in 'cargo test' environments.

This seems to not be working properly for documentation tests ([1]), and it's
similar for the alternative 'doctest' predicate ([2]).

As documentation tests currently don't play an important role for the project,
just disable them alltogether until the cargo community has settled on a
suitable alternative.

[1] rust-lang/rust#45599
[2] rust-lang/rust#67295

Signed-off-by: Nicolai Stange <nstange@suse.de>
00xc added a commit to 00xc/svsm that referenced this issue Sep 13, 2023
Our custom bare-metal allocator (`SvsmAllocator`) does not work during
tests, as they run as regular userspace binaries. Thus, we must not
set it as the global Rust allocator. To do this, we need a
compile-time directive that lets us know we are building doctests and
not the regular binary (`cfg(doctests)`), but sadly it is not
properly set during test compilation(see rust-lang/rust#45599,
rust-lang/rust#67295 and coconut-svsm#93).

In order to bypass this, set the compile time flag ourselves via the
RUSTFLAGS environment variable so that tests work. Also add the new
invocation to the `test` target in the Makefile.

Fixes: coconut-svsm#93
Fixes: 8cdea8e ("Cargo manifest: disable doctests")
Signed-off-by: Carlos López <carlos.lopez@suse.com>
00xc added a commit to 00xc/svsm that referenced this issue Sep 13, 2023
Our custom bare-metal allocator (`SvsmAllocator`) does not work during
tests, as they run as regular userspace binaries. Thus, we must not
set it as the global Rust allocator. To do this, we need a
compile-time directive that lets us know we are building doctests and
not the regular binary (`cfg(doctests)`), but sadly it is not
properly set during test compilation(see rust-lang/rust#45599,
rust-lang/rust#67295 and coconut-svsm#93).

In order to bypass this, set the compile time flag ourselves via the
RUSTFLAGS environment variable so that tests work. Also add the new
invocation to the `test` target in the Makefile.

Fixes: coconut-svsm#93
Fixes: 8cdea8e ("Cargo manifest: disable doctests")
Signed-off-by: Carlos López <carlos.lopez@suse.com>
00xc added a commit to 00xc/svsm that referenced this issue Sep 13, 2023
Our custom bare-metal allocator (`SvsmAllocator`) does not work during
tests, as they run as regular userspace binaries. Thus, we must not
set it as the global Rust allocator. To do this, we need a
compile-time directive that lets us know we are building doctests and
not the regular binary (`cfg(doctests)`), but sadly it is not
properly set during test compilation (see rust-lang/rust#45599,
rust-lang/rust#67295 and coconut-svsm#93).

In order to bypass this, set the compile time flag ourselves via the
RUSTFLAGS environment variable so that tests work. Also add the new
invocation to the `test` target in the Makefile.

Fixes: coconut-svsm#93
Fixes: 8cdea8e ("Cargo manifest: disable doctests")
Signed-off-by: Carlos López <carlos.lopez@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doctests Area: Documentation tests, run by rustdoc C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests