search

bugsnag / bugsnag-laravel

BugSnag notifier for the Laravel PHP framework. Monitor and report Laravel errors.
https://docs.bugsnag.com/platforms/php/laravel/
MIT License
876 stars 129 forks source link

Don't allow testbench 5.6 or testbench-core 3.3 #441

Closed imjoehaines closed 3 years ago

imjoehaines commented 3 years ago

Goal

AbstractTestCase::getServiceProviderClass has an $app parameter which was removed in 5.6/3.3 of the testbench packages. This broke our test suite as we can't be compatible with both the old and new versions with the same file

Restricting the version range to avoid the new versions fixes this in the short term, but we'll likely need further changes to support these packages in the future as future versions of PHPUnit will eventually force us to update

imjoehaines commented 3 years ago

Opened https://github.com/GrahamCampbell/Laravel-TestBench-Core/issues/12 and https://github.com/GrahamCampbell/Laravel-TestBench/issues/28 regarding this

GrahamCampbell commented 3 years ago

You can just change your code to not have the app param.

imjoehaines commented 3 years ago

You can just change your code to not have the app param.

That works with the new version, but we have to use older versions on older PHP versions so we'd still get a "Declaration must be compatible" error

GrahamCampbell commented 3 years ago

Hmmm, that is a pain. When I made this change, I assumed deleting params was fine, since PHP doesn't complain when people are overriding them. Seems I was wrong... maybe time for an RFC to fix this behaviour in PHP. Method definitions should be contravariant, not just in typing of parameters.

GrahamCampbell commented 3 years ago

I think the best thing you can do is just not use the new code, until you are ready to drop old PHP versions.

imjoehaines commented 3 years ago

Hmmm, that is a pain. When I made this change, I assumed deleting params was fine, since PHP doesn't complain when people are overriding them. Seems I was wrong... maybe time for an RFC to fix this behaviour in PHP. Method definitions should be contravariant, not just in typing of parameters.

Yeah that makes sense; given you can call a method with more parameters than it expects, it should have been fine to remove