Skip to content

Stop mocking immutable/value objects #176

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

Merged
merged 1 commit into from
Jun 27, 2017
Merged

Stop mocking immutable/value objects #176

merged 1 commit into from
Jun 27, 2017

Conversation

fbourigault
Copy link
Contributor

@fbourigault fbourigault commented Jun 26, 2017

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Related tickets partially #175
Documentation n/a
License MIT

What's in this PR?

This is a cleanup of tests to stop mocking immutable/value objects.

Psr\Http\Message\MessageInterface and Psr\Http\Message\ResponseInterface
mocks are replaced with GuzzleHttp\Psr7\Request and
GuzzleHttp\Psr7\Response respectively.

Http\Promise\Promise mocks are replaced either with
Http\Client\Promise\HttpFulfilledPromise or
Http\Client\Promise\HttpRejectedPromise depending on the test
requirements.

Why?

This make tests easier to read, thus to maintain, etc... Also this will help me to solve #175 as it will require some profiling rewrite.

@fbourigault fbourigault changed the title stop mocking immutable/value objects Stop mocking immutable/value objects Jun 26, 2017
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Type annotations does not need to be changed. But I'm happy with this. Just make sure the tests passes.

@fbourigault
Copy link
Contributor Author

Type annotations does not need to be changed. But I'm happy with this. Just make sure the tests passes.

I was hesitant about this, and ended to change temp to reduce the use list. But I will revert those as we are dealing with the PSR interfaces not guzzle implementation.

@fbourigault
Copy link
Contributor Author

I reverted the type annotations, rebased against #177 and reverted to setExpectedException. Things should be fine now.

@fbourigault
Copy link
Contributor Author

fbourigault commented Jun 26, 2017

There is still an error when dependencies are installed using the --prefer-lowest flag. The error occurs because php-http/httplug 1.0.0 is installed and I use HttpfulfliedPromise in my tests. We have two options here, either bump minimum php-http/httplug requirement to 1.1.0 or make the code compatible with both versions (I don't know if it's possible as FulfilledPrimise catch all exceptions while HttpFulfilledPromise only catches Http Exception).

WDYT?

@Nyholm
Copy link
Member

Nyholm commented Jun 27, 2017

Bump it to 1.1.0. No questions about it.

Psr\Http\Message\MessageInterface and Psr\Http\Message\ResponseInterface
mocks are replaced with GuzzleHttp\Psr7\Request and
GuzzleHttp\Psr7\Response respectively.

Http\Promise\Promise mocks are replaced either with
Http\Client\Promise\HttpFulfilledPromise or
Http\Client\Promise\HttpRejectedPromise depending on the test
requirements.
@fbourigault fbourigault merged commit 7447f5a into php-http:master Jun 27, 2017
@fbourigault
Copy link
Contributor Author

Thank your for reviewing this PR.

@fbourigault fbourigault deleted the stop-mocking-immutable-value-objects branch June 27, 2017 10:53
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 this pull request may close these issues.

2 participants