One Invocation Per Expected Exception

ID

php.one_method_invocation_expected_testing

Severity

high

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Reliability

Language

Php

Tags

reliability, testing

Description

Reports a test that expects an exception while invoking the code under test more than once. Only one of those calls is the one being tested, and nothing in the test says which.

Rationale

When such a test passes, it proves only that something in the block raised the declared exception. The day the wrong call starts raising it — a changed precondition, a stricter setter, a mock that now throws — the test keeps passing while the behaviour it was written for is no longer covered. Splitting the block so the expectation covers exactly one call makes the test say what it means, and makes a future failure point at the right line.

Both ways a PHP test expresses the expectation are covered.

<?php

use PHPUnit\Framework\TestCase;

final class CheckoutTest extends TestCase
{
    private Checkout $subject;

    public function testCheckoutRejectsAnEmptyBasket(): void
    {
        $this->expectException(EmptyBasket::class); // FLAW — two calls can each raise it

        $this->subject->lockBasket();
        $this->subject->complete();
    }

    public function testCheckoutRejectsAnExpiredCard(): void
    {
        try { // FLAW — two calls can each raise it
            $this->subject->authorise('4111');
            $this->subject->capture();
            $this->fail('an expired card must be rejected');
        } catch (CardExpired $e) {
            $this->assertSame('expired', $e->getCode());
        }
    }
}

What counts as invoking the subject is deliberately narrow, so that a finding names a genuine ambiguity:

  • an assignment keeps its value and is therefore setup — which also stops the everyday $s = new Service(); $s→run(); pair from reading as two candidates;

  • a statement carrying an assertion, an expect… declaration, a skip marker or fail(…) states an expectation rather than exercising anything;

  • a line of double wiring, $gateway→method('charge')→willReturn(false);, configures a mock;

  • only statements after the expectException(…) declaration count, and only at the top level of the method — a call before it is setup, and one nested in a loop or a condition is not plainly one of a sequence;

  • the manual idiom needs its fail(…); without it a try that happens to hold two calls is ordinary error handling, not an expected-exception block.

A PHP finding does not claim that each of the calls can raise the declared type — PHP has neither checked exceptions nor a throws clause, and in a PHP test the throwing call is nearly always a method of the subject declared in another file. What it says is that the expectation covers more than one invocation, so a failure cannot say which one raised.

Remediation

Move everything that is setup out from under the expectation, leaving the one call being tested.

<?php

use PHPUnit\Framework\TestCase;

final class CheckoutExpectationTest extends TestCase
{
    private Checkout $subject;

    public function testCheckoutRejectsAnEmptyBasket(): void
    {
        $this->subject->lockBasket();

        $this->expectException(EmptyBasket::class);

        $this->subject->complete();
    }
}

If two calls genuinely need covering, write two tests, so each names the behaviour it proves.

© 2026 Xygeni. All rights reserved