check --tests always succeeds if you have test = false in Cargo.toml

[davidpdrsn]

Problem
If a crate has

[lib]
test = false

in its Cargo.toml and you run cargo check --tests the crate always compiles regardless of what errors might actually be there.

I would expect --tests to still fail to compile or at least emit a clear warning.

Steps

1. Made a little reproduction repo: git clone https://github.com/davidpdrsn/cargo-bug
2. cd cargo-bug
3. cargo check --tests compiles
4. cargo check does not compile

Notes

Output of cargo version: cargo 1.52.0 (69767412a 2021-04-21)

[ehuss]

When passing a target selection flag, that tells cargo to check the specified targets instead of the default set of targets. With --tests, it only selects tested targets. Since you have specified the lib not to be tested, there is nothing for cargo to check. There is more information about target selection here.

I think, perhaps, cargo could print a warning if using --tests, --bins, --examples, or --benches, and no matching targets were found.

[Bryysen]

If we do cargo [command] --[target] target, where 'target' doesn't exist cargo will error accordingly. Would it also make sense if we error when doing cargo [command] --[targets] if there are zero matching targets? Also I would like claim this one if that's okay, and I'll work towards a solution once a consensus is reached.

[ehuss]

I think for now it should just be a warning. It might otherwise break legitimate use cases. An example is the rust build system, which here blindly passes a bunch of flags to different packages which may or may not contain all those targets. We probably don't want a bunch of noisy warnings in that case.

I also think this warning should only appear if no targets were found in total. That is, cargo test --lib --benches should be fine if there is a lib but no benches. The set of root targets is determined here, and I think (maybe) it just needs to check if units.is_empty().

It is totally OK to claim, and let me know if you need any help. There's a guide here for writing and running tests.

[Bryysen]

Yes I agree, we should just emit an error here (seems like a silly question in hindsight). I was actually wondering if we wanted to emit a warning for every single missed target, or only if no targets were found in total. I agree it's better to go with the second option like you say as this would cut down on warning noise.

I'm a little unsure on the formatting on the warning message. Here's what I have so far

Let me know if the formatting should be done differently, or if there is more useful information we can supply to the user. I was thinking maybe to create a special-case notification if we have [lib] test = false and --tests was passed without matching on anything, or would that be too much effort (diving into manifests and whatnot may just overcomplicate things)?

[ehuss]

The formatting looks good, feel free to post a PR and we can go over it in more detail. I wouldn't bother with special-casing if something like test=false is set, I think the wording you have above seems petty clear.