search

Sylius / ShopApiPlugin

Shop API for Sylius.
https://sylius.com
130 stars 89 forks source link

Init sylius 1.9 upgrade #702

Closed DennisdeBest closed 3 years ago

DennisdeBest commented 3 years ago

Hello, thanks for this bundle it has helped us quite a bit for our Sylius projects. With Sylius just moving to 1.9 I thought I would have a look at updating this bundle to be compatible.

Most of it works now but there are still a few test failures that I didn't really get

Tests: 209, Assertions: 583, Errors: 2, Failures: 5.

So I thought I'd share the start and when I get some more time I'll try and fix the latest errors.

DennisdeBest commented 3 years ago

There is now still a PHPSpec error from 

- use Symfony\Component\HttpKernel\Event\FilterControllerEvent;
+ use Symfony\Component\HttpKernel\Event\ControllerEvent;

In the RequestChannelEnsurer as the ControllerEvent is final.

I'm not yet very used to PHPSpec so I have no idea how to fix this for the moment

mamazu commented 3 years ago

Hello and thank you for your contribution. First of all to answer your question: If a class is final you can't mock it. Try to find an interface you can mock or if this class has no interface you can instantiate it.

Secondly you are also doing the update from Symfony 4 to 5 which is not necessary. It will force people to use an older version of Sylius because they don't want to (or can't) update to a newer version of Symfony. The same also goes for the test frameworks. Maybe we should try to limit the scope of this pull request to just the stuff we need to get it to work with Sylius 1.9.

DennisdeBest commented 3 years ago

I understand your point, I will have a look at keeping it on symfony 4.4

mamazu commented 3 years ago

Thanks, if you need anymore help feel free to ask.

lchrusciel commented 3 years ago

Hey Dennis,

thanks a lot for your contribution. I picked up from here and will continue in separate PR. Welcome to Sylius community :)

Closing in favor of #703.

DennisdeBest commented 3 years ago

Thanks, sorry I haven't had the time to finish this I'll follow your new pull request see if there is anything I can do

lchrusciel commented 3 years ago

Dennis new version should be already supported on master branch - I will run a few tests and upgrades to see if there is something I want to add to the next release and then tag this plugin