Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Prophecy in Sonata project #624

Closed
VincentLanglet opened this issue Mar 25, 2020 · 9 comments · Fixed by #1369
Closed

Remove Prophecy in Sonata project #624

VincentLanglet opened this issue Mar 25, 2020 · 9 comments · Fixed by #1369

Comments

@VincentLanglet
Copy link
Member

Cf: sebastianbergmann/phpunit#4141

Should we try to remove the dependency and re-write the tests with prophecy ?

@greg0ire
Copy link
Contributor

I think we could use the trait contributed here: phpspec/prophecy-phpunit#20

There will probably be a rector for this soon

@VincentLanglet
Copy link
Member Author

@sonata-project/contributors I feel like this could be reconsidered

Here, prophecy was recommended: #89

But if I search for createMock or prophesize I find:

  • 225 prophesize and 381 createMock/createStub for SonataAdmin
  • 33 prophesize and 61 createMock for SonataDoctrineORM

And I reviewed more PR with createMock/createStub than prophesize recently.

Shouldn't we recommend to follow the majority ?
IMHO both are fine, but if we want some consistency it could be easier to change all the prophesize rather than change and all the createMock/createStub.

@greg0ire
Copy link
Contributor

Another argument in favor of PHPunit is how better it plays with PHPStan and Psalm, in my experience it is easier to work with, but maybe my experience is outdated.

@jordisala1991
Copy link
Member

IMO phpuniy has improved a lot since when we recommended using prophecy. Now they look similar and we could consider stop using it.

@core23
Copy link
Member

core23 commented Aug 15, 2020

I prefer prophecy, because you can better mock method calls with different arguments. PHPUnit still requires the exact call order. You can also archive the same result with writing less code. (And the API is similar in Java :D)

prophecy:

        $this->request->get('start')->willReturn('2016-03-01');
        $this->request->get('end')->willReturn('2016-03-19 15:11:00');

phpunit

        $this->request->expects(static::exactly(2))
            ->method('get')
            ->withConsecutive(
                ['start'],
                ['end'],
            )
            ->willReturn(
                '2016-03-01',
                '2016-03-19 15:11:00'
            )
        ;

@VincentLanglet
Copy link
Member Author

PHPUnit still requires the exact call order.

Maybe one day it won't :)
sebastianbergmann/phpunit#4026

I'm sure we will always find people who prefer prophecy and some who prefer phpunit.
But since sebastianbergmann/phpunit#4141, I find weird to recommend prophecy.
And I think having consistency in our tests could be great, which seems easier to get if phpunit is the standard.

@VincentLanglet VincentLanglet changed the title Prophecy has been deprecated in PHPUnit Remove Prophecy in Sonata project Aug 30, 2020
@jordisala1991
Copy link
Member

This is almost done, AFAIK it only needs to be removed from docs now

@VincentLanglet
Copy link
Member Author

This is almost done, AFAIK it only needs to be removed from docs now

Could you provide a TODO list @jordisala1991 ? :)

@VincentLanglet
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants