phpstan-phpunit icon indicating copy to clipboard operation
phpstan-phpunit copied to clipboard

Detect risky PHPUnit test

Open theofidry opened this issue 1 month ago • 2 comments

Feature request

In the following scenario:

function test_exception_state(): void {
  $myService = new MyService();

  try {
    $myService->doSomethingWrong();

    // missing: $this->fail();
  } catch (MyException $exception) {
    $this->assertSame('Custom message', $exception->getMessage());
    $this->assertSame('Another property', $exception->smthNotTypicalOfException);
  }
}

Essentially, a case where we want to do an explicit try/catch because expectExceptionObject() doesn't cut it (want to catch a Throwable instead of test some state of the exception besides message/code), it is easy to forget a fail() statement and leading to a risky test.

Maybe it leading as a risky is fine... But maybe it's a kinda of issue that PHPStan could detect & report. Just an idea.

Did PHPStan help you today? Did it make you happy in any way?

Today it did not help, but I didn't run it either so cannot blame it :) I like PHPStan otherwise!

theofidry avatar Nov 18 '25 22:11 theofidry

I think this might be hard to detect.

in this simple example, I think we could e.g. detect that there is a path thru the test which does not contain any assertions. but as soon as there would be a singe assertion in the try-block this would no longer work.

staabm avatar Nov 19 '25 09:11 staabm

I think this is another application of a "missing return" rule idea. There could be an ExecutionEndNode rule, looking at statement throw points, and if there's not a single throw point for AssertionFailedError, then report an error.

ondrejmirtes avatar Nov 19 '25 09:11 ondrejmirtes