Copilot commented on code in PR #3796:
URL: https://github.com/apache/thrift/pull/3796#discussion_r3987947715
##########
lib/php/test/Unit/Lib/Transport/TSocketTest.php:
##########
@@ -469,6 +469,20 @@ public function testClose()
$this->assertNull($this->getPropertyValue($transport, 'handle'));
}
+ public function testClosePersistentSocket(): void
+ {
+ $transport = new TSocket('localhost', 9090, true, null);
+ $transport->setHandle(fopen('php://memory', 'r+'));
+
+ $this->assertNotNull($this->getPropertyValue($transport, 'handle'));
+
+ // Guard THRIFT-2151: close() must remain effective even when the
+ // socket was opened in persistent mode.
+ $transport->close();
+
+ $this->assertNull($this->getPropertyValue($transport, 'handle'));
Review Comment:
This test sets `persist=true` but injects a `php://memory` stream with
`setHandle()`, so it never exercises the persistent `pfsockopen()` path or
verifies that the underlying stream is actually closed. A regression that
clears `$this->handle` while skipping `fclose()` for persistent sockets would
still pass because the only postcondition is the property becoming `null`;
retain the local handle and assert it is no longer a resource, or open it
through the existing `pfsockopen` mock.
--
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]
