You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently there's no option for Collision to print the Throwable::getPrevious() exception alongside the main one, but it'd be nice to have one.
Our use case specifically is this: we have Laravel integration tests that make HTTP requests. When a HTTP code assertion fails due to an exception during the request, Laravel attempts to add some debugging information by adding this kind of text stack trace to the exception message (by modifying Exception::$message through reflection):
The following exception occurred during the last request:
TypeError: [redacted]: Return value must be of type float, null returned in [redacted]
Stack trace:
.. a lot of trace lines here
#84 /app/tests/TestCase.php(144): Illuminate\Foundation\Testing\TestCase->postJson()
.. a lot of trace lines here
#96 /app/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(63): PHPUnit\Framework\TestSuite->run()
#97 /app/vendor/phpunit/phpunit/src/TextUI/Application.php(177): PHPUnit\TextUI\TestRunner->run()
#98 /app/vendor/phpunit/phpunit/phpunit(99): PHPUnit\TextUI\Application->run()
#99 {main}
----------------------------------------------------------------------------------
[redacted]: Return value must be of type float, null returned
at tests/[redacted]
290▕ ],
291▕ ])
➜ 292▕ ->assertSuccessful()
293▕ ->assertJson([
294▕ 'meta' => [
295▕ 'rules' => [
Tests: 1 failed (1 assertions)
Duration: 20.35s
Although this does work, it'd be much nicer to have the original exception rendered natively by Collision. Laravel could then set Exception::$previous instead of the Exception::$message and it'd be rendered something like this:
at tests/[redacted]
290▕ ],
291▕ ])
➜ 292▕ ->assertSuccessful()
293▕ ->assertJson([
294▕ 'meta' => [
295▕ 'rules' => [
Caused by an exception at tests/[redacted]:103: Return value must be of type float, null returned
101▕ // code
102▕ // code
➜ 103▕ // code
104▕ // code
105▕ // code
106▕ // code
Tests: 1 failed (1 assertions)
Duration: 20.35s
That would improve QoL by reducing the stack trace (from 100+ lines of vendor code to just a few lines that are actually important) and rendering it in an eye-friendly way.
Besides the need of a new option and some additions to the inspectors, this addition looks trivial to implement, so if this is something that can get merged, I can PR the changes.
The text was updated successfully, but these errors were encountered:
So it turns out it does work, just not at all how you expect it to. The "previous" exception is included in the trace, but this is how it currently looks:
instead of the actual exception class name, we get NunoMaduro\Collision\Exceptions\TestException
throw location is duplicated twice
only 1 usable trace line is included. If you run tests with SHELL_VERBOSITY=1, it does show the complete trace - but in that case it also includes 100 trace lines from vendor which clutters the output. I'd rather see the complete stack trace of userland code than 1 line.
Hey.
Currently there's no option for Collision to print the
Throwable::getPrevious()
exception alongside the main one, but it'd be nice to have one.Our use case specifically is this: we have Laravel integration tests that make HTTP requests. When a HTTP code assertion fails due to an exception during the request, Laravel attempts to add some debugging information by adding this kind of text stack trace to the exception message (by modifying
Exception::$message
through reflection):Although this does work, it'd be much nicer to have the original exception rendered natively by Collision. Laravel could then set
Exception::$previous
instead of theException::$message
and it'd be rendered something like this:That would improve QoL by reducing the stack trace (from 100+ lines of
vendor
code to just a few lines that are actually important) and rendering it in an eye-friendly way.Besides the need of a new option and some additions to the inspectors, this addition looks trivial to implement, so if this is something that can get merged, I can PR the changes.
The text was updated successfully, but these errors were encountered: