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

PHPUnit silently ignores the return value on a void method #3602

Closed
morozov opened this issue Apr 8, 2019 · 3 comments
Closed

PHPUnit silently ignores the return value on a void method #3602

morozov opened this issue Apr 8, 2019 · 3 comments

Comments

@morozov
Copy link
Contributor

@morozov morozov commented Apr 8, 2019

Q A
PHPUnit version 8.1.0
PHP version 7.3.4
Installation Method Composer

Consider the following test:

declare(strict_types=1);

use PHPUnit\Framework\TestCase;
use PHPUnit\Framework\MockObject\MockObject;

class Doer
{
    public function doStuff() : void
    {
    }
}

class PHPUnitTest extends TestCase
{
    public function test() : void
    {
        /** @var Doer|MockObject $doer */
        $doer = $this->createMock(Doer::class);
        $doer->expects($this->any())
            ->method('doStuff')
            ->willReturn(true);

        $this->assertNull($doer->doStuff());
    }
}

The test passes:

PHPUnit 8.1.0 by Sebastian Bergmann and contributors.

Testing PHPUnitTest


Time: 69 ms, Memory: 8.00 MB

OK (1 test, 1 assertion)

The test shouldn't pass since the willReturn(true) behavior cannot be achieved.

Fixing this issue could help keep existing tests up to date when the return value of a method is changed from something to void.

@sebastianbergmann
Copy link
Owner

@sebastianbergmann sebastianbergmann commented Apr 22, 2019

Thank you for bringing this to my attention. Unfortunately, this is not as easy to fix as it should be or as one might assume it is.

In your example, the call to willReturn() should trigger an exception because a return value is about to be configured for doStuff() which is void. This exception would then be used by PHPUnit to report a warning that there is something wrong with regard to test double configuration.

However, inside willReturn() we do not have the information available for which method a return value is configured. And since we do not know which method is being configured we do not know whether that method is void or not and if its configuration is possible. This was okay back in 2006 when the functionality was originally created and return type declarations, for instance, did not exist. Today it is not good enough anymore.

TL;DR Fixing this issue very likely requires a substantial refactoring of the runtime (not the code generation) part of PHPUnit's test double functionality.

Loading

@morozov
Copy link
Contributor Author

@morozov morozov commented Apr 22, 2019

Thank you for the explanation. I understand the problem.

Loading

@sebastianbergmann
Copy link
Owner

@sebastianbergmann sebastianbergmann commented Apr 26, 2019

This is a special case of #3122.

Loading

MichelHartmann added a commit to MichelHartmann/phpunit that referenced this issue Apr 27, 2019
PHPUnit Code Sprint with bschulze
MichelHartmann added a commit to MichelHartmann/phpunit that referenced this issue Apr 28, 2019
PHPUnit Code Sprint with bschulze
MichelHartmann added a commit to MichelHartmann/phpunit that referenced this issue Apr 30, 2019
PHPUnit Code Sprint with bschulze
MichelHartmann added a commit to MichelHartmann/phpunit that referenced this issue Apr 30, 2019
morozov added a commit to morozov/dbal that referenced this issue Jun 7, 2019
The new version contains some improvements in handling mocks (sebastianbergmann/phpunit#3602) and will help identify some existing issues in DBAL 3.0 tests:

There were 2 warnings:

1) Doctrine\Tests\DBAL\Driver\OCI8\OCI8StatementTest::testExecute with data set #0 (array('test', null, 'value'))
Method bindValue may not return value of type boolean

2) Doctrine\Tests\DBAL\Driver\OCI8\OCI8StatementTest::testExecute with data set #1 (array(null, 'test', 'value'))
Method bindValue may not return value of type boolean
morozov added a commit to morozov/dbal that referenced this issue Jun 7, 2019
The new version contains some improvements in handling mocks (sebastianbergmann/phpunit#3602) and will help identify some existing issues in DBAL 3.0 tests:

There were 2 warnings:

1) Doctrine\Tests\DBAL\Driver\OCI8\OCI8StatementTest::testExecute with data set #0 (array('test', null, 'value'))
Method bindValue may not return value of type boolean

2) Doctrine\Tests\DBAL\Driver\OCI8\OCI8StatementTest::testExecute with data set #1 (array(null, 'test', 'value'))
Method bindValue may not return value of type boolean
morozov added a commit to morozov/dbal that referenced this issue Jun 10, 2019
The new version contains some improvements in handling mocks (sebastianbergmann/phpunit#3602) and will help identify some existing issues in DBAL 3.0 tests:

There were 2 warnings:

1) Doctrine\Tests\DBAL\Driver\OCI8\OCI8StatementTest::testExecute with data set #0 (array('test', null, 'value'))
Method bindValue may not return value of type boolean

2) Doctrine\Tests\DBAL\Driver\OCI8\OCI8StatementTest::testExecute with data set #1 (array(null, 'test', 'value'))
Method bindValue may not return value of type boolean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants