Fix the deprecation warnings #45
Conversation
There was a problem hiding this comment.
this looks like the right solution to me, totally sophisticated enough :-D
most builds are failing to install dependencies. that is not a change of your PR, but it would be good to see the changes build on a few more versions... any chance you could check what is going on with installing the clients?
src/HttpClientTest.php
Outdated
| @@ -103,6 +102,7 @@ public function testSendWithInvalidUri() | |||
| $this->defaultHeaders | |||
| ); | |||
|
|
|||
| $this->expectException(\Psr\Http\Client\NetworkExceptionInterface::class); | |||
There was a problem hiding this comment.
please use the classes used for these
|
@dbu checking the initial PR of @Nyholm I think the BC trait is an overkill.. Since Tobias is adding the return types a new major version is needed.. Because of the return types we must drop support for PhpUnit 7 and move to 8, hence the trait is useless. The alternative would be to apply the sf BC trait here as well: https://github.com/symfony/symfony/blob/v4.4.1/src/Symfony/Bundle/FrameworkBundle/Test/ForwardCompatTestTrait.php
I think most of them are failing due to this: My suggestion would be to finish here.. tag a new version and then check the clients. |
|
right, master of client-integration-tests is already designated as version 3 and i think with adding the return type declarations to setUp and tearDown, we should do a new major version of the integration tests. if we don't support phpunit 7 anymore, clients can't be tested with php 7.1. therefore i think your trait makes sense and we can merge this as is, and clients using the test suite can update to version 3 at their own pace. the forward compatibility trait only avoids the BC break for people using phpunit 7 and overwriting one of our tests setUp method. narrowing the return value to do you want to |
|
ok then.. done.. |
* Support PHPUnit 8 * Added return types * Show Guzzle tests * minor * Fix the deprecation warnings (#45) * Fix the deprecation warnings * Provide a bc way for assertStringContainsString Co-authored-by: Mponos George <gmponos@gmail.com>
What's in this PR?
@Nyholm I started this PR from your branch. I hope you haven't already been working on it..
The solution is not a very much sophisticated one.. but I gues for now it can do..