Phpunit: Tests without assertions does not generate coverage

[edruid]

| Q | A
| --------------------| ---------------
| PHPUnit version | 6.5.6
| PHP version | 7.0.27
| Installation Method | Composer

I have a test that does not do an assertion (it tests a guard that throws an exception if it fails). This test doesn't provide coverage for the code it runs. If I add $this->assertTrue(true); at the end of the test coverage is provided.

/**
 * @doesNotPerformAssertions
 */
public function testVerifyActivityAccessSameUserAllowed()
{
    $user = $this->userFactory->create();

    app(UserVerifier::class)->verifyActivityAccess($user, $user);
}

phpunit is run like

vendor/bin/phpunit --coverage-html cov path/to/test.php

composer_info.txt

[sebastianbergmann]

This is expected behaviour.

A test that has no expectations and no assertions is considered by PHPUnit as risky (by default, since PHPUnit 6). PHPUnit does not generate code coverage for risky tests.

[edruid]

The documentation for @doesNotPerformAssertions states that the test is no longer considered risky. Should it not then also generate coverage?

[Stadly]

I agree that it makes sense that @doesNotPerformAssertions would lead to code coverage generation.

[simPod]

Is there a way to mark it as a non risky and therefore make coverage count?

[edruid]

Is there a way to mark it as a non risky and therefore make coverage count?

Well what I resorted to is the idiotic $this->assertTrue(true); at the end of the test...

[simPod]

Same here but that's a bit dumb and @doesNotPerformAssertions has no point then :/

[koriym]

Same here with $this->expectNotToPerformAssertions();

[nibynool]

Wasted an entire morning trying to work out why I wasn't getting coverage. Turns out this is the exact issue.

Would love for this to be re-opened as it does not seem intuitive that a test marked as @doesNotPerformAssertions (ie non-risky) is excluded from coverage.

Alternatively, some additional input from @sebastianbergmann to suggest a valid workaround or to change my mind about this would also be good :)

[sebastianbergmann]

The purpose of $this->expectNotToPerformAssertions(); (and @doesNotPerformAssertions) is to suppress the risky test warning. It is my opinion that a test that does not perform assertions should not result in code being marked as being tested (covered by a test).

[simPod]

I think the reason why assertTrue(true) is usually done is to assert that the tested code does not throw an exception. See issue question here or this test for example.

Something contrary to expectException() should solve all valid use cases (correct me if I'm wrong, I tried to gather all valid examples of assertTrue(true)). I'm thinking of something like expectExceptionNotThrown().

[andreasschroth]

Well I also came here at first to suggest the issue being reopened, but I have to agree with @sebastianbergmann . I think this only happens when a test isn't properly written.

A test I saw the problem in code coverage:

if (!\is_array($config)) {
    self::fail('The config needs to be an array.');
}

Obviously this test is not properly written and should be rewritten to:

self::assertIsArray($config);

[edruid]

@sebastianbergmann If I have a validator that only consists of a throw statement within an if-statement, how should one test - and get coverage - for the positive case? For example, how should I test the positive case for this code?

function verifyPositive(int $number) {
    if ($number <= 0) {
        throw new Exception('Number is not positive');
    }
}

[sebastianbergmann]

This is neither a discussion nor a support forum, sorry.

[simPod]

@andreasschroth you're right but that is completely different case

[andreasschroth]

@simPod Yep, I know. Just wanted to share some piece of code I stumbled upon which was clearly poorly written.

@edruid Imo, the question here is - does the positive case even need to be tested in this case? How can you assert something that is non-existent? If the number is positive, nothing is done / executed / thrown / happens. As Sebastian doesn't seem to want this to be discussed here, feel free to send me an e-mail (see profile) if you want to discuss it further.

[fullbl]

IMHO it's possible that a class is being tested without assertions, for example a factory.
If I try to assert that the result of a factory is an instance of a specific class, psalm will throw an error: why are you checking that an already known class (hinted from the return type) is of that type?
So, I can choose to suppress psalm, or I can choose to test something random, just for having the test go green!

[joostsolvari]

@sebastianbergmann If I have a validator that only consists of a throw statement within an if-statement, how should one test - and get coverage - for the positive case? For example, how should I test the positive case for this code?

function verifyPositive(int $number) {
    if ($number <= 0) {
        throw new Exception('Number is not positive');
    }
}

I have several classes which only contains methods like this, would love to see my coverage turn green.

Please reopen en reconsider <3