feat: only count assertions on expectations which can fail a test

[jmauerhan]

Hi folks, I've opened this PR to address an issue I've identified with Mockery reporting an incorrect assertion count, which causes tests to not be marked as risky when they should be.

A very minimal example of this issue can be seen with these simple tests:

<?php

class FooTest extends \PHPUnit\Framework\TestCase
{
    use \Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;

    public function testWithNoAssertions(): void
    {
        //no-op
    }

    public function testWithAStub(): void{
        $stub = Mockery::mock(\DateTime::class);
        $stub->allows('format');
    }

    public function testWithAssertion(): void{
        $stub = Mockery::mock(\DateTime::class);
        $stub->expects('format');

        $stub->format('c');
    }
}

The output from these tests with the current version of Mockery is:

vendor/bin/phpunit tests
PHPUnit 9.5.20 #StandWithUkraine

R..                                                                 3 / 3 (100%)

Time: 00:00.061, Memory: 6.00 MB

There was 1 risky test:

1) FooTest::testWithNoAssertions
This test did not perform any assertions

FooTest.php:7

OK, but incomplete, skipped, or risky tests!
Tests: 3, Assertions: 2, Risky: 1.

The first test does not use Mockery, and has no assertions or expectations. It is correctly identified as a Risky test.

However, we can see the middle test does not fail, and is not identified as a risky test with no assertions - allows being a wrapper for shouldReceive, which indicates a method can be called zero or more times - or in other words, this syntax lets us set up a stub that would never cause a test to fail whether it is called or not.

This should not be counted as an assertion, since it is not possible for this syntax alone to cause a test to fail.

The third test is an example of an expectation which should count as an assertion, because without calling the method once, the test will fail.

This pull request changes how Mockery counts the number of assertions, which is used to inform PHP Unit about a risky test.

With this change, the above tests now output a more accurate report of two risky tests.

PHPUnit 9.5.20 #StandWithUkraine

RR.                                                                 3 / 3 (100%)

Time: 00:00.041, Memory: 6.00 MB

There were 2 risky tests:

1) FooTest::testWithNoAssertions
This test did not perform any assertions

FooTest.php:7

2) FooTest::testWithAStub
This test did not perform any assertions

FooTest.php:12

OK, but incomplete, skipped, or risky tests!
Tests: 3, Assertions: 1, Risky: 2.

I have added tests to cover multiple cases, but I'm happy to add more if folks have suggestions for other cases.

[jmauerhan]

I noticed this was discussed previously, but not addressed: #249

[davedevelopment]

👍 thank you. I'm happy to let fly with this in a minor version, but that might ruffle some feathers. Will merge to master and let it sit for a short while before release, unless there are any objections?

[mfn]

that might ruffle some feathers

Can confirm 😅

In our case there were data provider based tests which in some cases expected an exception and in other cases did not perform assertions; the latter was now reported by phpunit whereas with mockery 1.5.0 they weren't. And somewhere in mix we use mockery.

Not a big thing to fix in our case 🤞🏼