Official Sylius plugin

[Zales0123]

Hello, @odiseoteam, one more time!

We would really love to display this plugin on our officially accepted plugins list (https://sylius.com/plugins/), as I strongly believe it could be useful for other people in the community :)

The only problem is, we have some strict requirements for the plugin that is meant to be displayed on our website. One of the most important is the properly developed test suite. Sadly, your plugin has neither unit (PHPSpec/PHPUnit) nor acceptance (Behat) tests. Do you think you would be able to provide them? It would be great if we could extend our accepted plugins set with SyliusReportPlugin :)

The same refers to your other plugins - none of them has tests, which is disqualifying for them to be official plugins.

If you have any question, please, feel free to ask me. You can also contact me at mateusz.zalewski@sylius.com, if you want to talk about some ways we can help you with tests in your plugins :)

Nevertheless, thank you for choosing Sylius and for your great job!

[odiseoteam]

Hi again @Zales0123!

Thank you for wanting to add our plugins to the officially accepted plugins list. As you recommend we will add the properly test suites to be able to satisfy the Sylius plugin requirements.

I will let you know when we have done.

[odiseoteam]

Hello @Zales0123. We already did the tests as you suggested. Please, review it and tell me how you see it.
We have also written the tests of our other plugins, i will apreciate if you can review their too to be added to the official Sylius plugin list.

Thanks!

[Zales0123]

Hello @odiseoteam, sorry for a long response time :) I've checked out your test on this plugin and it's wonderful you'd added them! Here are some of my suggestions to make them even better

PHPSpec

1. spec/Controller/ReportControllerSpec.php: we usually don't write specifications for controllers and there is one very good reason for that - controllers should not contain too much logic, they should only operate on request, pass some data to other services and return response. If something is hard to test with PHPSpec (you need some crazy private functions to handle lot's of mocks etc.), then it's probably good to consider the design of the class and is it properly decoupled :)
2. Regarding forms, to test them it's better to use PHPUnit or depend on Behat tests, such spec as this one does not provide any value at all (and it's quite frustrating to specify the class with used builder pattern)
3. Remember about type hints and return types everywhere ;)

Behat

1. It's a good thing to remember, that Behat scenarios should describe a business value that a specific feature is providing. They should then be as less technical as they can be - as no technical details are de facto fulfillment of business value :) Therefore it's a good thing to avoid quasi-technical phrases like I fill the code with... and use some more neutral terms like I specify code as...
2. Each feature should be tested as fully as possible - you can probably do some more things with reports than only editing the name :)
3. The biggest missing test is for rendering different types of reports with different types of data fetchers and renderers. This scenario describes only tiny, a single thing that is displayed on report show page.
4. It's nice you've already decided to use PageObject pattern in your scenarios implementations 👍
5. The same with transformers, not everyone appreciates their power 🎉

Summary

I must say, I'm really happy I've prompt you to do some testing, this plugin is far more clean now :) Of course, there are still things to improve, but I strongly believe that you'll become more and more fluent and comfortable with tests, when you'll be doing them later (hopefully with respect to TDD and BDD methodology 😄). Good luck with your adventure with Sylius!