laravel-doctrine / orm

An integration library for Laravel and Doctrine ORM
MIT License
828 stars 178 forks source link

Issues with running PHPUnit coverage with Xdebug when middleware runs #562

Closed joshmurrayeu closed 1 month ago

joshmurrayeu commented 1 year ago

I've noticed an issue where if the logging middleware has been enabled, PHPUnit won't be able to complete the code coverage report and show it within PhpStorm. Please see the screenshot below:

image

As you can see on line 10, it's destructing the Middleware class once the code has finished executing. Typically, whenever the Target class [config] does not exist. exception is thrown, something is being called at the wrong time e.g. similar to using a Laravel factory in a @dataProvider or using Container methods within the setUp before the application has actually been able to boot up.

I've tried debugging for the last 20 minutes or so and the cause is this line: https://github.com/laravel-doctrine/orm/blob/106f0dc291b2111de67579baf655cb4eb2c1c242/config/doctrine.php#L90-L92

A possible fix is:

'middleware' => (new Illuminate\Support\Collection(env('DOCTRINE_MIDDLEWARE') ?? []))->toArray()

And of course, adding DOCTRINE_MIDDLEWARE=Doctrine\DBAL\Logging\Middleware to your .env.

I think this "solution" is a bit ugly. It does give a better ability to enable and disable middleware based on your environment. I haven't tested caching the configuration, yet.. although, I wouldn't anticipate any issues doing so.

It could be possible I've put a piece of code in the wrong place, so I'll use my fix for the time being. I'll update this issue should I find the root cause (if one exists).

@eigan What are your thoughts?

martio commented 2 months ago

I have the same...

TomHAnderson commented 1 month ago

This could be a welcome fix. Care to try the 3.0.0-dev branch?

TomHAnderson commented 1 month ago

@joshmurrayeu I don't think 3.0.0 will fix this issue. I like the idea of not including failing middleware based on environment. Coverage is a special case.