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

cargo test --examples does not work exactly as advertised #5177

Closed
infinity0 opened this issue Mar 14, 2018 · 10 comments · Fixed by #5867
Closed

cargo test --examples does not work exactly as advertised #5177

infinity0 opened this issue Mar 14, 2018 · 10 comments · Fixed by #5867
Labels

Comments

@infinity0
Copy link
Contributor

cargo test --help says:

    --example NAME ...           Check that the specified examples compile
    --examples                   Check that all examples compile

but what actually happens when you run cargo test --examples is that the examples are compiled and tested just like any other test, and the binary written to target/debug/examples/$NAME is a test-runner rather than the actual example binary.

This may seem harmless but it causes a surprising difference in behaviour when e.g. running cargo test && target/debug/examples/$NAME vs cargo test --examples && target/debug/examples/$NAME.

Uncovered by a proposed addition to #5146.

bors added a commit that referenced this issue Mar 21, 2018
Stricter need_dev_deps behaviour

The previous PR (#5012) contained an unnecessary work-around for behaviour of `--all-targets` that was misunderstood. This PR removes that work-around and adds some tests and comments to clarify the behaviour for future contributors, which may help to make easier a future fix for #5177 and #5178.
@ehuss
Copy link
Contributor

ehuss commented Apr 10, 2018

I've stumbled across this while reworking profile selection.

@alexcrichton / @matklad what is the intended behavior with examples for cargo test? Why does cargo test bother building examples (without the test profile)?

In particular, this line of code forces examples to be included with the test_deps profile:

profile: if t.is_example() { dep } else { profile },

I'm thinking that examples probably shouldn't be built at all with cargo test unless you have --example, --examples, or --all-targets, in which case they should be built with the "test" profile.

However, they have been built by default (as a "dependency") since at least Rust 1.0, so it seems to have been around a long time. Perhaps the intent is to allow integration tests to run examples (similar to the reasoning all binaries are built)?

@matklad
Copy link
Member

matklad commented Apr 10, 2018

Hm, I think the intention is indeed to check that examples actually compile, and to make them available to integration tests.

So, cargo test —examples should work as cargo test, and the current behavior is indeed a bug

@matklad
Copy link
Member

matklad commented Apr 10, 2018

As for rational for building examples when testing, it would be nice for CI to fail if you made a change to your API but forgot to update examples.

@alexcrichton
Copy link
Member

I think @matklad hit the nail head on

@ehuss
Copy link
Contributor

ehuss commented Apr 10, 2018

Ah, that makes sense. There is a small number of packages on crates.io that have test code within examples. I assume changing it is OK since it was documented to only try to compile?

Packages with tests in examples.
alga-0.5.1
algebra-0.2.0
algs4-0.7.0
celly-0.6.0
iron-test-0.5.0
jsonnet-rs-0.5.0
nickel-0.9.0
parrot-0.1.0
plague-0.6.3
rustcov-0.0.0
simple-munin-plugin-0.1.0
topdown-rs-0.3.3
yamlette-0.0.2

@matklad
Copy link
Member

matklad commented Apr 10, 2018

Yeah, I think it’s fine to change behavior here. I am wondering though, do both test —examples and test —example foo try to compile examples in test-mode?

@ehuss
Copy link
Contributor

ehuss commented Apr 10, 2018

do both test —examples and test —example foo try to compile examples in test-mode?

Correct.

@alexcrichton
Copy link
Member

Oh er wait sorry, I think I misread! I would be pretty hesitant about compiling examples in --test mode. That I think runs a pretty high risk of a breaking change and also is somewhat outside the spirit of the examples (binary programs that can be run).

We may be able to make such a change but I'd want to proceed quite cautiously and have necessary opt-outs and such stabilized if necessary

@ehuss
Copy link
Contributor

ehuss commented Apr 10, 2018

@alexcrichton I meant changing it the other way, so that examples are not compiled in --test mode. The current behavior is inconsistent. cargo test compiles w/o test, cargo test --examples compiles with test. I was saying changing it so examples are always compiled w/o test (to match the documentation).

@alexcrichton
Copy link
Member

Oh sorry I seems to be misunderstanding/misreading a lot! @ehuss that sounds perfect to me

ehuss added a commit to ehuss/cargo that referenced this issue Aug 5, 2018
As part of rust-lang#5464 I forgot to update the help text.

Closes rust-lang#5177
bors added a commit that referenced this issue Aug 5, 2018
Fix test --example docs.

As part of #5464 I forgot to update the help text.

Closes #5177
@bors bors closed this as completed in #5867 Aug 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants