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 a --fail-on-skipped option #2840

Closed
soullivaneuh opened this issue Nov 6, 2017 · 11 comments
Closed

Add a --fail-on-skipped option #2840

soullivaneuh opened this issue Nov 6, 2017 · 11 comments

Comments

@soullivaneuh
Copy link

Currently, we can make PHPUnit failing on warning and failed tests, but not on skipped ones.

We can only make PHPUnit stop, but with no error code:

$ phpunit --fail-on-warning tests/AppBundle/Security/

 // Clearing the cache for the test environment with debug true                                                         

 [OK] Cache for the "test" environment (debug=true) was successfully cleared.                                           

PHPUnit 6.3.1 by Sebastian Bergmann and contributors.

S.....                                                              6 / 6 (100%)

Time: 3.02 seconds, Memory: 50.00MB

OK, but incomplete, skipped, or risky tests!
Tests: 6, Assertions: 34, Skipped: 1.

I would like to make PHPUnit failing on skipped test to ensure the CI job as no skipped job (because the needed env should be complete).

Having a --fail-on-skipped option would be a great addition.

Regards.

@johnbillion
Copy link
Contributor

It sounds like your tests should just fail, instead of being marked as skipped.

What's your use case for marking a test as skipped when you in fact want them to fail?

@sebastianbergmann
Copy link
Owner

I do not think that this would be useful.

@soullivaneuh
Copy link
Author

What's your use case for marking a test as skipped when you in fact want them to fail?

@johnbillion Some test need specific environment like services that can not always be installed on local computer. Then I prefer to skip the test.

But on CI, the env should be complete and compliant to all the tests. This is why I would like this option.

@sebastianbergmann Can this be please re-considered? I think this option worth it.

@soullivaneuh
Copy link
Author

soullivaneuh commented Dec 13, 2017

@sebastianbergmann Any chance to get this issue reconsidered? Thanks!

BTW FYI, my use case is not concerning a library, but a final project. So I'm not testing against multiple environment.

@johnbillion
Copy link
Contributor

The best approach here is to place all of those environment specific tests into a @group and then run your tests locally with that group excluded.

Example:

phpunit --exclude-group service-dependent

@soullivaneuh
Copy link
Author

@johnbillion This is indeed technically possible, but does not really fit my case I'm afraid.

The case is: "All the tests of my private projects should run successfully on CI. Some can be skipped without issue if some requirement are no filled.".

The requirement is, for example, the presence of a ssh key on the fixture directory. So this will be dynamic and I should not have to specify groups on a command line.

@jakajancar
Copy link

jakajancar commented Feb 26, 2018

+1

I do not think that this would be useful.

Clearly it would be, at least to 3 people in this thread, unless you can think of a better way? Use case exactly as @soullivaneuh described — it's ok for tests to be skipped on dev laptops, but the CI environment should support everything.

@jakajancar
Copy link

jakajancar commented Feb 26, 2018

Alternatively maybe "--consider-skipped-risky". Clearly skipped tests carry some risk. Then existing --fail-on-risky can be used.

// arguably --report-useless-tests should also consider skipped tests as useless, but currently it does not

@andreasschroth
Copy link

This would definitely be useful. As @jakajancar already wrote, obviously there is some risk from skipped tests.

It might just be a mistake those tests are skipped, e.g. I saw it happen that a developer renamed a test method and another test method depending on it still had the old method name in @depends. In this case, it was totally unintentional that the test has been skipped. But on CI it all runs through.

I mean, it's just an additional option - nobody has to use it, if he doesn't need it, but I think there's a use-case for it.

@bobemoe
Copy link

bobemoe commented Oct 11, 2020

Another +1 for a similar use case. We've just had some bugs get into production because the selenium server failed to start which meant all the phpunit tests passed! I think we could extend selenium to return fail rather than skip, but I'm concerned other services may do the same thing, and we may see this issue reoccur in other cases.

I agree at certain points in the testing that all the tests need to run successfully for the CI to be considered passed, and there needs to be some way tell phpunit that a skipped test is not acceptable.

Thanks for reconsidering :)

@BenMorel
Copy link
Contributor

Since version 9.4.3, PHPUnit has a --fail-on-skipped option:

$ vendor/bin/phpunit --help
(...)
  --fail-on-incomplete        Treat incomplete tests as failures
  --fail-on-risky             Treat risky tests as failures
  --fail-on-skipped           Treat skipped tests as failures
  --fail-on-warning           Treat tests with warnings as failures
(...)

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

No branches or pull requests

7 participants