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-on-warning doesn't change status code #2246

Closed
perk11 opened this issue Jul 16, 2016 · 12 comments
Closed

--fail-on-warning doesn't change status code #2246

perk11 opened this issue Jul 16, 2016 · 12 comments
Labels
type/bug Something is broken

Comments

@perk11
Copy link

perk11 commented Jul 16, 2016

$ bin/phpunit --fail-on-warning tests/ExampleTest.php
PHPUnit 5.4.6 by Sebastian Bergmann and contributors.

W.                                                                  2 / 2 (100%)

Time: 2.95 seconds, Memory: 30.00MB

There was 1 warning:

1) Warning
The data provider specified for ExampleTest::testFoo is invalid.
Call to undefined method Foo::bar()

$ echo $?
0
thewilkybarkid referenced this issue in elifesciences/builder-base-formula Nov 8, 2016
@RolfBabijn
Copy link

I noticed the same problem. I've traced it to this bit of code (v5.6.3):

// \PHPUnit_TextUI_TestRunner line 595
if ($result->wasSuccessful()) {
    if ($arguments['failOnRisky'] && !$result->allHarmless()) {
        exit(self::FAILURE_EXIT);
    }

    if ($arguments['failOnWarning'] && $result->warningCount() > 0) {
        exit(self::FAILURE_EXIT);
    }

    exit(self::SUCCESS_EXIT);
}

// \PHPUnit_Framework_TestResult line 1231
public function wasSuccessful()
{
    return empty($this->errors) && empty($this->failures) && empty($this->warnings);
}

It seems to me that the if statement (about the failOnWarning) will never be reached due to the method $result->wasSuccessful() also checks if there are any warnings.

RolfBabijn added a commit to RolfBabijn/phpunit that referenced this issue Nov 15, 2016
@sebastianbergmann sebastianbergmann added the type/bug Something is broken label Jan 25, 2017
@sebastianbergmann
Copy link
Owner

I cannot reproduce this with current 5.7:

<?php
class Test extends PHPUnit\Framework\TestCase
{
    /**
     * @dataProvider invalid
     */
    public function testOne()
    {
    }
}
$ phpunit Test
PHPUnit 5.7.6-51-gb9287cb by Sebastian Bergmann and contributors.

W                                                                   1 / 1 (100%)

Time: 35 ms, Memory: 4.00MB

There was 1 warning:

1) Warning
The data provider specified for Test::testOne is invalid.
Method invalid does not exist

WARNINGS!
Tests: 1, Assertions: 0, Warnings: 1.
$ phpunit --fail-on-warning Test
PHPUnit 5.7.6-51-gb9287cb by Sebastian Bergmann and contributors.

W                                                                   1 / 1 (100%)

Time: 35 ms, Memory: 4.00MB

There was 1 warning:

1) Warning
The data provider specified for Test::testOne is invalid.
Method invalid does not exist

WARNINGS!
Tests: 1, Assertions: 0, Warnings: 1.

@xBazilio
Copy link

phpunit --fail-on-warning Test.php && echo "OK"
PHPUnit 5.6.4 by Sebastian Bergmann and contributors.

W                                                                   1 / 1 (100%)

Time: 161 ms, Memory: 16.00MB

There was 1 warning:

1) Warning
The data provider specified for Test::testOne is invalid.
Method invalid does not exist

/monamour/home/rumyantsev/.composer/vendor/phpunit/phpunit/src/TextUI/Command.php:188
/monamour/home/rumyantsev/.composer/vendor/phpunit/phpunit/src/TextUI/Command.php:118

WARNINGS!
Tests: 1, Assertions: 0, Warnings: 1.
OK

Problem still exists

@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Jan 25, 2017

In PHPUnit 5.7.6-51-gb9287cb (soon to be released as PHPUnit 5.7.7) the problem does not exist anymore.

@xBazilio
Copy link

Confirm

@aivus
Copy link
Contributor

aivus commented Jan 25, 2017

@sebastianbergmann it's side effect of this commit: f4cf4d1.

But failOnWarning="false" doesn't work now.

It returns FAILURE_EXIT in case of failOnWarning="false" always.

➜  phpunit git:(5.7) ✗ ./phpunit Test && echo OK
PHPUnit 5.7.6-51-gb9287cb by Sebastian Bergmann and contributors.

Runtime:       PHP 7.0.14 with Xdebug 2.5.0
Configuration: /Users/aivus/projects/phpunit/phpunit.xml

W                                                                   1 / 1 (100%)

Time: 72 ms, Memory: 2.00MB

There was 1 warning:

1) Warning
The data provider specified for Test::testOne is invalid.
Method invalid does not exist

/Users/aivus/projects/phpunit/src/Framework/WarningTestCase.php:57
/Users/aivus/projects/phpunit/src/Framework/TestCase.php:962
/Users/aivus/projects/phpunit/src/Framework/TestResult.php:709
/Users/aivus/projects/phpunit/src/Framework/TestCase.php:917
/Users/aivus/projects/phpunit/src/Framework/TestSuite.php:728
/Users/aivus/projects/phpunit/src/Framework/TestSuite.php:728
/Users/aivus/projects/phpunit/src/TextUI/TestRunner.php:487
/Users/aivus/projects/phpunit/src/TextUI/Command.php:188
/Users/aivus/projects/phpunit/src/TextUI/Command.php:118

WARNINGS!
Tests: 1, Assertions: 0, Warnings: 1.
➜  phpunit git:(5.7) ✗ echo $?
1

fracz added a commit to iisg/redo that referenced this issue Mar 24, 2019
sebastianbergmann/phpunit#2246

Change-Id: I8a61ed18fcbf847f65ae0f56969e477a8760e8a9
fracz added a commit to iisg/redo that referenced this issue Nov 10, 2019
sebastianbergmann/phpunit#2246

Change-Id: I8a61ed18fcbf847f65ae0f56969e477a8760e8a9
@afflerbach
Copy link

Yep, failOnWarning="false" still does not seem to have any effect (PHPUnit 10.2.3).

@sebastianbergmann
Copy link
Owner

Yep, failOnWarning="false" still does not seem to have any effect (PHPUnit 10.2.3).

I cannot reproduce this:

Test.php

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

final class Test extends TestCase
{
    public function testOne(): void
    {
        $a = $b;
        
        $this->assertTrue(true);
    }
}

No XML configuration, no --fail-on-warning CLI option

$ phpunit Test.php
PHPUnit 10.2.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.7

W                                                                   1 / 1 (100%)

Time: 00:00.008, Memory: 4.00 MB

OK, but there are issues!
Tests: 1, Assertions: 1, Warnings: 1.
$ print $?        
0

No XML configuration, --fail-on-warning CLI option

$ phpunit --fail-on-warning Test.php
PHPUnit 10.2.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.7

W                                                                   1 / 1 (100%)

Time: 00:00.007, Memory: 4.00 MB

OK, but there are issues!
Tests: 1, Assertions: 1, Warnings: 1.
$ print $?                          
1

XML configuration with failOnWarning="true", no --fail-on-warning CLI option

phpunit.xml
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.2/phpunit.xsd"
         failOnWarning="true">
</phpunit>
$ phpunit Test.php                  
PHPUnit 10.2.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.7
Configuration: /home/sb/phpunit.xml

W                                                                   1 / 1 (100%)

Time: 00:00.009, Memory: 4.00 MB

OK, but there are issues!
Tests: 1, Assertions: 1, Warnings: 1.
$ print $?                        
1

XML configuration with failOnWarning="false", no --fail-on-warning CLI option

phpunit.xml
<?xml version="1.0" encoding="UTF-8"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.2/phpunit.xsd"
         failOnWarning="false">
</phpunit>
$ phpunit Test.php                  
PHPUnit 10.2.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.7
Configuration: /home/sb/phpunit.xml

W                                                                   1 / 1 (100%)

Time: 00:00.009, Memory: 4.00 MB

OK, but there are issues!
Tests: 1, Assertions: 1, Warnings: 1.
$ print $?                        
0

@maks-rafalko
Copy link
Contributor

maks-rafalko commented Jul 5, 2023

We had a similar issue and the cause was Symfony PhpUnit Bridge (symfony/phpunit-bridge) that has its own error handler, breaking behavior of --fail-on-warnings.

At the time of writing, bridge is not compatible with PHPUnit 10+: symfony/symfony#49069

We had to remove the bridge and use just PHPUnit, now no issues with warnings.

@afflerbach
Copy link

Sorry for that inaccurate statement, it actually seems to work most of the time.

However, it is reproducable with an invalid test class name:

$ cat Test.php
<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

final class Test extends TestCase
{
    public function testOne(): void
    {
        $this->assertTrue(true);
    }
}

$ cat SecondTest.php
<?php declare(strict_types=1);
use PHPUnit\Framework\TestCase;

final class MisnamedTestClass extends TestCase
{
}

$ cat phpunit.xml
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.2/phpunit.xsd"
         failOnWarning="false">
</phpunit>

$ phpunit . ; echo $?
PHPUnit 10.2.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.5
Configuration: /tmp/phpunit-bug/phpunit.xml

.                                                                   1 / 1 (100%)

Time: 00:00, Memory: 22.41 MB

There was 1 PHPUnit test runner warning:

1) Class SecondTest cannot be found in /tmp/phpunit-bug/SecondTest.php

WARNINGS!
Tests: 1, Assertions: 1, Warnings: 1.
1

@sebastianbergmann
Copy link
Owner

Sorry for that inaccurate statement, it actually seems to work most of the time.
However, it is reproducable with an invalid test class name:

By design, test runner warnings are not affected by failOnWarning="false".

@afflerbach
Copy link

Okay, thanks for the explanation, makes sense.

Anyway, it's kind of surprising that both our status lines look identical, but show different kinds of Warnings, that have different impact on the exit code:

Tests: 1, Assertions: 1, Warnings: 1.

This is what made me try to temporarily ignore that issue via failOnWarning="false" in the first place.

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

No branches or pull requests

7 participants