PHPUnit Test Reviews in Teams: What Tests Should Actually Be Checked For

1. Why test reviews often fail

In most development teams, code review is understood as mandatory, for production code. Tests are often treated differently: they need to be green, they need to hit the coverage threshold, and then the review is checked off. This attitude is understandable, because tests are technically code that never gets executed in production. But it is dangerous, because bad tests build a false sense of safety that only becomes visible later, in production.

The actual damage does not appear immediately. A test that checks the wrong behavior turns green in the CI run and generates coverage. Months later, a developer changes the business logic, all tests stay green because none of them ever really secured the relevant behavior, and the bug reaches production. At that point, the connection between a flawed test review and the production defect is hard to trace back.

A good test review follows different criteria than a production code review. Instead of type safety and architecture, the center of attention is: does this test check the behavior that matters to the caller? Would the test fail if the implementation were wrong? These questions require context, and they form the basis of the guide that follows.

2. Assertions: quality over quantity

The most common review question about assertions is the wrong one: how many assertions does the test have? The right question is: does the assertion check behavior that is visible from the outside, or does it check an internal implementation detail? assertTrue($result) on a boolean return value, with no statement about what true actually means, is essentially empty. assertSame(42.50, $cart->getTotal()), on the other hand, makes a specific behavioral statement.

In review, every assertion needs to be checked for what it actually asserts. An assertion like assertNotNull($result) only fails when null is returned, not when a wrong value is returned. assertSame is stricter and better than assertEquals in most cases, because it also checks the type. assertInstanceOf only checks the class, not the content of the object. In review, it always helps to ask: what would happen if the code returned the wrong answer, would this assertion notice?

<?php

declare(strict_types=1);

namespace Tests\Unit\Domain\Cart;

use App\Domain\Cart\CartCalculator;
use App\Domain\Cart\CartItem;
use Money\Money;
use PHPUnit\Framework\TestCase;

/**
 * Test for CartCalculator, verifies business rules, not implementation details.
 */
final class CartCalculatorTest extends TestCase
{
    private CartCalculator $calculator;

    protected function setUp(): void
    {
        $this->calculator = new CartCalculator();
    }

    /** @test */
    public function it_applies_ten_percent_discount_when_total_exceeds_one_hundred_euros(): void
    {
        $items = [
            new CartItem('Book', Money::EUR(6000)),
            new CartItem('Pen', Money::EUR(5000)),
        ];

        $total = $this->calculator->calculate($items);

        // Specific behavioral assertion: discount applied, exact amount expected
        self::assertEquals(Money::EUR(9900), $total);
    }

    /** @test */
    public function it_does_not_apply_discount_when_total_is_below_threshold(): void
    {
        $items = [new CartItem('Eraser', Money::EUR(3000))];

        $total = $this->calculator->calculate($items);

        self::assertEquals(Money::EUR(3000), $total);
    }
}

3. Test names as living documentation

Test methods are specification documents. Their name should describe, in a single sentence, which behavior is expected under which conditions. The common style testCalculate() or test_returns_true() says nothing at all. When such a test fails, nobody knows which behavior is broken. The preferred style follows the pattern it_[describes_behavior]_when_[condition]().

In review, the reviewer should be able to understand what is being tested purely from the test name, without reading the test code. If that is not possible, the name is insufficient. Good test names emerge when the developer formulates the behavior in natural language first: "It applies a 10% discount when the order total exceeds 100 euros", and then translates that directly into a method name in snake_case. This discipline also forces the developer to actually test isolated scenarios instead of checking everything in one method.

4. Checking isolation and dependencies

A unit test that opens a database connection, makes an HTTP request, or reads the system clock is no longer a unit test. In review, it must be checked: which external dependencies does the system under test have, and are they fully replaced by test doubles (stubs, mocks, fakes)? If a class cannot be tested without side effects, that is a signal that the production architecture needs to be reworked, not that the test is allowed to get more complicated.

Isolation is also a matter of order. Tests that depend on execution order are inherently fragile. In review, it should be asked: does this test fail when it is run alone? Does it fail when it runs in a different order? PHPUnit attributes such as #[Depends] can be used to declare explicit dependencies, but in most cases such dependencies are a sign of missing isolation.

5. Boundary values and negative scenarios

Tests that only cover the happy path are the most common quality problem in PHP projects. In review, it must be actively asked: where are the boundary values of this function, and are they tested? Empty lists, null values, maximum values, negative numbers, empty strings, these inputs are frequently the source of production bugs. A discount calculator that produces a division by zero for 0 items would have been caught by a single edge case test.

Negative scenarios do not just mean checking exceptions. They also mean: what happens when the service is unavailable? What happens with race conditions? What happens when an external API returns an empty array instead of a list? In review, the reviewer should check for every test class whether exception and boundary scenarios are covered with the same care as the standard cases. PHPUnit offers assertThrows and data providers to cover these scenarios systematically.

<?php

declare(strict_types=1);

namespace Tests\Unit\Domain\Pricing;

use App\Domain\Pricing\DiscountCalculator;
use App\Domain\Pricing\Exception\InvalidQuantityException;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;

/**
 * Edge-case coverage for DiscountCalculator.
 */
final class DiscountCalculatorEdgeCasesTest extends TestCase
{
    /** @return array<string, array{int, float}> */
    public static function discountBoundaryProvider(): array
    {
        return [
            'zero items returns zero' => [0, 0.0],
            'exactly at threshold returns min discount' => [10, 5.0],
            'above threshold returns max discount' => [100, 20.0],
        ];
    }

    #[DataProvider('discountBoundaryProvider')]
    public function it_calculates_correct_discount_for_boundary_quantities(
        int $quantity,
        float $expectedDiscount
    ): void {
        $calculator = new DiscountCalculator();
        self::assertSame($expectedDiscount, $calculator->calculate($quantity));
    }

    public function it_throws_for_negative_quantity(): void
    {
        $this->expectException(InvalidQuantityException::class);
        $this->expectExceptionMessage('Quantity must not be negative');

        (new DiscountCalculator())->calculate(-1);
    }
}

6. Spotting mock abuse

Mocks are the most powerful and most commonly abused tool in PHPUnit. In review, two anti-patterns deserve specific attention: first, tests that set up so many mocks that the actual production code barely runs at all, in which case the test only checks whether methods are called in a certain order, not whether the result is correct. Second, tests that mock internal methods of the class under test. That causes refactorings to force test changes even though the observable behavior stays the same.

The rule of thumb in review: more than three mocks in a single unit test is a warning sign. In such cases, it should be asked whether the class carries too many responsibilities, or whether an integration test would be a better fit. Mocks should isolate boundaries to the outside world: HTTP clients, database repositories, email services. They should not be used to simulate or bypass internal behavior.

7. Mutation testing as an objective measure

Code coverage is a notoriously unreliable metric for test quality. 100% coverage means every line was executed, not that the correct behavior was checked. Mutation testing with Infection solves this problem: the tool automatically changes the production code (mutates it) and checks whether the test suite detects the mutation and reports a failure. A test that survives a mutation is a test that does not actually secure the behavior in question.

In a team context, mutation testing is best suited for critical business logic classes. Integrating it into the CI process with a minimum mutation score (e.g. 80%) gives the review process an objective basis: if the mutation score falls below the threshold, additional tests must be written. This approach prevents coverage gaming, meaning the writing of tests that generate coverage but do not really check anything, from staying undetected.

<?php

// infection.json5: Mutation Testing configuration for PHPUnit projects
{
    "timeout": 10,
    "source": {
        "directories": ["src/Domain"],
        "excludes": ["src/Domain/*/Exception"]
    },
    "logs": {
        "text": "infection.log",
        "html": "infection.html",
        "json": "infection.json"
    },
    "minMsi": 80,
    "minCoveredMsi": 85,
    "mutators": {
        "@default": true,
        "PublicVisibility": false,
        "ProtectedVisibility": false
    },
    "phpUnit": {
        "configDir": "."
    }
}

// Run: vendor/bin/infection --threads=4 --show-mutations
// CI: exit code non-zero when minMsi not reached, blocks merge

8. Checklist for the test review process

A structured test review process starts with a checklist that the team accepts as a standard. The most important points: is the test name a complete behavioral statement? Does every assertion check observable behavior, not an implementation detail? Are external dependencies fully replaced by test doubles? Are there tests for boundary values, empty inputs, and error cases? Can every test run independently of other tests?

In the practical review process, this means: the reviewer does not just scroll through the diff, but locally runs vendor/bin/phpunit --filter=NewTestName to see whether the test runs in isolation. They read the test name and formulate in one sentence what they expect, then check whether the assertions actually guarantee exactly that. They actively look for missing scenarios, not just syntax problems. This attitude costs more time up front but, in total, saves more debugging time than it costs.

9. Review criteria compared

Not all review criteria carry equal weight. The following table shows which properties genuinely make a test more valuable and which common metrics can be misleading.

The decisive point: mutation testing is the most objective metric, because it evaluates not by test structure but by actual behavioral safeguarding. Teams that integrate Infection into their CI process regularly report that coverage numbers and mutation scores barely correlate, a strong indication that coverage alone is an insufficient review criterion.

10. Summary

Test reviews in PHP teams become effective when they are not reduced to coverage numbers and green CI runs. The decisive criteria are: test names that make complete behavioral statements; assertions that specifically check the observable outcome; complete isolation of external dependencies; systematic boundary value and error scenario tests; and mutation testing as an objective quality measure.

The organizational lever lies in a jointly agreed checklist that is binding in the review process. Teams that establish this standard report, after a few months, noticeably fewer regression bugs, because their tests do not just execute code but actually secure the behavior that the next developer and the next deploy rely on.

11. FAQ: PHPUnit Test Reviews in Teams