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

Raising an exception from a test double's configured method does not work #4162

Closed
driesvints opened this issue Apr 3, 2020 · 7 comments
Closed
Assignees
Labels
feature/test-doubles Stubs and Mock Objects type/bug Something is broken

Comments

@driesvints
Copy link

driesvints commented Apr 3, 2020

Q A
PHPUnit version 9.1.0
PHP version 7.3.16
Installation Method Composer

Summary

With the new 9.1.0 update, one of the tests in the laravel/framework test suite suddenly started to fail. I'm still uncertain what's causing this but thought to open this issue to make you aware and perhaps see if someone could help track down the change that's causing this.

Current behavior

We're now getting this failure:

There was 1 failure:

1) Illuminate\Tests\Database\DatabaseConnectionTest::testTransactionRetriesOnSerializationFailure
Expectation failed for method name is "commit" when invoked 3 time(s).
Method was expected to be called 3 times, actually called 0 times.

Here's an example run: https://github.com/laravel/framework/runs/557414302?check_suite_focus=true

And here's the test in question: https://github.com/laravel/framework/blob/55189c1128d692e3abe194ea91b85377d1a1f5a5/tests/Database/DatabaseConnectionTest.php#L256-L269

How to reproduce

Install laravel/framework locally with PHPUnit 9.1.0 and run the above test using PHP 7.3

Expected behavior

The test was passing before PHPUnit 9.1.0 was released.

@sebastianbergmann
Copy link
Owner

Thank you for your report.

Please provide a minimal, self-contained, reproducing test case that shows the problem you are reporting.

Without such a minimal, self-contained, reproducing test case I will not be able to investigate this issue.

@sebastianbergmann sebastianbergmann added the feature/test-doubles Stubs and Mock Objects label Apr 3, 2020
@driesvints
Copy link
Author

@sebastianbergmann investigating further to see if I can boil it down to that 👍

@sebastianbergmann
Copy link
Owner

Just to be sure: with

The test was passing before PHPUnit 9.1.0 was released.

you mean that your test works with PHPUnit 9.0.2 but does not work with PHPUnit 9.1.0?

@driesvints
Copy link
Author

@sebastianbergmann exactly.

I've made some progress. The test passes when I change the code to this:

laravel/framework@ffb812c

@driesvints
Copy link
Author

I tried boiling it down to a simple test but no luck I'm afraid :/

@AlfonsMartin
Copy link

<?php

declare(strict_types=1);

namespace test;

use Exception;
use PHPUnit\Framework\TestCase;

final class ClassTest extends TestCase
{
    public function testCreateDb()
    {
        $mock =  $this->getMockBuilder(ClassToMock::class)
        ->disableOriginalConstructor()
        ->setMethods(['one','two'])
        ->getMock();
        $mock->expects($this->at(0))->method("one")->will($this->throwException(new Exception()));
       
        $mock->expects($this->at(1))->method("two")->with($this->equalTo("Test1"));
        $mock->expects($this->at(2))->method("two")->with($this->equalTo("Test2"));

        $mock->test();
    }

}

class ClassToMock
{
    public function __construct()
    {}

    public function one()
    {
        throw new Exception();
        return null;
    }

    public function two()
    {
        return null;
    }

    public function test()
    {
        try{
            $this->one();
        } catch (Exception $e) {
            $this->two("Test1");
            $this->two("Test2");
        }

        return null;
    }
}

Leads to

PHPUnit 9.1.0 by Sebastian Bergmann and contributors.

F                                                                   1 / 1 (100%)

Time: 31 ms, Memory: 4.00 MB

There was 1 failure:

1) test\ClassTest::testCreateDb
Expectation failed for method name is "two" when invoked at sequence index 1
Parameter 0 for invocation test\ClassToMock::two('Test2') does not match expected value.
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Test1'
+'Test2'

/home/alma/phpunitCheck/ClassTest.php:50
/home/alma/phpunitCheck/ClassTest.php:23

FAILURES!
Tests: 1, Assertions: 0, Failures: 1.

And

PHPUnit 9.0.2 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 26 ms, Memory: 4.00 MB

OK (1 test, 1 assertion)

This only happens with the exception in there. Seems like the exception resets the counter.
Because this works for PHPUnit 9.1.0 but not for PHPUnit 9.0.2:

$mock->expects($this->at(0))->method("one")->will($this->throwException(new Exception()));
$mock->expects($this->at(0))->method("two")->with($this->equalTo("Test1"));
$mock->expects($this->at(1))->method("two")->with($this->equalTo("Test2"));

@sebastianbergmann sebastianbergmann changed the title 9.1.0 update causes suddenly failing test Raising an exception from a test double's configured method does not work Apr 3, 2020
@sebastianbergmann sebastianbergmann self-assigned this Apr 3, 2020
@driesvints
Copy link
Author

@sebastianbergmann thanks for looking into this so quickly! :)

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

No branches or pull requests

3 participants