feat(client): add isConnected method to check server connection status#41
feat(client): add isConnected method to check server connection status#41tohenk merged 2 commits intoElephantIO:masterfrom
Conversation
- Introduced a new method `isConnected` in the Client class to determine if the client is currently connected to the server. - This method delegates the connection check to the engine's `connected` method, enhancing the client's ability to manage connection states. - Added unit tests to verify the behavior of the `isConnected` method, ensuring it returns true when connected and false otherwise.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new isConnected() method to the Client class to check the server connection status. The method provides a simple wrapper around the engine's connected() method, making it easier for consumers to check connection state directly from the Client.
Key changes:
- Added
isConnected()method to Client class that delegates to the engine'sconnected()method - Added comprehensive unit tests for the new method
- Enhanced SocketStream's
readable()method with EOF detection for better disconnect handling
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/Client.php | Added isConnected() method with documentation |
| test/ClientTest.php | Added unit tests for the new isConnected() method |
| src/Stream/SocketStream.php | Added EOF check for improved disconnect detection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,32 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Missing the standard file header docblock that exists in other test files. Add the copyright and license information as seen in PacketTest.php and StoreTest.php.
| <?php | |
| <?php | |
| /** | |
| * Copyright (c) 2012-2024, ElephantIO contributors | |
| * | |
| * This file is part of ElephantIO. | |
| * | |
| * Licensed under the MIT License. See LICENSE file in the project root for full license information. | |
| */ |
|
|
||
| class ClientTest extends TestCase | ||
| { | ||
| public function testIsConnectedDelegatesToEngine() |
There was a problem hiding this comment.
Missing return type declaration. Other test methods in the codebase (e.g., PacketTest.php, StoreTest.php) use : void return type hints for consistency.
| public function testIsConnectedDelegatesToEngine() | |
| public function testIsConnectedDelegatesToEngine(): void |
| $this->assertTrue($client->isConnected()); | ||
| } | ||
|
|
||
| public function testIsConnectedReturnsFalseWhenEngineIsNotConnected() |
There was a problem hiding this comment.
Missing return type declaration. Other test methods in the codebase (e.g., PacketTest.php, StoreTest.php) use : void return type hints for consistency.
| public function testIsConnectedReturnsFalseWhenEngineIsNotConnected() | |
| public function testIsConnectedReturnsFalseWhenEngineIsNotConnected(): void |
| $engine->expects($this->any()) | ||
| ->method('connected') | ||
| ->willReturn(true); |
There was a problem hiding this comment.
Using expects($this->any()) is imprecise. Since the method is called exactly once in each test, use expects($this->once()) to verify the delegation happens as expected.
| $engine->expects($this->any()) | ||
| ->method('connected') | ||
| ->willReturn(false); |
There was a problem hiding this comment.
Using expects($this->any()) is imprecise. Since the method is called exactly once in each test, use expects($this->once()) to verify the delegation happens as expected.
Signed-off-by: Toha <tohenk@yahoo.com>
isConnectedin the Client class to determineif the client is currently connected to the server.
connectedmethod, enhancing the client's ability to manage connection states.
isConnectedmethod,ensuring it returns true when connected and false otherwise.
Fixes: #40