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

Recommend Prophecy for test doubles creation #198

Merged
merged 1 commit into from
Mar 6, 2017

Conversation

greg0ire
Copy link
Contributor

Fixes #89.

Since phpunit 4.5, phpunit requires and [integrates](https://phpunit.de/manual/current/en/test-doubles.html#test-doubles.prophecy)
the [phpspec/prophecy](https://github.com/phpspec/prophecy).
Historically, Sonata has been using [the built-in test doubles implementation](https://phpunit.de/manual/current/en/test-doubles.html),
but has deciding to move to Prophecy, which is more concise than its built-in counterpart is most cases.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe link to the discussion, if someone is interested?

@OskarStark
Copy link
Member

LGTM 👍

@greg0ire greg0ire force-pushed the recommend_prophecy branch from 6ea1599 to e1e9a26 Compare August 24, 2016 09:05
@greg0ire
Copy link
Contributor Author

@OskarStark : I changed some things, please review it again entirely.

@@ -97,6 +97,24 @@ Some rules have to be respected about the test:
* Most of the time, the test class should have the same name as the targeted class, suffixed by `Test`.
* The `@expectedException*` annotations are prohibited. Use `PHPUnit_Framework_TestCase::setExpectedException()`.

##### Using test doubles

Since phpunit 4.5, phpunit requires and [integrates](https://phpunit.de/manual/current/en/test-doubles.html#test-doubles.prophecy)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phpunit -> PHPUnit

Because, it is it name. 😛

@greg0ire greg0ire force-pushed the recommend_prophecy branch from e1e9a26 to 1bc94b9 Compare August 24, 2016 09:10
@OskarStark
Copy link
Member

Good job @greg0ire, i like it 👍

@OskarStark OskarStark added the RTM label Aug 24, 2016
@@ -97,6 +97,24 @@ Some rules have to be respected about the test:
* Most of the time, the test class should have the same name as the targeted class, suffixed by `Test`.
* The `@expectedException*` annotations are prohibited. Use `PHPUnit_Framework_TestCase::setExpectedException()`.

##### Using test doubles

Since version 4.5, PHPUnit requires and [integrates](https://phpunit.de/manual/current/en/test-doubles.html#test-doubles.prophecy)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we link to "current" or version 4.5 explicitely?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should link to current b/c we are using the current version in our software. No need to link to outdated instructions

@OskarStark OskarStark added RTM and removed RTM labels Aug 24, 2016
@jordisala1991
Copy link
Member

LGTM, we should start moving to prophecy 👍

@greg0ire
Copy link
Contributor Author

@soullivaneuh this is 6 months old. What's the deal?

@soullivaneuh soullivaneuh merged commit 6c2d7fe into sonata-project:master Mar 6, 2017
@greg0ire greg0ire deleted the recommend_prophecy branch March 6, 2017 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants