phpunit icon indicating copy to clipboard operation
phpunit copied to clipboard

CLI and XML option to not fail on PHPUnit warnings

Open nikophil opened this issue 6 months ago • 2 comments

Q A
PHPUnit version 12.2.2
PHP version 8.3.20
Installation Method Composer

Summary

PHPUnit warnings are always failing the testsuite, diregarding of failOnWarning configuration.

The final outcome of the testsuite is:

OK, but there were issues!
Tests: 1, Assertions: 1, Warnings: 1.

but always return an exit code of 1

This wasn't a problem until https://github.com/sebastianbergmann/phpunit/pull/6213 which started to trigger PHPUnit warnings when the number of arguments returned by the data provider does not match the number of arguments of the test method.

This warning seems totally legit, but in some case, it could be useful to allow those numbers of arguments to not match, example:

final class SomeTest extends TestCase
{
    #[Before]
    public function initializationMethod(): void
    {
        // do some heavy initialization using `$this->providedData()`
        // which would be reused by all test methods in the class
    }

    #[Test]
    #[DataProvider('provider')]
    public function testMethod() void
    {
        self::assertTrue(true);
    }

    public static function provider(): iterable
    {
        yield ['foo'];
    }
}

I know this is a little bit hacky, but in some complex scenario, it is pretty handy.

How to reproduce

final class ReproducingTest extends TestCase
{
    #[Test]
    #[DataProvider('provider')]
    public function testMethod() void
    {
        self::assertTrue(true);
    }

    public static function provider(): iterable
    {
        yield ['foo'];
    }
}

Expected behavior

I'm not sure what's the best way to fix this... There could be some "global" solutions:

  • make the config failOnWarning to act on PHPUnit warning as well
  • add a new config failOnPHPUnitWarning

but both of them would hide other more legit warnings, I think they are not a really good solution.

Or maybe we could go for a more fine-grained solution:

  • allow to ignore PHPUnit warning along with a baseline, as well as deprecations
  • add a new parameter to DataProvider attribute? #[DataProvider('providerMethodName', allowExtraArguments: true)]
  • introduce a new attribute #[IgnoreWarnings] / #[IgnorePHPUnitWarnings]

I'd be willing to provide a PR if we agree on a solution to implement.

thanks!

nikophil avatar Jun 13 '25 07:06 nikophil

PHPUnit warnings are always failing the testsuite, diregarding of failOnWarning configuration.

This is not a bug, this is by design. I am willing to discuss this design and the change the behaviour, of course.

sebastianbergmann avatar Jun 13 '25 09:06 sebastianbergmann

Another legit usecase (at least, in my opinion 😅 ) to have a different arguments number between the data provider and the test method, is when using the same data provider for several test methods, but one of the test methods does not need all the arguments:

final class ReproducingTest extends TestCase
{
    #[Test]
    #[DataProvider('provider')]
    public function testMethod1($arg1, $arg2) void
    {
        // ...
    }

    #[Test]
    #[DataProvider('provider')]
    public function testMethod2($arg1) void
    {
        // ...
    }

    public static function provider(): iterable
    {
        yield ['foo', 'bar'];
    }
}

I am willing to discuss this design and the change the behaviour, of course.

sweat! any thoughts about the solutions I proposed?

I think the one I prefer is definitely #[IgnoreWarnings] / #[IgnorePHPUnitWarnings].

Maybe, we could even go further and accept a pattern for a regex of warnings to ignore?

nikophil avatar Jun 13 '25 11:06 nikophil

Hi @sebastianbergmann

I saw the issue https://github.com/sebastianbergmann/phpunit/issues/6239 and its resolution. Thanks for this! It actually unlocks my CI!

But on the other hand, it ignores all warnings in all test cases, which feels a little bit insecure. Any chance to consider the solution I'm proposing with attributes #[IgnoreWarnings(?string $regex = null)] / #[IgnorePHPUnitWarnings(?string $regex = null)]? It would allow to ignore few warnings in some test cases.

thanks!

nikophil avatar Jun 20 '25 06:06 nikophil

Any chance to consider the solution I'm proposing with attributes #[IgnoreWarnings(?string $regex = null)] / #[IgnorePHPUnitWarnings(?string $regex = null)]?

I have to say that I am reluctant to introduce more complexity for this. Without actually implementing this, I do not know what the complexity will be. Feel free to send a PR for this, but be prepared that I might say no in case I think it adds too much complexity.

sebastianbergmann avatar Jun 20 '25 08:06 sebastianbergmann

ok, fair enough, I'll try this soonish

thanks.

nikophil avatar Jun 20 '25 08:06 nikophil