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

Fail or warn when using an overrestricitve test filter that filters all tests #6151

Open
phansch opened this issue Oct 7, 2018 · 10 comments
Labels
Command-test S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix

Comments

@phansch
Copy link
Member

phansch commented Oct 7, 2018

I'm not sure if this or the rust repo (libtest) is the right place to report this.

It's possible to run cargo test <pattern> to run the tests that match the given pattern.

However, if I run cargo test this_does_not_exist, everything looks green in the output. For example, if I run cargo test mod_floors in Chrono, it will look like this:

selection_008

On the first glance, it looks like everything is green and all tests passed.
It doesn't tell me that no tests were executed. I really wanted to run cargo test mod_floor:

selection_009

It would be nice if cargo test somehow tells me that no tests were executed.

Maybe adding a yellow or red warning text could be a good start?

@phansch phansch changed the title Fail or warn when using an overrestricitve filter that filters everything Fail or warn when using an overrestricitve test filter that filters all tests Oct 7, 2018
@ehuss
Copy link
Contributor

ehuss commented Oct 7, 2018

There have been some similar issues reported previously (#2832, #4324). A problem is that there is no structured output from libtest, so cargo doesn't know that the filter matched nothing. There is experimental JSON support available (rust-lang/rust#49359) and some progress has been made for experimental custom frameworks (rust-lang/rust#50297), but it looks like it will be a while before those get stabilized.

@phansch
Copy link
Member Author

phansch commented Oct 8, 2018

I see, thank you! fwiw, there is also this RFC which was never merged: rust-lang/rfcs#1284

@dwijnand
Copy link
Member

dwijnand commented Oct 9, 2018

Maybe libtest can return a different return code when no test ran, and then cargo test can handle the combinations?

Say 2 means "no test ran".

Return codes 0 2 mean at least one test ran and was successful, so "pass".
Return codes 1 2 mean at least one test ran and was unsucesssful, so "fail".
Return codes 2 2 mean not tests ran, so "fail".

WDYT?

@alexcrichton
Copy link
Member

It's possible yeah, but I'd prefer we stick with the pretty strongly ingrained unixism of "nonzero is failure". I think this can probably be tweaked in upstream libtest or wait for custom test harnesses

@dwijnand
Copy link
Member

dwijnand commented Oct 9, 2018

Failure's in the eye of the beholder?

If libtest is asked to run all tests that contain "flippity-bippity", for which there are no test, "success" is.. debatable, no?

@dwijnand
Copy link
Member

dwijnand commented Oct 9, 2018

Btw, I'm kind of arguing on the topic in a vacuum. I can understand the desire to maintain the status quo, at this point.

@alexcrichton
Copy link
Member

I agree that failing if no tests run makes the most sense if this is all we're considering, but there's far more to test suites than just the filter applied to it, so do think, yeah, that it's not globally the right thing to do to exit with a nonzero code if no tests are run.

@weihanglo weihanglo added the S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix label May 15, 2023
@epage
Copy link
Contributor

epage commented May 24, 2023

For #5609, I want to change the contract between libtest and cargo which would allow us to better handle cases like this

@Goirad
Copy link
Contributor

Goirad commented Mar 27, 2024

Can we do something like --fail-if-noop or equivalent? If that's acceptable (up to naming or maybe making it an unstable flag) I'd be willing to put in a PR for it

@epage
Copy link
Contributor

epage commented Mar 27, 2024

Until we change the contract and allow more control in cargo, this lives in libtest and libtest is in a soft feature freeze.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Command-test S-blocked-external Status: ❌ blocked on something out of the direct control of the Cargo project, e.g., upstream fix
Projects
Status: No status
Development

No branches or pull requests

7 participants