Expectation is not counted correctly when a doubled method is called more often than is expected #6095
Comments
I assume by "deprecation warnings" you mean "risky tests", as I do not see any deprecation warnings in your output for PHPUnit 10.5. |
|
Do you use Also note that creating mock objects in a data provider does not work. Your workaround ( |
|
I ran your tests
|
|
For some reason, I do not remember why, the case that a doubled method is called more often than it is expected to be called is handled differently than all other mock object-related expectation failures. If a doubled method is expected to be called once and is called a second time then the execution of the test is immediately aborted and the test is considered a failure. However, the fact that an expectation was verified is not recorded by incrementing the assertion counter. A look into the Git history of this part of the code reminded me that I was working on this a while ago: |
The "deprecation warning" from PHPUnit 11 was due to using a docblock for the |
No we do not currently use it anywhere in our test suite. It was added to the reproduction case as one of our engineers noted the difference between the behavior of the locally created mocks and the injected mocks was that the injected ones were not registered. After seeing that it almost completely resolved the issues with the pattern, I do have a PR started to add it to the actual tests in our suite that are encountering this... but as you say, it's not something we are supposed to be doing, and as I said in the initial report, it feels dirty to have to do it. I would rather not.
Until now, it has worked just fine... we do this in a number of our tests. With the addition of the We also create a number of stubs in this same method, often those are then injected into the mocks. For example a domain object mock will have a repository stub injected which has some domain object stubs injected into it, all of which are configured for the given test variation to represent a given state of the objects in the situation under test. Can you point to any additional documentation or background on what doesn't work about this? (I've just re-read both the test doubles and data providers section of the PHPUnit manual, and unless I missed something, neither section mentions the other's concepts at all, let alone gives any advice not to mix the two.)
Sorry, I perhaps was not clear... |
|
I've re-worked the test case that originally exposed this now to have the data provider inject a closure which takes a Do you foresee any issues with this pattern? Do you think this is also needed for Stubs, or will the workaround of creating a new testcase instance in the dataprovider work for those? To simulate this in the bugReport file, here is the additional functions: and it's run: |
|
The actual bug you discovered in |
Summary
We have some testcases that rely 100% on configuring mocks with expectations for their assertions. I'll focus on one of them that has 6 variations from it's data provider. With the upgrade to phpunit 11, all 6 variations now report as risky, saying they did not perform any assertions. This is demonstrably false, as I can intentionally break the mocked objects passed into the test from the dataProvider and observe that it does in fact fail the test because the expectations on another mocked object don't align with how the object is called in the code under test, however if the expectation is not violated it doesn't trigger. (For example, if the expectation is that a method will be called once, and it is never called, then the failure will not be seen. But if it is called twice, the failure will be reported.) This only happens with mocks provided via dataProviders, those created in the test function do not exhibit this behavior.
Current behavior
How to reproduce
bugDemo.php.gz
I've extracted the issue out of our test cases and attached a simple testcase file which has 9 test functions in a 3x3 grid as follows:
a. created locally
b. injected from a dataProvider
c. injected from a dataProvider and registered with
$this->registerMockObject()in the functiondemo()function is:a. zero (should fail)
b. one (should pass)
c. two (should fail)
Using the internal
registerMockObject()function in the test to register the mocks injected from the data provider appears to bring the behavior closer into alignment with expectations, but still unexpectedly reports a risky test on the two call version. This leaves me with very little faith that using this internal function is a proper solution to our issue. (and needing to use an internal function to make the test case whole seems... alarming.)(phpunit 11.5.2 behaves the same as 11.4.3 with this test; but there are issues with some of our other tests that prevent me from using 11.5 across the whole test suite just yet.)
Phpunit 10.5.40 with this test case is slightly different, but is consistent:
Expected behavior
Under phpunit 10.5.40 these tests run perfectly with no deprecation warnings:
The only difference here is that I did
composer require --dev phpunit/phpunit:^10.0 -Wafter running with 11.4.3 above to back down to 10.5.40... all code under test was the same.The text was updated successfully, but these errors were encountered: