[WIP] fix issue #37

[quetzyg]

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Related tickets	fixes #37
Documentation	N/A
License	MIT

What's in this PR?

The changes in this PR, allow the RedirectPlugin to properly track requests using full URLs instead of comparing only the URI.

Checklist

• Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix

To Do

• Fix tests

[dbu]

__toString is indeed not allowed to throw an exception - its unfortunate we don't see the stack trace.

is the test using a mock for UriInterface? could we use a real instance of Uri for the test instead?

[quetzyg]

I tried running with -vvv:

vendor/bin/phpspec -vvv spec/Plugin/RedirectPluginSpec.php

but there was no change in the output I got.

[sagikazarmark]

I think the problem is that __toString is not expected to be called on the URI object. So far we used getRequestTarget:

https://github.com/php-http/client-common/blob/master/spec/Plugin/RedirectPluginSpec.php#L47

[dbu]

mark, would you recommend changing the mock, or do you agree with me that we better would use a real request implementation? there would be no harm in adding `guzzlehttp/psr7` to require-dev...

[sagikazarmark]

Yes, but not in this PR. I would rather fix the tests by altering the spec expectations and rewrite the tests afterwards.

[sagikazarmark]

I would be happier if we could solve the promise/exception thing first.

[sagikazarmark]

@quetzyg can you please check my recommended fix?

[quetzyg]

Hi @sagikazarmark,

I'll try to find some time to make the changes to the spec expectations.

[joelwurtz]

I've fixed test and add a specific one for the one mentioned in the issue under #43

[dbu]

continued in #43. thanks @quetzyg for your contribution!

[quetzyg]

Soz @dbu, I've been swamped and I haven't had time to check phpSpec properly to update the tests.
Glad @joelwurtz stepped up.

Keep up the good work guys!