markitosgv/JWTRefreshTokenBundle

Replace phpspec tests with phpunit tests?

webignition opened this issue · 6 comments

Symfony 6.0 suppport is expected to be introduced in PR #284 and this is being blocked by phpspec/phpspec support for Symfony 6.0.

There are various issues regarding the capacity for phpspec/phpspec to smoothly support Symfony 6.0 and/or PHP 8.1 which may well delay things somewhat.

One option to consider with regards to be able to support Symfony 6.0 and/or PHP 8.1 (as well as future Symfony and PHP versions) is the re-implementation of current phpspec tests as phpunit tests.

From my (somewhat limited) understanding, phpspec is ideal for specifying the behaviour of code prior to the creation of any code in a way that is not practical for approaches that use phpunit or similar.

If phpspec is not being used to specify behaviour in advance of any code being written but is instead being used to verify the behaviour of code that already exists (or has just been created), the equivalent utility could be achieved from re-implementing as phpunit tests without using phpspec.

I can see that the coupling to phpspec is blocking support for Symfony 6.0 and that this could arise again with future Symfony or PHP version support matters. I can see how re-implementing what is provided by the phpspec tests as phpunit tests will remove the option for similar matters in the future since phpunit is not dependent on any Symfony packages.

I am by no means suggesting that such as re-implementation is trivial (it is no doubt a large undertaking) and I have no real understanding of why phpspec is being used so as to understand if no longer using phpspec is a viable option.

Nonetheless, I just want to check if this is something that could be achieved. I am more than happy to help in the re-implementation such that test coverage and the value of tests remains unchanged.

What are your thoughts?

@mbabker I take it from your thumbs up that you're in support of this then? Just double checking before I make a start on the changes.

@markitosgv Any thoughts on this?

Just in case this wasn't known already, rector should be able to help you on the way quickly. Saying should, as I haven't tried it myself.

https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#phpspectophpunit

@darthf1 thanks, I wasn't aware of that specific set of rector rules.

I'd be surprised if rector could do all that is needed in one go but, as you say, it'll certainly get things going on the right direction.

@webignition its ok to make this change to add support to symfony 6!

Great, I'll go ahead with the changes.

Since there is a little wiggle room in terms of interpreting the best manner in which to carry out the conversion, I'll do my best just to run things through rector with zero additional changes. This should help to avoid any bike shedding discussions for the time being :)