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

shouldBeCalled being true even when not called #251

Closed
pedrofornaza opened this issue Jan 30, 2016 · 8 comments
Closed

shouldBeCalled being true even when not called #251

pedrofornaza opened this issue Jan 30, 2016 · 8 comments

Comments

@pedrofornaza
Copy link

Ive made a gist to illustrate my problem: https://gist.github.com/pedrofornaza/b819977e5415a0fc1643

The 'releaseEvents' should be called, but its not. The tests works even if i comment all the try body. The test fails if i use a spy (shouldHaveBeenCalled).

I did something wrong is that a bug?

Thanks in advance. :)

@aik099
Copy link
Member

aik099 commented Jan 31, 2016

In which way you're using Prophecy:

  1. via PHPUnit's own integration
  2. via manual PHPUnit integration
  3. via PHPSpec

?

If it's the 2nd case, then I'm suspecting that ->checkPredictions() method isn't being called to verify expectations.

@pedrofornaza
Copy link
Author

Im using 1. From my composer.lock:

"name": "phpunit/phpunit",
"version": "4.8.21",

"name": "phpspec/prophecy",
"version": "v1.5.0",

@kekko1212
Copy link

Here the same exact problem. Using Prophecy integrated in PHPUnit.

If I manually go for a checkPredictions call in the teardown (overriding the $prophet var that is private) everything works as should be.

Notice that only the assertions on the method calls aren't working.
The expectations on the returns (e.g. ->willReturn()) or the expected arguments are working properly.

This is my composer.json content

"phpunit/phpunit": "5.1",
"phpspec/prophecy": "dev-master"

@kekko1212
Copy link

Ok now I went a little bit more in a deep and I found the reason for this "strange" behaviour.

vendor/phpunit/phpunit/src/Framework/TestCase.php:1008

            try {
                $this->prophet->checkPredictions();
            } catch (Throwable $t) {
                /* Intentionally left empty */
            } catch (Exception $e) {
                /* Intentionally left empty */
            }

the exception for the failure is correctly thrown, but the catch does simply nothing with the exception, so it is failing silently.

The real strange thing are those comments. Why aren't you re-throwing the exception?

Do you "intentionally" wanted a red test to stay green? :D

@aik099
Copy link
Member

aik099 commented Feb 9, 2016

It's PHPUnit problem obviously. I recommend posting in there as well. Looking through PHPUnit commit history (at point where this code was created) might explain (in commit message) why this was done like this.

@jakzal
Copy link
Member

jakzal commented Feb 9, 2016

@kekko1212 would you mind to check what type of an exception is thrown and ignored exactly? Including the message?

@aik099
Copy link
Member

aik099 commented Feb 9, 2016

Looking at code below mentioned one shows if (isset($e)) { which does handle that exception correctly.

And yes, the problem is that Throwable $t should be Throwable $e instead (see sebastianbergmann/phpunit#1959). Fixed in 5.x (but not 5.0) and not 4.8 releases of PHPUnit.

@kekko1212
Copy link

Yes, it's a phpunit issue (only related to phpspec). The fix in PHPUnit 5.x solves this issue.

Thanks

@everzet everzet closed this as completed Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants