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 check --test foo compiles too much #4059

Closed
alexcrichton opened this issue May 16, 2017 · 4 comments
Closed

cargo check --test foo compiles too much #4059

alexcrichton opened this issue May 16, 2017 · 4 comments
Labels

Comments

@alexcrichton
Copy link
Member

From a local compilation:

$ cargo new foo
$ cd foo
$ mkdir tests
$ touch tests/a.rs
$ cargo check --test a -v
   Compiling foo v0.1.0 (file:///home/alex/code/foo)
     Running `rustc --crate-name foo src/lib.rs --crate-type lib --emit=dep-info,link -C debuginfo=2 -C metadata=6d607d3e3579f789 -C extra-filename=-6d607d3e3579f789 --out-dir /home/alex/code/foo/target/debug/deps -L dependency=/home/alex/code/foo/target/debug/deps`
     Running `rustc --crate-name a tests/a.rs --emit=dep-info,link -C debuginfo=2 --test -C metadata=e8ff428dccc81b86 -C extra-filename=-e8ff428dccc81b86 --out-dir /home/alex/code/foo/target/debug/deps -L dependency=/home/alex/code/foo/target/debug/deps --extern foo=/home/alex/code/foo/target/debug/deps/libfoo-6d607d3e3579f789.rlib`
    Finished dev [unoptimized + debuginfo] target(s) in 0.32 secs

Notably nothing was compiled with metadata as an output.

cc @nrc

@tbourvon
Copy link

After investigating this issue, it seems that the problem comes from this line in cargo_compile.rs:

targets.append(&mut propose_indicated_targets(pkg, tests, "test", Target::is_test, test)?);

This line adds integration tests that match the --test X (or --tests) flag to the list of targets to be built. However, the profile passed to that function (the last argument) is the test profile, which has check == false. Hence code gets generated.

This could be fixed by passing the check_test profile from PR #4039 when we are checking, and test otherwise.

@tbourvon
Copy link

tbourvon commented May 16, 2017

Should I take care of this issue in the same PR or should I open another one once the first is closed?

@alexcrichton
Copy link
Member Author

@burningmind oh feel free to take care of this issue in your PR! No need for a separate one.

ehuss added a commit to ehuss/cargo that referenced this issue Oct 15, 2017
- Add `--profile test` flag to `cargo check` that will enable checking of unit tests.
- `--tests` will implicitly enable checking of unit tests within the lib.
- Don't implicitly compile binaries when using just `--test` filters.
- Fix erroneously linking tests when run with `--test`.

Fixes rust-lang#3431, rust-lang#4003, rust-lang#4059, rust-lang#4330.
bors added a commit that referenced this issue Oct 30, 2017
Add unit test checking to `cargo check`

This is an extension of PR #4039, fixing #3431, #4003, #4059, #4330.  The fixes for #4059 can potentially be separated into a separate PR, though there may be some overlap.

The general gist of the changes:

- Add `--profile test` flag to `cargo check` that will enable checking of unit tests.
- `--tests` will implicitly enable checking of unit tests within the lib (checks the same targets as `cargo test`).  This affects the `check`, `test`, and `build` commands.
- `--benches` behaves similarly by using the same targets as `cargo bench`.
- Fix erroneously linking tests when run with `--test`.

There is one thing I did not do because I wanted more feedback on what others think the expected behavior should be.  What should the behavior of `--all-targets` be?  This patch does not (yet) make any changes to its behavior.  My initial thinking is that it should *add* a target of `--lib --bins --profile test`, but that essentially means the lib and bin targets will be checked twice (and thus any errors/warnings outside of `#[cfg(test)]` will appear twice, which may be confusing, and generally take twice as long to run).  I can add that, but I think it would require adding a new `All` variant to `CompileFilter` so that the code in `generate_targets` can detect this scenario.  I wanted feedback before making a more extensive change like that.  The downside of not adding it is that `--all-targets` will ignore unit tests (if you don't specify `--profile test`).

Summary of the profiles used with this patch:

Command                         | Lib               | Bin foo     | Test t1 | Example e1 | Bench b1 |
-------                         | ---               | -------     | ------- | ---------- | -------- |
`check`                         | check             | check       | -       | -          | -        |
`check --profile test`          | check_test†       | check_test† | -       | -          | -        |
`check --lib`                   | check             | -           | -       | -          | -        |
`check --lib --profile test`    | check_test†       | -           | -       | -          | -        |
`check --bin foo`               | check             | check       | -       | -          | -        |
`check -–bin foo –profile test` | check_test†       | check_test† | -       | -          | -        |
`check --bins`                  | check             | check       | -       | -          | -        |
`check --test t1`               | check             | check       | check_test   | -          | -        |
`check --tests`                 | check, check_test†  | check, check_test† | check_test | check†, check_test†  | -    |
`check --all-targets`           | check, check_test†  | check, check_test†  | check_test   | check, check_test† | check_test    |

† = different behavior from today
@ehuss
Copy link
Contributor

ehuss commented Oct 30, 2017

This is fixed by #4592.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants