only assert sentry-tracing header, no need to fully test guzzle client

getsentry · Jun 14, 2022 · 72094da · 72094da
1 parent fdea652
commit 72094da
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 110 deletions.
diff --git a/composer.json b/composer.json
@@ -40,7 +40,6 @@
     },
     "require-dev": {
         "friendsofphp/php-cs-fixer": "^2.19|3.4.*",
-        "guzzlehttp/guzzle": "^7.4",
         "http-interop/http-factory-guzzle": "^1.0",
         "monolog/monolog": "^1.6|^2.0|^3.0",
         "nikic/php-parser": "^4.10.3",

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -45,6 +45,21 @@ parameters:
 			count: 2
 			path: src/EventHint.php
 
+		-
+			message: "#^Access to constant CONNECT_TIMEOUT on an unknown class GuzzleHttp\\\\RequestOptions\\.$#"
+			count: 1
+			path: src/HttpClient/HttpClientFactory.php
+
+		-
+			message: "#^Access to constant PROXY on an unknown class GuzzleHttp\\\\RequestOptions\\.$#"
+			count: 1
+			path: src/HttpClient/HttpClientFactory.php
+
+		-
+			message: "#^Access to constant TIMEOUT on an unknown class GuzzleHttp\\\\RequestOptions\\.$#"
+			count: 1
+			path: src/HttpClient/HttpClientFactory.php
+
 		-
 			message: "#^Call to static method create\\(\\) on an unknown class Symfony\\\\Component\\\\HttpClient\\\\HttpClient\\.$#"
 			count: 1
@@ -285,6 +300,16 @@ parameters:
 			count: 3
 			path: src/State/HubInterface.php
 
+		-
+			message: "#^Call to method getResponse\\(\\) on an unknown class GuzzleHttp\\\\Exception\\\\RequestException\\.$#"
+			count: 1
+			path: src/Tracing/GuzzleTracingMiddleware.php
+
+		-
+			message: "#^Class GuzzleHttp\\\\Exception\\\\RequestException not found\\.$#"
+			count: 1
+			path: src/Tracing/GuzzleTracingMiddleware.php
+
 		-
 			message: "#^Unsafe usage of new static\\(\\)\\.$#"
 			count: 1

diff --git a/tests/Tracing/GuzzleTracingMiddlewareTest.php b/tests/Tracing/GuzzleTracingMiddlewareTest.php
@@ -4,10 +4,9 @@
 
 namespace Sentry\Tests\Tracing;
 
-use GuzzleHttp\Client;
-use GuzzleHttp\Handler\MockHandler;
-use GuzzleHttp\HandlerStack;
-use GuzzleHttp\Middleware;
+use GuzzleHttp\Promise\FulfilledPromise;
+use GuzzleHttp\Promise\PromiseInterface;
+use GuzzleHttp\Promise\RejectedPromise;
 use GuzzleHttp\Psr7\Request;
 use GuzzleHttp\Psr7\Response;
 use PHPUnit\Framework\TestCase;
@@ -23,54 +22,31 @@
 
 final class GuzzleTracingMiddlewareTest extends TestCase
 {
-    /**
-     * @var ClientInterface
-     */
-    private $sentryClient;
-
-    /**
-     * @var Hub
-     */
-    private $hub;
+    public function testTraceDoesNothingIfSpanIsNotSet(): void
+    {
+        $client = $this->createMock(ClientInterface::class);
 
-    /**
-     * @var Client
-     */
-    private $guzzleClient;
+        $hub = new Hub($client);
 
-    /**
-     * @var MockHandler
-     */
-    private $mockGuzzleHandler;
+        $expectedPromiseResult = new Response();
 
-    /**
-     * @var array<
-     *     array{
-     *         request: RequestInterface,
-     *         response: ResponseInterface|null,
-     *         error: mixed,
-     *         options: mixed
-     *     }
-     * >
-     */
-    private $requestResponseHistory;
+        $middleware = GuzzleTracingMiddleware::trace($hub);
+        $function = $middleware(static function () use ($expectedPromiseResult): PromiseInterface {
+            return new FulfilledPromise($expectedPromiseResult);
+        });
 
-    public function testTraceDoesNothingIfSpanIsNotSet(): void
-    {
-        $expectedResponse = new Response();
-        $this->addResponseToClientMock($expectedResponse);
+        /** @var PromiseInterface $promise */
+        $promise = $function(new Request('GET', 'https://www.example.com'), []);
 
-        $method = 'GET';
-        $uri = 'https://www.example.com';
-        $actualResponse = $this->guzzleClient->request($method, $uri);
-        $actualRequest = $this->popRequestFromClientMock();
+        try {
+            $promiseResult = $promise->wait();
+        } catch (\Throwable $exception) {
+            $promiseResult = $exception;
+        }
 
-        $this->assertNotContains('sentry-trace', $actualRequest->getHeaders());
-        $this->assertEquals($method, $actualRequest->getMethod());
-        $this->assertEquals($uri, $actualRequest->getUri());
-        $this->assertSame($expectedResponse, $actualResponse);
+        $this->assertSame($expectedPromiseResult, $promiseResult);
 
-        $this->hub->configureScope(function (Scope $scope): void {
+        $hub->configureScope(function (Scope $scope): void {
             $event = Event::createEvent();
 
             $scope->applyToEvent($event);
@@ -84,15 +60,19 @@ public function testTraceDoesNothingIfSpanIsNotSet(): void
      */
     public function testTrace(Request $request, $expectedPromiseResult, array $expectedBreadcrumbData): void
     {
-        $this->sentryClient->expects($this->exactly(2))
+        $client = $this->createMock(ClientInterface::class);
+        $client->expects($this->exactly(2))
             ->method('getOptions')
             ->willReturn(new Options(['traces_sample_rate' => 1]));
-        $this->sentryClient->expects($this->once())
+
+        $hub = new Hub($client);
+
+        $client->expects($this->once())
             ->method('captureEvent')
-            ->with($this->callback(function (Event $eventArg) use ($request, $expectedPromiseResult, $expectedBreadcrumbData): bool {
+            ->with($this->callback(function (Event $eventArg) use ($hub, $request, $expectedPromiseResult, $expectedBreadcrumbData): bool {
                 $this->assertSame(EventType::transaction(), $eventArg->getType());
 
-                $this->hub->configureScope(static function (Scope $scope) use ($eventArg): void {
+                $hub->configureScope(static function (Scope $scope) use ($eventArg): void {
                     $scope->applyToEvent($eventArg);
                 });
 
@@ -119,26 +99,30 @@ public function testTrace(Request $request, $expectedPromiseResult, array $expec
                 return true;
             }));
 
-        $transaction = $this->hub->startTransaction(new TransactionContext());
-
-        $this->hub->setSpan($transaction);
+        $transaction = $hub->startTransaction(new TransactionContext());
 
-        $this->addResponseToClientMock($expectedPromiseResult);
+        $hub->setSpan($transaction);
 
-        try {
-            $actualResponse = $this->guzzleClient->send($request);
-        } catch (\Throwable $throwable) {
+        $middleware = GuzzleTracingMiddleware::trace($hub);
+        $function = $middleware(static function (Request $request) use ($expectedPromiseResult): PromiseInterface {
+            self::assertNotEmpty($request->getHeader('sentry-trace'));
             if ($expectedPromiseResult instanceof \Throwable) {
-                $actualResponse = $throwable;
-            } else {
-                throw $throwable;
+                return new RejectedPromise($expectedPromiseResult);
             }
-        }
 
-        $actualRequest = $this->popRequestFromClientMock();
+            return new FulfilledPromise($expectedPromiseResult);
+        });
+
+        /** @var PromiseInterface $promise */
+        $promise = $function($request, []);
+
+        try {
+            $promiseResult = $promise->wait();
+        } catch (\Throwable $exception) {
+            $promiseResult = $exception;
+        }
 
-        $this->assertSame($expectedPromiseResult, $actualResponse);
-        $this->assertNotEmpty($actualRequest->getHeader('sentry-trace'));
+        $this->assertSame($expectedPromiseResult, $promiseResult);
 
         $transaction->finish();
     }
@@ -179,50 +163,4 @@ public function traceDataProvider(): iterable
             ],
         ];
     }
-
-    protected function setUp(): void
-    {
-        $this->sentryClient = $this->createMock(ClientInterface::class);
-        $this->hub = new Hub($this->sentryClient);
-
-        $this->initMockGuzzleClient();
-    }
-
-    /**
-     * Set up the Guzzle HTTP client for mocking.
-     *
-     * @see http://docs.guzzlephp.org/en/stable/testing.html
-     */
-    private function initMockGuzzleClient()
-    {
-        // The MockHandler is where we manually queue up responses for our tests.
-        // EX: $this->addResponseToClientMock(new Response(200, [], '[]'));
-        $this->mockGuzzleHandler = new MockHandler();
-
-        // The middleware stack.
-        $handlerStack = HandlerStack::create($this->mockGuzzleHandler);
-
-        // Set up the History middleware to track requests and responses.
-        $this->requestResponseHistory = [];
-        $history = Middleware::history($this->requestResponseHistory);
-        $handlerStack->push($history);
-        $tracingMiddleware = GuzzleTracingMiddleware::trace($this->hub);
-        $handlerStack->unshift($tracingMiddleware);
-
-        // Create the Guzzle HTTP client not pointed at anything
-        $this->guzzleClient = new Client(['handler' => $handlerStack, 'http_errors' => false]);
-    }
-
-    /**
-     * @param Response|\Throwable $response
-     */
-    private function addResponseToClientMock($response): void
-    {
-        $this->mockGuzzleHandler->append($response);
-    }
-
-    private function popRequestFromClientMock(): ?Request
-    {
-        return array_shift($this->requestResponseHistory)['request'];
-    }
 }