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

Add option to cargo check to check all targets, including test code #3431

Closed
bruno-medeiros opened this issue Dec 20, 2016 · 20 comments
Closed

Comments

@bruno-medeiros
Copy link

Basically more or less the check version of cargo test --no-run

@bruno-medeiros bruno-medeiros changed the title Add option to check test code for cargo check Add option to cargo check to also check test code Dec 20, 2016
@jonhoo
Copy link
Contributor

jonhoo commented Dec 20, 2016

See also discussion following #3296 (comment) for motivation

@alexcrichton
Copy link
Member

IIRC Cargo does support cargo test --test foo to check a particular test, but I think we need two more additions here:

  • Like with cargo rustc, --profile test. This'd allow cargo check --lib --profile test to test compiling the library with tests
  • Support cargo check --tests to just check all tests, etc. Also things like cargo check --bins, cargo check --examples, etc.

@bruno-medeiros bruno-medeiros changed the title Add option to cargo check to also check test code Add option to cargo check to check all targets, including test code Dec 20, 2016
@bruno-medeiros
Copy link
Author

Support cargo check --tests to just check all tests, etc. Also things like cargo check --bins, cargo check --examples, etc.

Actually, to be more precise: this should an option that checks all targets (lib, bin, test, examples, doc). The full-build equivalent (cargo test --no-run) does that already, it build all targets - not just test targets but bin, doc, examples, too. (dunno if that behaviour was the original intention, but it's good that it does that)

@alexcrichton
Copy link
Member

Ah yeah that makes sense to me to have some form of as well. We may be able to emulate that with cargo check --all or something like that.

@crumblingstatue
Copy link

crumblingstatue commented Feb 17, 2017

Don't know about checking tests specifically, but cargo build has the --all option to build all targets in a workspace. It feels unsymmetrical that cargo check doesn't have this option.

@alexcrichton
Copy link
Member

I've added check --all in #3731, but it doesn't handle tests (e.g. the default current behavior), so I'm going to clleave this open.

@jonhoo
Copy link
Contributor

jonhoo commented Mar 17, 2017

@alexcrichton any idea when --all will get support for running test (or when we'll see --test)? It'd be really nice to have in the vim syntax checker (see neomake/neomake#843 (comment))

@ehuss
Copy link
Contributor

ehuss commented Mar 17, 2017

Just a warning for whoever works on this, when the test harness is included (rustc --test), it disables the missing_docs lint. Also, it disables the "main function not found" error for binaries. I'm not sure how it will be implemented in the context of cargo check, but something to watch out for.

@koivunej
Copy link
Contributor

I'd really like to see this implememented, but I can't really see how it should be implemented -- naively conditionally adding --test to rustc args in src/bin/check.rs was not the right solution.

Does someone already have a plan of how this should be implemented? If you do, and are too busy to work on this yourself, please share your thoughts so perhaps I or someone else could come up with at least the initial PR.

@alexcrichton
Copy link
Member

@koivunej this is likely related to #3112 which @BenWiederhake is working on, and after that it'd be relatively easy to add --all to just apply to all targets.

@BenWiederhake
Copy link
Contributor

Not sure how you think about interface consistency, but --all very often stands for "all packages", so this might be confusing.

@alexcrichton
Copy link
Member

That's true yeah, we may want to hold off on implementing this just yet, just mentioning that it should be relatively easy to support with that added in #3112

@koivunej
Copy link
Contributor

koivunej commented May 8, 2017

Just tried against latest nightly cargo 0.19.0-nightly (fa7584c14 2017-04-26) and it'd seem that cargo check --tests is supported, from help:

    --tests                      Check all tests

but it does not seem to check my #[test] function, as in cargo check && cargo check --tests && cargo test fails at last stage (cargo test) due to type checking error. As this issue is still open this is probably expected, as in --tests is for code in tests/ directory, not inline #[test] code?

@BenWiederhake
Copy link
Contributor

--tests only means that the files in the tests/ folder are included. so yes, you are correct.

I have no idea how to apply check on the in-line #[test]s, but I guess it involves some feature-magic.

@jonhoo
Copy link
Contributor

jonhoo commented May 15, 2017

Having a --tests that actually checks all tests code would be a hugely welcome addition. In the current scheme of things, editors are generally forced to use cargo test --no-run if they want to provide error highlighting for all errors, which is quite unfortunate. @alexcrichton how much work is this likely to be?

@alexcrichton
Copy link
Member

@jonhoo I think #4039 is a PR targeted at solving @koivunej's concern, although is that the feature you're thinking of?

@jonhoo
Copy link
Contributor

jonhoo commented May 15, 2017

Yes, that's exactly what I wanted. Thanks for the PR pointer!

@thegranddesign
Copy link

Just to give a newbie's perspective I would expect that cargo check --all would check everything. Period. If --all is intended to mean "all packages", then I'd suggest a --packages or --all-packages option instead which mirrors --all-features or --benches.

Consistency and intuitive APIs are important, especially for people just learning.

@thegranddesign
Copy link

I don't see that it was explicitly stated here, but I would also expect cargo check --tests or cargo check --docs to check documentation tests (and by extension cargo check --all should do the same).

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
@bors bors closed this as completed in 235712f Oct 30, 2017
@vn971
Copy link

vn971 commented Mar 9, 2021

For people who find this via search, the solution to typecheck tests is cargo check --tests

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

No branches or pull requests

9 participants