Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specific exceptions to be marked as failures instead of errors #4983

Closed
longwave opened this issue Jun 8, 2022 · 4 comments
Closed
Labels
type/enhancement A new idea that should be implemented

Comments

@longwave
Copy link

longwave commented Jun 8, 2022

In the Drupal project we are using Mink to drive emulated web browsers and perform assertions against web content. Mink throws its own exception when an assertion fails, and by default PHPUnit catches this and reports it as an error:

PHPUnit 9.5.20 #StandWithUkraine

Testing Drupal\Tests\node\Functional\NodeTitleTest
E                                                                   1 / 1 (100%)

Time: 00:07.227, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\node\Functional\NodeTitleTest::testNodeTitle
Behat\Mink\Exception\ExpectationException: Title found

/var/www/html/drupal/core/tests/Drupal/Tests/WebAssert.php:540
/var/www/html/drupal/core/tests/Drupal/Tests/WebAssert.php:283
/var/www/html/drupal/core/modules/node/tests/src/Functional/NodeTitleTest.php:94
/var/www/html/drupal/vendor/phpunit/phpunit/src/Framework/TestResult.php:726

ERRORS!
Tests: 1, Assertions: 17, Errors: 1.

However, we would like these to be treated as assertion failures instead of errors. When presented with multiple test failures, it is usually easier to tackle the real errors first, and so being able to identify assertion failures separately is a useful feature.

Is it possible to add a feature to PHPUnit where specific classes of exceptions can be reported as failures? I have already tried catching the exception and throwing a new AssertionFailedError from onNotSuccessfulTest() but this loses the stack trace, which is critical to debugging a test fail, so it would be helpful if this feature could be configured in PHPUnit directly.

The Drupal issue where this is being discussed is https://www.drupal.org/project/drupal/issues/3271214

@longwave longwave added the type/enhancement A new idea that should be implemented label Jun 8, 2022
@sebastianbergmann
Copy link
Owner

I have already tried catching the exception and throwing a new AssertionFailedError from onNotSuccessfulTest() but this loses the stack trace

You do not lose the stack trace information if you chain the exceptions.

@longwave
Copy link
Author

longwave commented Jun 9, 2022

When process isolation is used, the previous exception is lost due to #1351 - the \PHPUnit\Framework\Exception::$serializableTrace property and constructor do not take into account any previous exceptions. Maybe that constructor should be extended to handle this?

I see \PHPUnit\Framework\ExceptionWrapper is designed to handle this case, but if I throw this instead of AssertionFailedError, or an AssertionFailedError wrapped in an ExceptionWrapper, or vice versa, this doesn't work either - I can get a failure and no stack trace, or an error with a stack trace.

@alexpott
Copy link

@sebastianbergmann thanks for thinking about the technical solution.

So while we can technically fix this in Drupal I think there is a question about whether this is the correct fix. Integrating PHPUnit with something like Mink that provides its own assertions is really useful. Having to use reflection to handle setting properties on exceptions feels like a not great solution. As an alternative is it possible to explore the question asked in the original comment:

Is it possible to add a feature to PHPUnit where specific classes of exceptions can be reported as failures?

@sebastianbergmann
Copy link
Owner

Won't implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A new idea that should be implemented
Projects
None yet
Development

No branches or pull requests

3 participants