Copilot commented on code in PR #3892:
URL: https://github.com/apache/thrift/pull/3892#discussion_r4054507483


##########
lib/php/test/Unit/Lib/Transport/TSSLSocketTest.php:
##########
@@ -141,50 +139,38 @@ public function testDoubleConnect(): void
         $transport->open();
     }
 
-    public function testDebugHandler()
+    public function testDebugHandlerWithLoggerInterface(): void

Review Comment:
   The test name still refers to `DebugHandler`, even though that API is 
removed by this change and the test now verifies PSR-3 logging. Rename it so 
the test accurately describes the supported behavior.



##########
lib/php/test/Unit/Lib/Transport/TSocketPoolTest.php:
##########
@@ -305,7 +305,7 @@ public function testOpen(
             $this->expectExceptionMessage($expectedExceptionMessage);
         }
 
-        $socketPool = new TSocketPool($hosts, $ports, $persist, $debugHandler 
?? $logger);
+        $socketPool = new TSocketPool($hosts, $ports, $persist, $logger ?? 
$logger);

Review Comment:
   `$logger` is overwritten with a mock above, so `$logger ?? $logger` is 
tautological and appears to be a leftover from the `$debugHandler` rename. Pass 
the mock directly, or keep the data-provider logger under a separate name if 
that parameter is intended to remain injectable.



##########
lib/php/test/Unit/Lib/Transport/TSocketTest.php:
##########
@@ -162,28 +160,19 @@ public function testDoubleConnect(): void
         $transport->open();
     }
 
-    public function testDebugHandler()
+    public function testDebugHandlerWithLoggerInterface(): void

Review Comment:
   The test name still refers to `DebugHandler`, even though that API is 
removed by this change and the test now verifies PSR-3 logging. Rename it so 
the test accurately describes the supported behavior.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to