Rewrite tests to avoid mocking HTTP messages

[sagikazarmark]

Q	A
Bug?	no
New Feature?	no
Version	<=1.3.x

Actual Behavior

Testing our code is pretty hard because of all the mocks necessary to test HTTP Message related code and Plugin Chain.

Expected Behavior

It should be pretty easy to understand tests, modify behaviour, provide new test cases, etc.

Possible Solutions

• Rewrite specifications to simple unit tests using the MockClient.
• Rewrite specifications to use actual Message objects (so only remove mocking, but preserve specifications)

WDYT?

[joelwurtz]

Rewrite specifications to simple unit tests using the MockClient.

:+1: but would need to do this : https://github.com/php-http/mock-client/issues/3

[dbu]

what is the problem, can php-spec not use actual message objects instead of mocks? i'd rather stay consistent with the testing tool if that is not adding another complexity

[sagikazarmark]

Of course PHPSpec can use actual objects, but it would be pointless to use the MockClient in that case, since PHPSpec (obviously) does not allow such assertions.

[dbu]

nothing is obvious for me with phpspec :-P

should we mix phpunit and phpspec there, or just write some phpunit tests in addition? we should not overcomplicate things, having one way to do tests would keep things simple but if phpspec makes it super complicated to assert things on the MockClient its probably the lesser evil to have to kind of tests.

[joelwurtz]

asserting things with phpspec would make us using phpunit to do assert, so we should go full phpunit IMO

[sagikazarmark]

I think using actual message implementations is kind of a midway. But it would not work with mock client.

If we want to use MockClient here, we need PHPUnit, and in that case, I would probable migrate all tests to PHPUnit. Downside is that it's huge amount of work.

But do we really want to use MockClient?

[dbu]

if MockClient makes things harder, can we use message implementations with a phpspec mocked client? i see no reason not to do that if thats easier.

[joelwurtz]

Problem is not the MockClient, if we use actual message implementations we still have to do asserts on the output which will be also message implementations (and not prophecy) so we will have the same problem IMO

[fbourigault]

I wrote those days tests which use mesage implementations. It's phpunit based, but I felt comfortable with using message implementation instead of unreadable mock. I don't know if this could work with phpspec, but it probably worth it to try. See https://github.com/m4tthumphrey/php-gitlab-api/pull/191/files#diff-48ee2e1f1f63bacd2d694cc463c0c8fe for my phpunit example.

[sagikazarmark]

HTTP Messages are kind of Value Objects, so I think it was a mistake that we started mocking them. PHPSpec does not force using mocks, especially not when we don't really mock behaviour of HTTP messages. So just using an implementation should work IMO.