From c96df58054deef3770c41ad2e30545dbb2491d06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 3 Oct 2025 13:05:06 +0300 Subject: [PATCH 1/3] feat(data): add ServerDTO for server configuration Add readonly data transfer object to represent server configuration with properties for name, host, port, username, and SSH private key path. Includes sensible defaults for optional properties (port: 22, username: root). --- app/Contracts/BaseCommand.php | 7 + app/DTOs/ServerDTO.php | 17 ++ app/Repositories/ServerRepository.php | 168 ++++++++++++++++++ tests/Fixtures/TestConsoleCommand.php | 4 +- tests/TestHelpers.php | 21 +++ tests/Unit/Contracts/BaseCommandTest.php | 10 +- tests/Unit/DTOs/ServerDTOTest.php | 35 ++++ .../Repositories/ServerRepositoryTest.php | 156 ++++++++++++++++ tests/Unit/Traits/ConsoleInputTraitTest.php | 2 +- tests/Unit/Traits/ConsoleOutputTraitTest.php | 2 +- 10 files changed, 416 insertions(+), 6 deletions(-) create mode 100644 app/DTOs/ServerDTO.php create mode 100644 app/Repositories/ServerRepository.php create mode 100644 tests/Unit/DTOs/ServerDTOTest.php create mode 100644 tests/Unit/Repositories/ServerRepositoryTest.php diff --git a/app/Contracts/BaseCommand.php b/app/Contracts/BaseCommand.php index 06a08477..2bc705cf 100644 --- a/app/Contracts/BaseCommand.php +++ b/app/Contracts/BaseCommand.php @@ -5,6 +5,7 @@ namespace Bigpixelrocket\DeployerPHP\Contracts; use Bigpixelrocket\DeployerPHP\Container; +use Bigpixelrocket\DeployerPHP\Repositories\ServerRepository; use Bigpixelrocket\DeployerPHP\Services\EnvService; use Bigpixelrocket\DeployerPHP\Services\InventoryService; use Bigpixelrocket\DeployerPHP\Traits\ConsoleInputTrait; @@ -34,6 +35,7 @@ public function __construct( protected readonly Container $container, protected readonly EnvService $env, protected readonly InventoryService $inventory, + protected readonly ServerRepository $servers, ) { parent::__construct(); } @@ -90,6 +92,11 @@ protected function initialize(InputInterface $input, OutputInterface $output): v $customInventoryPath = $input->getOption('inventory'); $this->inventory->setCustomPath($customInventoryPath); $this->inventory->loadInventoryFile(); + + // + // Initialize repositories + + $this->servers->loadInventory($this->inventory); } // diff --git a/app/DTOs/ServerDTO.php b/app/DTOs/ServerDTO.php new file mode 100644 index 00000000..10c75a26 --- /dev/null +++ b/app/DTOs/ServerDTO.php @@ -0,0 +1,17 @@ +> */ + private array $servers = []; + + // + // Public + // ------------------------------------------------------------------------------- + + /** + * Set the inventory service instance to use for storage operations. + */ + public function loadInventory(InventoryService $inventory): void + { + $this->inventory = $inventory; + + $servers = $inventory->get(self::PREFIX); + if (!is_array($servers)) { + $servers = []; + $inventory->set(self::PREFIX, $servers); + } + + /** @var array> $servers */ + $this->servers = $servers; + } + + /** + * Create a new server in the inventory. + */ + public function create(ServerDTO $server): void + { + $this->assertInventoryLoaded(); + + $existing = $this->findByName($server->name); + if (null !== $existing) { + throw new \RuntimeException("Server '{$server->name}' already exists"); + } + + $this->servers[] = $this->dehydrateServerDTO($server); + + $this->inventory->set(self::PREFIX, $this->servers); + } + + /** + * Find a server by name. + */ + public function findByName(string $name): ?ServerDTO + { + $this->assertInventoryLoaded(); + + foreach ($this->servers as $server) { + if (isset($server['name']) && $server['name'] === $name) { + return $this->hydrateServerDTO($server); + } + } + + return null; + } + + /** + * Get all servers from the inventory. + * + * @return array + */ + public function all(): array + { + $this->assertInventoryLoaded(); + + $result = []; + foreach ($this->servers as $server) { + $result[] = $this->hydrateServerDTO($server); + } + + return $result; + } + + /** + * Delete a server from the inventory. + */ + public function delete(string $name): void + { + $this->assertInventoryLoaded(); + + $filtered = []; + foreach ($this->servers as $server) { + if (isset($server['name']) && $server['name'] !== $name) { + $filtered[] = $server; + } + } + + $this->servers = $filtered; + + $this->inventory->set(self::PREFIX, $this->servers); + } + + // + // Private + // ------------------------------------------------------------------------------- + + /** + * Ensure inventory service is loaded before operations. + * + * @throws \RuntimeException If inventory is not set + * @phpstan-assert !null $this->inventory + */ + private function assertInventoryLoaded(): void + { + if ($this->inventory === null) { + throw new \RuntimeException('Inventory not set. Call loadInventory() first.'); + } + } + + /** + * Convert ServerDTO to array for storage. + * + * @return array + */ + private function dehydrateServerDTO(ServerDTO $server): array + { + return [ + 'name' => $server->name, + 'host' => $server->host, + 'port' => $server->port, + 'username' => $server->username, + 'privateKeyPath' => $server->privateKeyPath, + ]; + } + + /** + * Hydrate a ServerDTO from inventory data. + * + * @param array $data + */ + private function hydrateServerDTO(array $data): ServerDTO + { + $name = $data['name'] ?? ''; + $host = $data['host'] ?? ''; + $port = $data['port'] ?? 22; + $username = $data['username'] ?? 'root'; + $privateKeyPath = $data['privateKeyPath'] ?? null; + + return new ServerDTO( + name: is_string($name) ? $name : '', + host: is_string($host) ? $host : '', + port: is_int($port) ? $port : 22, + username: is_string($username) ? $username : 'root', + privateKeyPath: is_string($privateKeyPath) ? $privateKeyPath : null, + ); + } +} diff --git a/tests/Fixtures/TestConsoleCommand.php b/tests/Fixtures/TestConsoleCommand.php index 65c3d099..1f4614ed 100644 --- a/tests/Fixtures/TestConsoleCommand.php +++ b/tests/Fixtures/TestConsoleCommand.php @@ -6,6 +6,7 @@ use Bigpixelrocket\DeployerPHP\Container; use Bigpixelrocket\DeployerPHP\Contracts\BaseCommand; +use Bigpixelrocket\DeployerPHP\Repositories\ServerRepository; use Bigpixelrocket\DeployerPHP\Services\EnvService; use Bigpixelrocket\DeployerPHP\Services\InventoryService; use Symfony\Component\Console\Command\Command; @@ -28,8 +29,9 @@ public function __construct( Container $container, EnvService $env, InventoryService $inventory, + ServerRepository $servers, ) { - parent::__construct($container, $env, $inventory); + parent::__construct($container, $env, $inventory, $servers); } /** diff --git a/tests/TestHelpers.php b/tests/TestHelpers.php index 6805bd06..c6e938d6 100644 --- a/tests/TestHelpers.php +++ b/tests/TestHelpers.php @@ -2,6 +2,7 @@ declare(strict_types=1); +use Bigpixelrocket\DeployerPHP\Repositories\ServerRepository; use Bigpixelrocket\DeployerPHP\Services\EnvService; use Bigpixelrocket\DeployerPHP\Services\FilesystemService; use Bigpixelrocket\DeployerPHP\Services\InventoryService; @@ -229,3 +230,23 @@ function mockVersionService( return new VersionService($processFactory, $filesystemService); } } + +if (!function_exists('mockServerRepository')) { + /** + * Create a ServerRepository for testing with a loaded inventory service. + */ + function mockServerRepository( + bool $fileExists = true, + array|string $data = '', + bool $throwOnRead = false, + bool $throwOnWrite = false + ): ServerRepository { + $inventory = mockInventoryService($fileExists, $data, $throwOnRead, $throwOnWrite); + $inventory->loadInventoryFile(); + + $repository = new ServerRepository(); + $repository->loadInventory($inventory); + + return $repository; + } +} diff --git a/tests/Unit/Contracts/BaseCommandTest.php b/tests/Unit/Contracts/BaseCommandTest.php index 2d4abdfe..af76f903 100644 --- a/tests/Unit/Contracts/BaseCommandTest.php +++ b/tests/Unit/Contracts/BaseCommandTest.php @@ -6,6 +6,7 @@ use Bigpixelrocket\DeployerPHP\Container; use Bigpixelrocket\DeployerPHP\Contracts\BaseCommand; +use Bigpixelrocket\DeployerPHP\Repositories\ServerRepository; use Bigpixelrocket\DeployerPHP\Services\EnvService; use Bigpixelrocket\DeployerPHP\Services\InventoryService; use Symfony\Component\Console\Command\Command; @@ -25,9 +26,10 @@ public function __construct( Container $container, EnvService $env, InventoryService $inventory, + ServerRepository $servers, private readonly string $testName = 'test-command', ) { - parent::__construct($container, $env, $inventory); + parent::__construct($container, $env, $inventory, $servers); } protected function configure(): void @@ -54,9 +56,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $container = new Container(); $env = mockEnvService(true); $inventory = mockInventoryService(true); + $servers = mockServerRepository(); // ACT - $command = new TestableBaseCommand($container, $env, $inventory, 'test'); + $command = new TestableBaseCommand($container, $env, $inventory, $servers, 'test'); // ASSERT expect($command->getName())->toBe('test') @@ -73,7 +76,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $container = new Container(); $env = mockEnvService($hasEnvFile); $inventory = mockInventoryService(true); - $command = new TestableBaseCommand($container, $env, $inventory); + $servers = mockServerRepository(); + $command = new TestableBaseCommand($container, $env, $inventory, $servers); $tester = new CommandTester($command); // ACT diff --git a/tests/Unit/DTOs/ServerDTOTest.php b/tests/Unit/DTOs/ServerDTOTest.php new file mode 100644 index 00000000..249ee863 --- /dev/null +++ b/tests/Unit/DTOs/ServerDTOTest.php @@ -0,0 +1,35 @@ +name)->toBe('production-web') + ->and($server->host)->toBe('192.168.1.100') + ->and($server->port)->toBe(2222) + ->and($server->username)->toBe('deployer') + ->and($server->privateKeyPath)->toBe('~/.ssh/custom_key'); + }); + + it('uses default values for optional properties', function () { + // ARRANGE & ACT + $server = new ServerDTO(name: 'test-server', host: '127.0.0.1'); + + // ASSERT + expect($server->port)->toBe(22) + ->and($server->username)->toBe('root') + ->and($server->privateKeyPath)->toBeNull(); + }); +}); diff --git a/tests/Unit/Repositories/ServerRepositoryTest.php b/tests/Unit/Repositories/ServerRepositoryTest.php new file mode 100644 index 00000000..5eb5098a --- /dev/null +++ b/tests/Unit/Repositories/ServerRepositoryTest.php @@ -0,0 +1,156 @@ + $repository->all()) + ->toThrow(\RuntimeException::class, 'Inventory not set'); + }); + + // + // CRUD Operations + // ------------------------------------------------------------------------------- + + it('handles complete CRUD lifecycle', function () { + // ARRANGE + $inventory = mockInventoryService(true, ['servers' => []]); + $inventory->loadInventoryFile(); + $repository = new ServerRepository(); + $repository->loadInventory($inventory); + + // ACT & ASSERT - Create + $server1 = new ServerDTO('web1', '192.168.1.1', 2222, 'deployer', '~/.ssh/key'); + $server2 = new ServerDTO('web2', '192.168.1.2'); + + $repository->create($server1); + $repository->create($server2); + + // ASSERT - Find by name + $found = $repository->findByName('web1'); + expect($found)->not->toBeNull() + ->and($found->name)->toBe('web1') + ->and($found->host)->toBe('192.168.1.1') + ->and($found->port)->toBe(2222) + ->and($found->username)->toBe('deployer') + ->and($found->privateKeyPath)->toBe('~/.ssh/key'); + + // ASSERT - Find returns null for missing + expect($repository->findByName('nonexistent'))->toBeNull(); + + // ASSERT - All returns both servers + $all = $repository->all(); + expect($all)->toHaveCount(2) + ->and($all[0])->toBeInstanceOf(ServerDTO::class) + ->and($all[1]->name)->toBe('web2'); + + // ACT & ASSERT - Delete + $repository->delete('web1'); + expect($repository->findByName('web1'))->toBeNull() + ->and($repository->all())->toHaveCount(1); + + // ASSERT - Delete nonexistent doesn't error + $repository->delete('never-existed'); + expect($repository->all())->toHaveCount(1); + }); + + it('prevents duplicate server creation', function () { + // ARRANGE + $inventory = mockInventoryService(true, ['servers' => [ + ['name' => 'existing', 'host' => '192.168.1.1', 'port' => 22, 'username' => 'root', 'privateKeyPath' => null], + ]]); + $inventory->loadInventoryFile(); + $repository = new ServerRepository(); + $repository->loadInventory($inventory); + + // ACT & ASSERT + expect(fn () => $repository->create(new ServerDTO('existing', '192.168.1.2'))) + ->toThrow(\RuntimeException::class, "Server 'existing' already exists"); + }); + + // + // Data Hydration Robustness + // ------------------------------------------------------------------------------- + + it('handles malformed inventory data gracefully', function (array $rawData, string $expectedName, string $expectedHost, int $expectedPort) { + // ARRANGE + $inventory = mockInventoryService(true, ['servers' => [$rawData]]); + $inventory->loadInventoryFile(); + $repository = new ServerRepository(); + $repository->loadInventory($inventory); + + // ACT + $servers = $repository->all(); + + // ASSERT + expect($servers)->toHaveCount(1) + ->and($servers[0]->name)->toBe($expectedName) + ->and($servers[0]->host)->toBe($expectedHost) + ->and($servers[0]->port)->toBe($expectedPort); + })->with([ + 'missing name' => [ + ['host' => '192.168.1.1', 'port' => 22, 'username' => 'root'], + '', '192.168.1.1', 22, + ], + 'missing host' => [ + ['name' => 'web1', 'port' => 22, 'username' => 'root'], + 'web1', '', 22, + ], + 'invalid port type' => [ + ['name' => 'web1', 'host' => '192.168.1.1', 'port' => 'not-a-number'], + 'web1', '192.168.1.1', 22, + ], + 'wrong name type' => [ + ['name' => 12345, 'host' => '192.168.1.1'], + '', '192.168.1.1', 22, + ], + ]); + + // + // Initialization Edge Cases + // ------------------------------------------------------------------------------- + + it('initializes empty array when servers key missing', function () { + // ARRANGE + $inventory = mockInventoryService(true, []); + $inventory->loadInventoryFile(); + $repository = new ServerRepository(); + + // ACT + $repository->loadInventory($inventory); + + // 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(); + }); +}); diff --git a/tests/Unit/Traits/ConsoleInputTraitTest.php b/tests/Unit/Traits/ConsoleInputTraitTest.php index 41d2311d..ccc5fbf1 100644 --- a/tests/Unit/Traits/ConsoleInputTraitTest.php +++ b/tests/Unit/Traits/ConsoleInputTraitTest.php @@ -13,7 +13,7 @@ describe('ConsoleInputTrait', function () { beforeEach(function () { $container = new Container(); - $this->command = new TestConsoleCommand($container, mockEnvService(true), mockInventoryService(true)); + $this->command = new TestConsoleCommand($container, mockEnvService(true), mockInventoryService(true), mockServerRepository()); $this->tester = new CommandTester($this->command); }); diff --git a/tests/Unit/Traits/ConsoleOutputTraitTest.php b/tests/Unit/Traits/ConsoleOutputTraitTest.php index e6f09a0c..87de3910 100644 --- a/tests/Unit/Traits/ConsoleOutputTraitTest.php +++ b/tests/Unit/Traits/ConsoleOutputTraitTest.php @@ -13,7 +13,7 @@ describe('ConsoleOutputTrait', function () { beforeEach(function () { $container = new Container(); - $this->command = new TestConsoleCommand($container, mockEnvService(true), mockInventoryService(true)); + $this->command = new TestConsoleCommand($container, mockEnvService(true), mockInventoryService(true), mockServerRepository()); $this->tester = new CommandTester($this->command); }); From 7e9cf80616f1d7218b5918d8bd86a6f1607bde82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 3 Oct 2025 13:05:17 +0300 Subject: [PATCH 2/3] refactor(tests): use domain-agnostic terminology in inventory tests Replace server-specific terminology (servers, production, web1) with generic widget terminology (widgets, alpha, beta) in InventoryService documentation and tests to prevent confusion with the new ServerRepository implementation. --- app/Services/InventoryService.php | 20 ++-- tests/TestHelpers.php | 2 +- tests/Unit/Services/InventoryServiceTest.php | 100 +++++++++---------- tests/Unit/TestHelpersTest.php | 8 +- 4 files changed, 65 insertions(+), 65 deletions(-) diff --git a/app/Services/InventoryService.php b/app/Services/InventoryService.php index a5541c0e..218486c0 100644 --- a/app/Services/InventoryService.php +++ b/app/Services/InventoryService.php @@ -11,24 +11,24 @@ * * @example * // Store values using dot notation - * $inventory->set('servers.production.host', 'example.com'); - * $inventory->set('servers.production.user', 'deployer'); + * $inventory->set('widgets.alpha.color', 'blue'); + * $inventory->set('widgets.alpha.size', 'large'); * * // Or set entire object at once - * $inventory->set('servers.production', ['host' => 'example.com', 'user' => 'deployer']); + * $inventory->set('widgets.alpha', ['color' => 'blue', 'size' => 'large']); * * // Retrieve values at any depth - * $inventory->get('servers.production.host'); // 'example.com' - * $inventory->get('servers.production'); // ['host' => 'example.com', 'user' => 'deployer'] - * $inventory->get('servers'); // ['production' => ['host' => 'example.com', 'user' => 'deployer']] + * $inventory->get('widgets.alpha.color'); // 'blue' + * $inventory->get('widgets.alpha'); // ['color' => 'blue', 'size' => 'large'] + * $inventory->get('widgets'); // ['alpha' => ['color' => 'blue', 'size' => 'large']] * * // Default values when path doesn't exist - * $inventory->get('servers.staging'); // null - * $inventory->get('servers.staging', []); // [] - * $inventory->get('servers.staging.host', 'localhost'); // 'localhost' + * $inventory->get('widgets.beta'); // null + * $inventory->get('widgets.beta', []); // [] + * $inventory->get('widgets.beta.color', 'red'); // 'red' * * // Delete path - * $inventory->delete('servers.production'); + * $inventory->delete('widgets.alpha'); */ class InventoryService { diff --git a/tests/TestHelpers.php b/tests/TestHelpers.php index c6e938d6..d84e2531 100644 --- a/tests/TestHelpers.php +++ b/tests/TestHelpers.php @@ -164,7 +164,7 @@ function mockInventoryService( if (is_array($data)) { $fileContent = empty($data) ? '' : Yaml::dump($data, 2, 4, Yaml::DUMP_EMPTY_ARRAY_AS_SEQUENCE); } else { - $defaultContent = 'servers:' . PHP_EOL . ' web1:' . PHP_EOL . ' host: example.com'; + $defaultContent = 'widgets:' . PHP_EOL . ' alpha:' . PHP_EOL . ' color: red'; $fileContent = $data ?: ($fileExists ? $defaultContent : ''); } diff --git a/tests/Unit/Services/InventoryServiceTest.php b/tests/Unit/Services/InventoryServiceTest.php index 6f4cdca4..6f69b484 100644 --- a/tests/Unit/Services/InventoryServiceTest.php +++ b/tests/Unit/Services/InventoryServiceTest.php @@ -30,16 +30,16 @@ expect($result)->toBe($value); })->with([ // New file scenarios - 'simple nested path' => ['servers.web1', 'value', null, false], - 'deep nested path' => ['app.db.host', 'localhost', null, false], - 'array value' => ['servers.web1', ['host' => 'example.com', 'port' => 22], null, false], - 'complex nested structure' => ['deployments.prod.servers.web.config', ['cpu' => '2'], null, false], - 'single segment path' => ['servers', ['web1' => ['host' => 'example.com']], null, false], + 'simple nested path' => ['widgets.alpha', 'value', null, false], + 'deep nested path' => ['toys.robot.color', 'blue', null, false], + 'array value' => ['widgets.alpha', ['color' => 'red', 'size' => 10], null, false], + 'complex nested structure' => ['categories.shapes.widgets.circle.radius', ['value' => '5'], null, false], + 'single segment path' => ['widgets', ['alpha' => ['color' => 'red']], null, false], // Existing file scenarios - 'overwrite existing value' => ['servers.web1.host', 'new.com', ['servers' => ['web1' => ['host' => 'old.com']]], true], - 'create intermediate paths' => ['servers.web2.database.host', 'db.example.com', ['servers' => ['web1' => ['host' => 'example.com']]], true], - 'type conflict resolution' => ['servers.web1', 'new-string-value', ['servers' => ['web1' => ['host' => 'example.com']]], true], + 'overwrite existing value' => ['widgets.alpha.color', 'blue', ['widgets' => ['alpha' => ['color' => 'red']]], true], + 'create intermediate paths' => ['widgets.beta.nested.color', 'green', ['widgets' => ['alpha' => ['color' => 'red']]], true], + 'type conflict resolution' => ['widgets.alpha', 'new-string-value', ['widgets' => ['alpha' => ['color' => 'red']]], true], ]); // @@ -59,44 +59,44 @@ })->with([ // File exists - positive cases 'deep nested value' => [ - 'servers.production.host', - 'prod.example.com', - ['servers' => ['production' => ['host' => 'prod.example.com', 'user' => 'deploy'], 'staging' => ['host' => 'staging.example.com']], 'databases' => ['primary' => ['host' => 'db1.example.com', 'port' => 5432]]], + 'widgets.alpha.color', + 'red', + ['widgets' => ['alpha' => ['color' => 'red', 'size' => 'large'], 'beta' => ['color' => 'blue', 'size' => 'small']], 'animals' => ['cat' => ['sound' => 'meow', 'legs' => 4]]], true ], 'nested object' => [ - 'servers.production', - ['host' => 'prod.example.com', 'user' => 'deploy'], - ['servers' => ['production' => ['host' => 'prod.example.com', 'user' => 'deploy'], 'staging' => ['host' => 'staging.example.com']], 'databases' => ['primary' => ['host' => 'db1.example.com', 'port' => 5432]]], + 'widgets.alpha', + ['color' => 'red', 'size' => 'large'], + ['widgets' => ['alpha' => ['color' => 'red', 'size' => 'large'], 'beta' => ['color' => 'blue', 'size' => 'small']], 'animals' => ['cat' => ['sound' => 'meow', 'legs' => 4]]], true ], 'top level collection' => [ - 'servers', - ['production' => ['host' => 'prod.example.com', 'user' => 'deploy'], 'staging' => ['host' => 'staging.example.com']], - ['servers' => ['production' => ['host' => 'prod.example.com', 'user' => 'deploy'], 'staging' => ['host' => 'staging.example.com']], 'databases' => ['primary' => ['host' => 'db1.example.com', 'port' => 5432]]], + 'widgets', + ['alpha' => ['color' => 'red', 'size' => 'large'], 'beta' => ['color' => 'blue', 'size' => 'small']], + ['widgets' => ['alpha' => ['color' => 'red', 'size' => 'large'], 'beta' => ['color' => 'blue', 'size' => 'small']], 'animals' => ['cat' => ['sound' => 'meow', 'legs' => 4]]], true ], - 'different collection port' => [ - 'databases.primary.port', - 5432, - ['servers' => ['production' => ['host' => 'prod.example.com', 'user' => 'deploy'], 'staging' => ['host' => 'staging.example.com']], 'databases' => ['primary' => ['host' => 'db1.example.com', 'port' => 5432]]], + 'different collection value' => [ + 'animals.cat.legs', + 4, + ['widgets' => ['alpha' => ['color' => 'red', 'size' => 'large'], 'beta' => ['color' => 'blue', 'size' => 'small']], 'animals' => ['cat' => ['sound' => 'meow', 'legs' => 4]]], true ], 'single segment' => [ - 'databases', - ['primary' => ['host' => 'db1.example.com', 'port' => 5432]], - ['servers' => ['production' => ['host' => 'prod.example.com', 'user' => 'deploy'], 'staging' => ['host' => 'staging.example.com']], 'databases' => ['primary' => ['host' => 'db1.example.com', 'port' => 5432]]], + 'animals', + ['cat' => ['sound' => 'meow', 'legs' => 4]], + ['widgets' => ['alpha' => ['color' => 'red', 'size' => 'large'], 'beta' => ['color' => 'blue', 'size' => 'small']], 'animals' => ['cat' => ['sound' => 'meow', 'legs' => 4]]], true ], // File exists - negative cases (non-existent paths) - 'non-existent deep path' => ['servers.web2.host', null, ['servers' => ['web1' => ['host' => 'example.com']]], true], - 'non-existent collection' => ['databases.primary', null, ['servers' => ['web1' => ['host' => 'example.com']]], true], - 'non-existent root' => ['missing', null, ['servers' => ['web1' => ['host' => 'example.com']]], true], - 'partial path match' => ['servers.web1.port', null, ['servers' => ['web1' => ['host' => 'example.com']]], true], + 'non-existent deep path' => ['widgets.gamma.color', null, ['widgets' => ['alpha' => ['color' => 'red']]], true], + 'non-existent collection' => ['animals.dog', null, ['widgets' => ['alpha' => ['color' => 'red']]], true], + 'non-existent root' => ['missing', null, ['widgets' => ['alpha' => ['color' => 'red']]], true], + 'partial path match' => ['widgets.alpha.weight', null, ['widgets' => ['alpha' => ['color' => 'red']]], true], // File doesn't exist - 'file not found' => ['servers', null, null, false], + 'file not found' => ['widgets', null, null, false], ]); // @@ -105,7 +105,7 @@ it('returns default value when path does not exist', function (string $path, mixed $default, mixed $expected) { // ARRANGE - $inventoryData = ['servers' => ['web1' => ['host' => 'example.com']]]; + $inventoryData = ['widgets' => ['alpha' => ['color' => 'red']]]; $this->service = mockInventoryService(true, $inventoryData); $this->service->loadInventoryFile(); @@ -115,12 +115,12 @@ // ASSERT expect($result)->toBe($expected); })->with([ - 'non-existent path with default' => ['servers.web2', 'default-server', 'default-server'], - 'non-existent path with array default' => ['servers.web2', ['host' => 'default.com'], ['host' => 'default.com']], - 'non-existent path with null default' => ['servers.web2', null, null], - 'non-existent path with numeric default' => ['servers.web1.port', 22, 22], - 'non-existent path with boolean default' => ['servers.web1.enabled', true, true], - 'existing path ignores default' => ['servers.web1.host', 'ignored', 'example.com'], + 'non-existent path with default' => ['widgets.beta', 'default-value', 'default-value'], + 'non-existent path with array default' => ['widgets.beta', ['color' => 'blue'], ['color' => 'blue']], + 'non-existent path with null default' => ['widgets.beta', null, null], + 'non-existent path with numeric default' => ['widgets.alpha.size', 10, 10], + 'non-existent path with boolean default' => ['widgets.alpha.visible', true, true], + 'existing path ignores default' => ['widgets.alpha.color', 'ignored', 'red'], ]); // @@ -139,25 +139,25 @@ expect($this->service->get($path))->toBeNull(); // Also verify other data remains intact (for precision testing) - if ($path === 'servers.web1.port') { - expect($this->service->get('servers.web1.host'))->toBe('example.com'); - } elseif ($path === 'servers.web1') { - expect($this->service->get('servers.web2'))->not->toBeNull(); + if ($path === 'widgets.alpha.size') { + expect($this->service->get('widgets.alpha.color'))->toBe('red'); + } elseif ($path === 'widgets.alpha') { + expect($this->service->get('widgets.beta'))->not->toBeNull(); } })->with([ 'removes specific property' => [ - 'servers.web1.port', - ['servers' => ['web1' => ['host' => 'example.com', 'port' => 22], 'web2' => ['host' => 'test.com']]], + 'widgets.alpha.size', + ['widgets' => ['alpha' => ['color' => 'red', 'size' => 10], 'beta' => ['color' => 'blue']]], 'property removal' ], 'removes entire nested structure' => [ - 'servers.web1', - ['servers' => ['web1' => ['host' => 'example.com'], 'web2' => ['host' => 'test.com']]], + 'widgets.alpha', + ['widgets' => ['alpha' => ['color' => 'red'], 'beta' => ['color' => 'blue']]], 'structure removal' ], 'handles non-existent path gracefully' => [ - 'servers.web2', - ['servers' => ['web1' => ['host' => 'example.com']]], + 'widgets.gamma', + ['widgets' => ['alpha' => ['color' => 'red']]], 'graceful handling' ], ]); @@ -181,7 +181,7 @@ $service->loadInventoryFile(); // ACT & ASSERT - expect(fn () => $service->set('servers.web1', 'value')) + expect(fn () => $service->set('widgets.alpha', 'value')) ->toThrow(RuntimeException::class, 'Error writing inventory file'); }); @@ -199,7 +199,7 @@ $service = mockInventoryService(false, ''); // ACT & ASSERT - expect(fn () => $service->set('servers.web1', 'value')) + expect(fn () => $service->set('widgets.alpha', 'value')) ->toThrow(RuntimeException::class, 'Inventory not loaded. Call loadInventoryFile() first.'); }); @@ -222,7 +222,7 @@ } })->with([ // File exists with content - [true, ['servers' => ['web1' => ['host' => 'example.com', 'port' => 22]]], false, false, false, '/^Reading inventory from .+\.yml$/'], + [true, ['widgets' => ['alpha' => ['color' => 'red', 'size' => 10]]], false, false, false, '/^Reading inventory from .+\.yml$/'], // File exists with single item [true, ['single_key' => 'value'], false, false, false, '/^Reading inventory from .+\.yml$/'], @@ -231,7 +231,7 @@ [true, [], false, false, false, '/^Reading inventory from .+\.yml$/'], // File exists with complex structure - [true, ['environments' => ['prod' => ['db' => ['host' => 'prod-db']]]], false, false, false, '/^Reading inventory from .+\.yml$/'], + [true, ['categories' => ['shapes' => ['circle' => ['radius' => 5]]]], false, false, false, '/^Reading inventory from .+\.yml$/'], // File exists but has read error (throws exception) [true, ['key' => 'value'], true, false, true, null], diff --git a/tests/Unit/TestHelpersTest.php b/tests/Unit/TestHelpersTest.php index 08f079ee..97c3d5ce 100644 --- a/tests/Unit/TestHelpersTest.php +++ b/tests/Unit/TestHelpersTest.php @@ -144,13 +144,13 @@ $service->loadInventoryFile(); // ACT - $result = $service->get('servers.web1.host'); + $result = $service->get('widgets.alpha.color'); // ASSERT - expect($result)->toBe('example.com'); + expect($result)->toBe('red'); })->with([ - 'array data' => [['servers' => ['web1' => ['host' => 'example.com']]]], - 'string data' => ['servers:' . PHP_EOL . ' web1:' . PHP_EOL . ' host: example.com'], + 'array data' => [['widgets' => ['alpha' => ['color' => 'red']]]], + 'string data' => ['widgets:' . PHP_EOL . ' alpha:' . PHP_EOL . ' color: red'], ]); }); From 1fbf3d06cc8c72510c0ba6530d00a0293f82314a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucian=20V=C4=83c=C4=83roiu?= Date: Fri, 3 Oct 2025 13:05:28 +0300 Subject: [PATCH 3/3] docs(cursor): expand review commands to check for bugs --- .cursor/commands/_review.md | 2 +- .cursor/commands/review-branch.md | 2 +- .cursor/commands/review-diff.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.cursor/commands/_review.md b/.cursor/commands/_review.md index 8f246a83..0325e1af 100644 --- a/.cursor/commands/_review.md +++ b/.cursor/commands/_review.md @@ -1 +1 @@ -review everything thoroughly with a focus on where these fall short of our development, architecture and testing rules +review everything thoroughly with a focus on where these fall short of our development, architecture and testing rules as well as finding potential bugs and regressions diff --git a/.cursor/commands/review-branch.md b/.cursor/commands/review-branch.md index 7a32fbeb..7de8fc93 100644 --- a/.cursor/commands/review-branch.md +++ b/.cursor/commands/review-branch.md @@ -1,5 +1,5 @@ Meticulously catalog and analyze all the changes in this branch, compared to the branch it's based on. -Review everything thoroughly with a focus on where these fall short of our development, architecture and testing rules. +Review everything thoroughly with a focus on where these fall short of our development, architecture and testing rules as well as finding potential bugs and regressions Provide a detailed report but don't make any changes yet. diff --git a/.cursor/commands/review-diff.md b/.cursor/commands/review-diff.md index 9beb3d00..d4cc786f 100644 --- a/.cursor/commands/review-diff.md +++ b/.cursor/commands/review-diff.md @@ -1,5 +1,5 @@ Meticulously catalog and analyze all the changes in this Git working tree, staged or unstaged. -Review everything thoroughly with a focus on where these fall short of our development, architecture and testing rules. +Review everything thoroughly with a focus on where these fall short of our development, architecture and testing rules as well as finding potential bugs and regressions. Provide a detailed report but don't make any changes yet.