From c626fa768ab6749da8d811616f91516cfb2cb576 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 10 Oct 2025 14:43:44 +0300 Subject: [PATCH 1/4] test(repository): remove redundant server loading test Remove 'loads existing servers from inventory' test as it duplicates functionality already covered by the comprehensive CRUD lifecycle test. The existing test already verifies server loading and findByName operations. --- .../Unit/Repositories/ServerRepositoryTest.php | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/Unit/Repositories/ServerRepositoryTest.php b/tests/Unit/Repositories/ServerRepositoryTest.php index 580bf6b8..8b357bb5 100644 --- a/tests/Unit/Repositories/ServerRepositoryTest.php +++ b/tests/Unit/Repositories/ServerRepositoryTest.php @@ -135,22 +135,4 @@ // ASSERT expect($repository->all())->toBeArray()->toBeEmpty(); }); - - it('loads existing servers from inventory', function () { - // ARRANGE - $inventory = mockInventoryService(true, ['servers' => [ - ['name' => 'web1', 'host' => '192.168.1.1', 'port' => 22, 'username' => 'root', 'privateKeyPath' => null], - ['name' => 'web2', 'host' => '192.168.1.2', 'port' => 22, 'username' => 'root', 'privateKeyPath' => null], - ]]); - $inventory->loadInventoryFile(); - $repository = new ServerRepository(); - - // ACT - $repository->loadInventory($inventory); - - // ASSERT - expect($repository->all())->toHaveCount(2) - ->and($repository->findByName('web1'))->not->toBeNull() - ->and($repository->findByName('web2'))->not->toBeNull(); - }); }); From d51f5ada6780fc084d37cd4b07c24bfc2d19e8c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 10 Oct 2025 14:43:48 +0300 Subject: [PATCH 2/4] test(server-add): consolidate redundant tests - Remove 'saves server when confirmation is given' (duplicate of SSH test) - Remove 'shows default SSH key path when not provided' (covered by minimal options test) - Consolidate 'persists server data' and 'displays complete server info' into single comprehensive test - Maintain full coverage while reducing test redundancy by 75 lines --- .../Console/Server/ServerAddCommandTest.php | 100 +++--------------- 1 file changed, 13 insertions(+), 87 deletions(-) diff --git a/tests/Integration/Console/Server/ServerAddCommandTest.php b/tests/Integration/Console/Server/ServerAddCommandTest.php index a27dbae7..1f4c44c6 100644 --- a/tests/Integration/Console/Server/ServerAddCommandTest.php +++ b/tests/Integration/Console/Server/ServerAddCommandTest.php @@ -253,72 +253,21 @@ function createServerAddCommandTester(?SSHService $sshService = null): CommandTe ->and($output)->toContain('--skip'); }); - it('saves server when confirmation is given', function () { - // ARRANGE - $sshService = mockSSHServiceWithBehavior(true); - $tester = createServerAddCommandTester($sshService); - - // ACT - Provide all required options - ob_start(); - $exitCode = $tester->execute([ - '--name' => 'confirmed-server', - '--host' => '192.168.1.1', - '--port' => '22', - '--username' => 'root', - '--private-key-path' => '', - '--skip' => true, - '--yes' => true, - ]); - ob_end_clean(); - - // ASSERT - $output = $tester->getDisplay(); - expect($exitCode)->toBe(Command::SUCCESS) - ->and($output)->toContain('✓') - ->and($output)->toContain('Server added successfully'); - }); - // - // Inventory Persistence + // Inventory Persistence & Display // ------------------------------------------------------------------------------- - it('persists server data to inventory correctly', function () { - // ARRANGE - $sshService = mockSSHServiceWithBehavior(true); - $tester = createServerAddCommandTester($sshService); - - // ACT - Provide all required options - ob_start(); - $tester->execute([ - '--name' => 'persisted-server', - '--host' => '10.20.30.40', - '--port' => '8022', - '--username' => 'admin', - '--private-key-path' => '~/.ssh/admin_key', - '--skip' => true, - '--yes' => true, - ]); - ob_end_clean(); - - // ASSERT - Verify server persisted by checking command output - $output = $tester->getDisplay(); - expect($output)->toContain('✓') - ->and($output)->toContain('Server added successfully') - ->and($output)->toContain('persisted-server') - ->and($output)->toContain('10.20.30.40'); - }); - - it('displays complete server information before saving', function () { + it('displays complete server information and persists to inventory', function () { // ARRANGE $sshService = mockSSHServiceWithBehavior(true); $tester = createServerAddCommandTester($sshService); // ACT - Provide all required options ob_start(); - $tester->execute([ - '--name' => 'display-test', + $exitCode = $tester->execute([ + '--name' => 'complete-test', '--host' => 'example.com', - '--port' => '22', + '--port' => '8022', '--username' => 'deployer', '--private-key-path' => '~/.ssh/key', '--skip' => true, @@ -326,43 +275,20 @@ function createServerAddCommandTester(?SSHService $sshService = null): CommandTe ]); ob_end_clean(); - // ASSERT + // ASSERT - Verify display AND persistence $output = $tester->getDisplay(); - expect($output)->toContain('Name:') - ->and($output)->toContain('display-test') + expect($exitCode)->toBe(Command::SUCCESS) + ->and($output)->toContain('Name:') + ->and($output)->toContain('complete-test') ->and($output)->toContain('Host:') ->and($output)->toContain('example.com') ->and($output)->toContain('Port:') - ->and($output)->toContain('22') + ->and($output)->toContain('8022') ->and($output)->toContain('User:') ->and($output)->toContain('deployer') ->and($output)->toContain('Key:') - ->and($output)->toContain('~/.ssh/key'); - }); - - it('shows default SSH key path when not provided', function () { - // ARRANGE - $sshService = mockSSHServiceWithBehavior(true); - $tester = createServerAddCommandTester($sshService); - - // ACT - Provide all required options except private-key-path to test default - ob_start(); - $tester->execute([ - '--name' => 'default-key', - '--host' => '192.168.1.1', - '--port' => '22', - '--username' => 'root', - '--private-key-path' => '', - '--skip' => true, - '--yes' => true, - ]); - ob_end_clean(); - - // ASSERT - $output = $tester->getDisplay(); - expect($output)->toContain('Key:') - ->and($output)->toContain('default') - ->and($output)->toContain('~/.ssh/id_ed25519') - ->and($output)->toContain('~/.ssh/id_rsa'); + ->and($output)->toContain('~/.ssh/key') + ->and($output)->toContain('✓') + ->and($output)->toContain('Server added successfully'); }); }); From 7c6428a3b4fb2c0e136b25616976c3b87e748635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 10 Oct 2025 14:43:51 +0300 Subject: [PATCH 3/4] test(server-delete): consolidate display tests with datasets - Remove duplicate 'deletes server when confirmation is provided' test - Consolidate two display verification tests into single dataset-driven test - Use Pest's ->with() pattern to test both custom and default SSH key scenarios - Reduce redundancy by 41 lines while maintaining comprehensive coverage --- .../Server/ServerDeleteCommandTest.php | 73 ++++--------------- 1 file changed, 16 insertions(+), 57 deletions(-) diff --git a/tests/Integration/Console/Server/ServerDeleteCommandTest.php b/tests/Integration/Console/Server/ServerDeleteCommandTest.php index c871928a..877f37c6 100644 --- a/tests/Integration/Console/Server/ServerDeleteCommandTest.php +++ b/tests/Integration/Console/Server/ServerDeleteCommandTest.php @@ -77,25 +77,6 @@ function createServerDeleteCommandTester(array $existingServers = []): CommandTe '--yes' => true, ]); - // ASSERT - $output = $tester->getDisplay(); - expect($exitCode)->toBe(Command::SUCCESS) - ->and($output)->toContain('deleted successfully'); - }); - - it('deletes server when confirmation is provided', function () { - // ARRANGE - $existingServers = [ - new ServerDTO('delete-me', '192.168.1.1', 22, 'root', null), - ]; - $tester = createServerDeleteCommandTester($existingServers); - - // ACT - $exitCode = $tester->execute([ - '--name' => 'delete-me', - '--yes' => true, - ]); - // ASSERT $output = $tester->getDisplay(); expect($exitCode)->toBe(Command::SUCCESS) @@ -146,53 +127,31 @@ function createServerDeleteCommandTester(array $existingServers = []): CommandTe // Display Verification // ------------------------------------------------------------------------------- - it('displays server info before confirmation', function () { - // ARRANGE - $existingServers = [ - new ServerDTO('display-test', 'example.com', 2222, 'deployer', '~/.ssh/key'), - ]; - $tester = createServerDeleteCommandTester($existingServers); - - // ACT - $tester->execute([ - '--name' => 'display-test', - '--yes' => true, - ]); - - // ASSERT - $output = $tester->getDisplay(); - expect($output)->toContain('Name:') - ->and($output)->toContain('display-test') - ->and($output)->toContain('Host:') - ->and($output)->toContain('example.com') - ->and($output)->toContain('Port:') - ->and($output)->toContain('2222') - ->and($output)->toContain('User:') - ->and($output)->toContain('deployer') - ->and($output)->toContain('Key:') - ->and($output)->toContain('~/.ssh/key'); - }); - - it('displays default SSH key message when path is null', function () { + it('displays server information before deletion', function (ServerDTO $server, array $expectedOutput) { // ARRANGE - $existingServers = [ - new ServerDTO('default-key-server', '192.168.1.1', 22, 'root', null), - ]; - $tester = createServerDeleteCommandTester($existingServers); + $tester = createServerDeleteCommandTester([$server]); // ACT $tester->execute([ - '--name' => 'default-key-server', + '--name' => $server->name, '--yes' => true, ]); // ASSERT $output = $tester->getDisplay(); - expect($output)->toContain('Key:') - ->and($output)->toContain('default') - ->and($output)->toContain('~/.ssh/id_ed25519') - ->and($output)->toContain('~/.ssh/id_rsa'); - }); + foreach ($expectedOutput as $expected) { + expect($output)->toContain($expected); + } + })->with([ + 'custom key path' => [ + new ServerDTO('custom-key', 'example.com', 2222, 'deployer', '~/.ssh/key'), + ['Name:', 'custom-key', 'Host:', 'example.com', 'Port:', '2222', 'User:', 'deployer', 'Key:', '~/.ssh/key'], + ], + 'default key path' => [ + new ServerDTO('default-key', '192.168.1.1', 22, 'root', null), + ['Name:', 'default-key', 'Host:', '192.168.1.1', 'Port:', '22', 'User:', 'root', 'Key:', 'default', '~/.ssh/id_ed25519', '~/.ssh/id_rsa'], + ], + ]); it('shows command hint with correct parameters', function () { // ARRANGE From 27dbafbd9bdec86f4e498154561ad570bf22a29c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 10 Oct 2025 14:43:57 +0300 Subject: [PATCH 4/4] test(server-list): remove redundant field display test Remove 'displays all server fields correctly' test as it duplicates functionality already covered by 'lists single server with complete details'. The existing test already verifies all server field display behavior. --- .../Console/Server/ServerListCommandTest.php | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/tests/Integration/Console/Server/ServerListCommandTest.php b/tests/Integration/Console/Server/ServerListCommandTest.php index 5b181f1c..0146ec7a 100644 --- a/tests/Integration/Console/Server/ServerListCommandTest.php +++ b/tests/Integration/Console/Server/ServerListCommandTest.php @@ -146,30 +146,6 @@ function createServerListCommandTester(array $existingServers = []): CommandTest ->and($output)->not->toContain('default'); }); - it('displays all server fields correctly', function () { - // ARRANGE - $existingServers = [ - new ServerDTO('full-details', 'server.example.com', 9022, 'sysadmin', '/home/user/.ssh/key'), - ]; - $tester = createServerListCommandTester($existingServers); - - // ACT - $exitCode = $tester->execute([]); - - // ASSERT - $output = $tester->getDisplay(); - expect($output)->toContain('Name:') - ->and($output)->toContain('full-details') - ->and($output)->toContain('Host:') - ->and($output)->toContain('server.example.com') - ->and($output)->toContain('Port:') - ->and($output)->toContain('9022') - ->and($output)->toContain('User:') - ->and($output)->toContain('sysadmin') - ->and($output)->toContain('Key:') - ->and($output)->toContain('/home/user/.ssh/key'); - }); - it('lists servers in order they appear in inventory', function () { // ARRANGE $existingServers = [