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

"No tests executed" does not always result in exit code 1 #4299

Closed
remicollet opened this issue Jun 17, 2020 · 13 comments
Closed

"No tests executed" does not always result in exit code 1 #4299

remicollet opened this issue Jun 17, 2020 · 13 comments
Assignees
Labels
feature/test-runner CLI test runner type/bug Something is broken
Milestone

Comments

@remicollet
Copy link
Contributor

And exit with an error code.

If you run phpunit, this means you very probably have tests.

This can avoid such issue: egulias/EmailValidator#244
as for now travis stay "green" while there is obviously something wrong.

Perhaps in next 9.3 or 10.0
Perhaps with a configuration option, with default=no in next and default=yes in next++

Open for discussion.

@remicollet remicollet added the type/enhancement A new idea that should be implemented label Jun 17, 2020
@sebastianbergmann sebastianbergmann self-assigned this Jun 17, 2020
@sebastianbergmann sebastianbergmann added this to the PHPUnit 9.3 milestone Jun 17, 2020
@sebastianbergmann
Copy link
Owner

This already exists.

<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

final class Test extends TestCase
{
}
$ phpunit Test.php
PHPUnit 9.2.3 by Sebastian Bergmann and contributors.

W                                                                   1 / 1 (100%)

Time: 00:00.012, Memory: 6.00 MB

There was 1 warning:

1) Warning
No tests found in class "Test".

WARNINGS!
Tests: 1, Assertions: 0, Warnings: 1.
$ echo $?                     
0
$ phpunit --fail-on-warning Test.php
PHPUnit 9.2.3 by Sebastian Bergmann and contributors.

W                                                                   1 / 1 (100%)

Time: 00:00.014, Memory: 6.00 MB

There was 1 warning:

1) Warning
No tests found in class "Test".

WARNINGS!
Tests: 1, Assertions: 0, Warnings: 1.
$ echo $?                           
1

@sebastianbergmann sebastianbergmann removed this from the PHPUnit 9.3 milestone Jun 17, 2020
@sebastianbergmann sebastianbergmann removed the type/enhancement A new idea that should be implemented label Jun 17, 2020
@sebastianbergmann sebastianbergmann removed their assignment Jun 17, 2020
@remicollet
Copy link
Contributor Author

Nah, not the same case.

$ mkdir foo
$ phpunit9 --fail-on-warning  foo
PHPUnit 9.2.3 by Sebastian Bergmann and contributors.

Time: 00:00.001, Memory: 4.00 MB

No tests executed!

$ echo $?
0

@sebastianbergmann sebastianbergmann self-assigned this Jun 17, 2020
@sebastianbergmann sebastianbergmann added feature/test-runner CLI test runner type/enhancement A new idea that should be implemented labels Jun 17, 2020
@sebastianbergmann sebastianbergmann added this to the PHPUnit 9.3 milestone Jun 17, 2020
@sebastianbergmann sebastianbergmann changed the title make "no tests executed" an error "No tests executed" should always be a warning Jun 17, 2020
@sebastianbergmann
Copy link
Owner

Thank you, @remicollet.

@sebastianbergmann sebastianbergmann changed the title "No tests executed" should always be a warning "No tests executed" should always result in exit code 1 Jun 17, 2020
@remicollet
Copy link
Contributor Author

remicollet commented Jun 17, 2020

To summarize the need, in linked project:

Tests directory was rename to tests but phpunit.xml was not updated.

@sebastianbergmann sebastianbergmann added type/bug Something is broken and removed type/enhancement A new idea that should be implemented labels Jun 17, 2020
@sebastianbergmann sebastianbergmann changed the title "No tests executed" should always result in exit code 1 "No tests executed" does not always result in exit code 1 Jun 17, 2020
@remicollet
Copy link
Contributor Author

thanks

@localheinz
Copy link
Collaborator

localheinz commented Jun 22, 2020

Hmm, not sure - why would this be an error?

Imagine you set up a project using a skeleton, but do not have tests at the time. Why should the build fail when there are no tests?

Updating from phpunit/phpunit:9.2.3 to phpunit/phpunit:9.2.4 now effectively fails all builds for projects executing empty test suites.

@sebastianbergmann
Copy link
Owner

The reasoning is this: PHPUnit was asked to run tests. You expect tests to be run. No tests were run. This does not match your expectation.

@localheinz
Copy link
Collaborator

localheinz commented Jun 22, 2020

It did for me - I have somewhere between 20 and 30 repositories where I have

  • auto-review
  • unit
  • integration

test suites, all configured with separate phpunit.xml configuration files.

Not all of these test suites have tests in every repository, which has worked fine. All of these repositories use the same or similar workflow definitions, so I can easily keep workflows - whether intended for GitHub Actions or other build tools - synchronized.

With this change, I now need to go through all of the repositories making use of phpunit/phpunit:^9 and remove empty test suites and adjust workflows to make the builds pass.

While subtle, I would consider this a breaking change.

@localheinz
Copy link
Collaborator

localheinz commented Jun 22, 2020

Perhaps we can introduce an option that allows failing with an error when a test suite fails is empty?

Similar to how infection/infection allows to run without failure when no mutations were created?

@sebastianbergmann
Copy link
Owner

PHPUnit behaved exactly like this for a long time. Then at some point this behaviour was unintentionally changed. I was not aware of this until @remicollet brought this to my attention.

I think that PHPUnit should behave as it does right now. I understand (now, thank you, @localheinz) that fixing this after it was broken for so long is a break of backward compatibility. I shall revert the change now.

@localheinz
Copy link
Collaborator

Thank you, @sebastianbergmann!

I'll be happy to provide an option to fail for empty test suites - what do you think?

@sebastianbergmann
Copy link
Owner

That will be the topic of a ticket that I will open soon (after I have rolled back the change).

@sebastianbergmann
Copy link
Owner

For reference: #4313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/test-runner CLI test runner type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

3 participants