From 78e2089140a518347e9e44dd85a4f72041e51a01 Mon Sep 17 00:00:00 2001 From: daniel Date: Mon, 26 Oct 2020 23:03:36 +0100 Subject: [PATCH] PHPUnit tests: prevent real Guzzle requests from tests. Per I1702c11928f8760, tests should fail when trying to make a HTTP request, unless they have explicitely defined the desired response. This patch implements the same behavior for the newly introduced createGuzzleClient() method. Bug: T263816 Bug: T262443 Change-Id: Ie3c2efb1387ecc5bc29ff1749cae4bb33e636e9f --- tests/phpunit/mocks/MockHttpTrait.php | 51 +++++++++++++-- .../phpunit/mocks/NullHttpRequestFactory.php | 10 +++ .../MediaWikiIntegrationTestCaseTest.php | 9 +++ tests/phpunit/tests/MockHttpTraitTest.php | 64 +++++++++++++++++++ 4 files changed, 129 insertions(+), 5 deletions(-) diff --git a/tests/phpunit/mocks/MockHttpTrait.php b/tests/phpunit/mocks/MockHttpTrait.php index a65769b1725..c1357c18029 100644 --- a/tests/phpunit/mocks/MockHttpTrait.php +++ b/tests/phpunit/mocks/MockHttpTrait.php @@ -21,6 +21,7 @@ use MediaWiki\Http\HttpRequestFactory; use PHPUnit\Framework\MockObject\MockBuilder; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseInterface; use Psr\Log\NullLogger; /** @@ -45,15 +46,16 @@ trait MockHttpTrait { * Install a mock HttpRequestFactory in MediaWikiServices, for the duration * of the current test case. * - * @param null|string|array|callable|MWHttpRequest|MultiHttpClient $request A list of - * MWHttpRequest to return on consecutive calls to HttpRequestFactory::create(). + * @param null|string|array|callable|MWHttpRequest|MultiHttpClient|GuzzleHttp\Client $request + * A list of MWHttpRequest to return on consecutive calls to HttpRequestFactory::create(). * These MWHttpRequest also represent the desired response. * For convenience, a single MWHttpRequest can be given, * or a callable producing such an MWHttpRequest, * or a string that will be used as the response body of a successful request. * If a MultiHttpClient is given, createMultiClient() is supported. - * If null or a MultiHttpClient is given instead of a MWHttpRequest, - * a call to create() will cause the test to fail. + * If a GuzzleHttp\Client is given, createGuzzleClient() is supported. + * If null is given, any call to create(), createMultiClient() or createGuzzleClient() + * will cause the test to fail. */ private function installMockHttp( $request = null ) { $this->setService( 'HttpRequestFactory', function () use ( $request ) { @@ -87,7 +89,7 @@ trait MockHttpTrait { /** @var HttpRequestFactory|MockObject $mockHttpRequestFactory */ $mockHttpRequestFactory = $this->getMockBuilder( HttpRequestFactory::class ) ->setConstructorArgs( [ $options, new NullLogger() ] ) - ->onlyMethods( [ 'create', 'createMultiClient' ] ) + ->onlyMethods( [ 'create', 'createMultiClient', 'createGuzzleClient' ] ) ->getMock(); if ( $request instanceof MultiHttpClient ) { @@ -98,12 +100,23 @@ trait MockHttpTrait { ->willReturnCallback( [ TestCase::class, 'fail' ] ); } + if ( $request instanceof GuzzleHttp\Client ) { + $mockHttpRequestFactory->method( 'createGuzzleClient' ) + ->willReturn( $request ); + } else { + $mockHttpRequestFactory->method( 'createGuzzleClient' ) + ->willReturnCallback( [ TestCase::class, 'fail' ] ); + } + if ( $request === null ) { $mockHttpRequestFactory->method( 'create' ) ->willReturnCallback( [ TestCase::class, 'fail' ] ); } elseif ( $request instanceof MultiHttpClient ) { $mockHttpRequestFactory->method( 'create' ) ->willReturnCallback( [ TestCase::class, 'fail' ] ); + } elseif ( $request instanceof GuzzleHttp\Client ) { + $mockHttpRequestFactory->method( 'create' ) + ->willReturnCallback( [ TestCase::class, 'fail' ] ); } elseif ( $request instanceof MWHttpRequest ) { $mockHttpRequestFactory->method( 'create' ) ->willReturn( $request ); @@ -249,4 +262,32 @@ trait MockHttpTrait { return $mockHttpRequestMulti; } + /** + * Constructs a fake GuzzleHttp\Client which will return the given response. + * + * @note Not all methods on GuzzleHttp\Client are mocked, calling other methods will + * cause the test to fail. + * + * @param ResponseInterface|string $response The response to return. + * + * @return GuzzleHttp\Client + */ + private function makeFakeGuzzleClient( $response ) { + if ( is_string( $response ) ) { + $response = new GuzzleHttp\Psr7\Response( 200, [], $response ); + } + + $mockHttpClient = $this->createNoOpMock( + GuzzleHttp\Client::class, + [ 'request', 'get', 'put', 'post' ] + ); + + $mockHttpClient->method( 'request' )->willReturn( $response ); + $mockHttpClient->method( 'get' )->willReturn( $response ); + $mockHttpClient->method( 'put' )->willReturn( $response ); + $mockHttpClient->method( 'post' )->willReturn( $response ); + + return $mockHttpClient; + } + } diff --git a/tests/phpunit/mocks/NullHttpRequestFactory.php b/tests/phpunit/mocks/NullHttpRequestFactory.php index 03accea76ea..430bd0eb96b 100644 --- a/tests/phpunit/mocks/NullHttpRequestFactory.php +++ b/tests/phpunit/mocks/NullHttpRequestFactory.php @@ -54,4 +54,14 @@ class NullHttpRequestFactory extends HttpRequestFactory { return new NullMultiHttpClient( $options ); } + /** + * @param array $config + * + * @return \GuzzleHttp\Client + */ + public function createGuzzleClient( array $config = [] ): \GuzzleHttp\Client { + // NOTE: if needed, we can also return a mock here, like we do in createMultiClient() + Assert::fail( "HTTP request blocked. Use MockHttpTrait." ); + } + } diff --git a/tests/phpunit/tests/MediaWikiIntegrationTestCaseTest.php b/tests/phpunit/tests/MediaWikiIntegrationTestCaseTest.php index d54596fb0af..1602eec32f9 100644 --- a/tests/phpunit/tests/MediaWikiIntegrationTestCaseTest.php +++ b/tests/phpunit/tests/MediaWikiIntegrationTestCaseTest.php @@ -385,6 +385,15 @@ class MediaWikiIntegrationTestCaseTest extends MediaWikiIntegrationTestCase { $this->assertTrue( $prevented, 'create() should fail' ); + try { + $httpRequestFactory->createGuzzleClient(); + $prevented = false; + } catch ( AssertionFailedError $e ) { + // pass + } + + $this->assertTrue( $prevented, 'createGuzzleClient() should fail' ); + $multiClient = $httpRequestFactory->createMultiClient(); $req = [ 'url' => 'http://0.0.0.0/' ]; diff --git a/tests/phpunit/tests/MockHttpTraitTest.php b/tests/phpunit/tests/MockHttpTraitTest.php index 5ff0c658cf2..c2559434f97 100644 --- a/tests/phpunit/tests/MockHttpTraitTest.php +++ b/tests/phpunit/tests/MockHttpTraitTest.php @@ -1,6 +1,7 @@ assertSame( $expected, $data ); } + public function provideGuzzleClientData() { + yield [ + 'Hello Wörld', + new GuzzleHttp\Psr7\Response( 200, [], 'Hello Wörld' ), + ]; + yield [ + new GuzzleHttp\Psr7\Response( 404, [ 'Test' => 'hi' ], 'nope' ), + new GuzzleHttp\Psr7\Response( 404, [ 'Test' => 'hi' ], 'nope' ), + ]; + } + + /** + * @dataProvider provideGuzzleClientData + */ + public function testFakeGuzzleClientEmulatesRequests( $response, $expected ) { + $client = $this->makeFakeGuzzleClient( $response ); + + $this->assertGuzzleResponse( $expected, $client->request( 'TEST', 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->get( 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->put( 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->post( 'http://b.example.com' ) ); + } + + /** + * @dataProvider provideGuzzleClientData + */ + public function testInstallMockHttpEmulatesGuzzleClient( $response, $expected ) { + $client = $this->makeFakeGuzzleClient( $response ); + $this->installMockHttp( $client ); + + $client = $this->getServiceContainer()->getHttpRequestFactory() + ->createGuzzleClient(); + + $this->assertGuzzleResponse( $expected, $client->request( 'TEST', 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->get( 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->put( 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->post( 'http://b.example.com' ) ); + } + + /** + * @dataProvider provideGuzzleClientData + */ + public function testMakeMockHttpRequestFactoryEmulatesGuzzleClient( $response, $expected ) { + $client = $this->makeFakeGuzzleClient( $response ); + $client = $this->makeMockHttpRequestFactory( $client ) + ->createGuzzleClient(); + + $this->assertGuzzleResponse( $expected, $client->request( 'TEST', 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->get( 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->put( 'http://b.example.com' ) ); + $this->assertGuzzleResponse( $expected, $client->post( 'http://b.example.com' ) ); + } + + /** + * @param ResponseInterface $expected + * @param ResponseInterface $actual + */ + private function assertGuzzleResponse( $expected, ResponseInterface $actual ) { + $this->assertSame( $expected->getStatusCode(), $actual->getStatusCode(), 'Status' ); + $this->assertSame( $expected->getHeaders(), $actual->getHeaders(), 'Headers' ); + $this->assertSame( strval( $expected->getBody() ), strval( $actual->getBody() ), 'Body' ); + } + }