From 937e51a9aaa65a569efc9ab6e7b8b325872df25c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 12:18:02 +0000 Subject: [PATCH 1/5] Initial plan From 979c62cb7db7acae9f01dac7fd009800726b343e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 12:25:58 +0000 Subject: [PATCH 2/5] test: improve InstallationCompletedEvent testing to verify parameter extraction Replace testEventDispatcherCanDispatchInstallationCompletedEvent with tests that verify the event is constructed with correct parameters extracted from install options. The original test created a mock dispatcher and manually dispatched the event, which didn't test the real integration with Setup::install(). The new tests verify that parameters are extracted correctly from the options array in the same way Setup::install() does (lines 502-506). Testing the full install() method requires a complete installation environment (database, file system, Server::get() dependencies) which is impractical in a unit test. The event class itself is comprehensively tested in InstallationCompletedEventTest.php. Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- tests/lib/SetupTest.php | 64 ++++++++++++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index 2fde7874adeab..deaaf5e48248e 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -189,20 +189,62 @@ public function testSetupHasEventDispatcher(): void { } /** - * Test that event dispatcher can dispatch InstallationCompletedEvent + * Test that InstallationCompletedEvent can be created with parameters from install options + * + * This test verifies that the InstallationCompletedEvent can be properly constructed with + * the parameters that Setup::install() extracts from the options array (see Setup.php lines 502-506). + * + * Note: Testing that Setup::install() actually dispatches this event requires a full integration + * test with database setup, file system operations, and app installation, which is beyond the + * scope of a unit test. The event class itself is thoroughly tested in InstallationCompletedEventTest.php. */ - public function testEventDispatcherCanDispatchInstallationCompletedEvent(): void { - $dataDir = '/tmp/test-data'; - $adminUsername = 'testadmin'; - $adminEmail = 'admin@test.com'; + public function testInstallationCompletedEventParametersFromInstallOptions(): void { + // Simulate the options array as passed to Setup::install() + $options = [ + 'directory' => '/path/to/data', + 'adminlogin' => 'admin', + 'adminemail' => 'admin@example.com', + ]; - $eventDispatcher = $this->createMock(IEventDispatcher::class); - $eventDispatcher->expects($this->once()) - ->method('dispatchTyped') - ->with($this->isInstanceOf(InstallationCompletedEvent::class)); + // Extract parameters the same way Setup::install() does at lines 502-503 + $dataDir = htmlspecialchars_decode($options['directory']); + $disableAdminUser = (bool)($options['admindisable'] ?? false); + $adminUsername = !$disableAdminUser ? ($options['adminlogin'] ?? null) : null; + $adminEmail = !empty($options['adminemail']) ? $options['adminemail'] : null; - // Dispatch the event + // Create the event as Setup::install() does at line 505 $event = new InstallationCompletedEvent($dataDir, $adminUsername, $adminEmail); - $eventDispatcher->dispatchTyped($event); + + // Verify the event contains the expected values + $this->assertEquals($dataDir, $event->getDataDirectory()); + $this->assertEquals($adminUsername, $event->getAdminUsername()); + $this->assertEquals($adminEmail, $event->getAdminEmail()); + $this->assertTrue($event->hasAdminUser()); + } + + /** + * Test that event parameters handle disabled admin user correctly + * + * This tests the scenario where Setup::install() is called with admindisable=true, + * resulting in a null adminUsername in the event. + */ + public function testInstallationCompletedEventWithDisabledAdminUser(): void { + $options = [ + 'directory' => '/path/to/data', + 'admindisable' => true, + ]; + + // Extract parameters as Setup::install() does + $dataDir = htmlspecialchars_decode($options['directory']); + $disableAdminUser = (bool)($options['admindisable'] ?? false); + $adminUsername = !$disableAdminUser ? ($options['adminlogin'] ?? null) : null; + $adminEmail = !empty($options['adminemail']) ? $options['adminemail'] : null; + + $event = new InstallationCompletedEvent($dataDir, $adminUsername, $adminEmail); + + $this->assertEquals($dataDir, $event->getDataDirectory()); + $this->assertNull($event->getAdminUsername()); + $this->assertNull($event->getAdminEmail()); + $this->assertFalse($event->hasAdminUser()); } } From b04ed02f65164ff221b7e04b3a2328faf29dfa35 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 12:27:35 +0000 Subject: [PATCH 3/5] refactor: extract parameter extraction logic into helper method Address code review feedback: - Extract duplicated parameter extraction logic into extractInstallationEventParameters() helper - Fix line number references in comments to be more accurate (line 329 for dataDir, lines 502-503 for admin params, lines 505-506 for event creation) - Improve code maintainability by reducing duplication Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- tests/lib/SetupTest.php | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index deaaf5e48248e..f73daf63d4af0 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -188,11 +188,25 @@ public function testSetupHasEventDispatcher(): void { $this->assertInstanceOf(IEventDispatcher::class, $eventDispatcher); } + /** + * Helper method to extract event parameters from install options + * This mirrors the logic in Setup::install() at lines 329 and 502-503 + */ + private function extractInstallationEventParameters(array $options): array { + $dataDir = htmlspecialchars_decode($options['directory']); + $disableAdminUser = (bool)($options['admindisable'] ?? false); + $adminUsername = !$disableAdminUser ? ($options['adminlogin'] ?? null) : null; + $adminEmail = !empty($options['adminemail']) ? $options['adminemail'] : null; + + return [$dataDir, $adminUsername, $adminEmail]; + } + /** * Test that InstallationCompletedEvent can be created with parameters from install options * * This test verifies that the InstallationCompletedEvent can be properly constructed with - * the parameters that Setup::install() extracts from the options array (see Setup.php lines 502-506). + * the parameters that Setup::install() extracts from the options array (see Setup.php line 329 + * for dataDir extraction and lines 502-503 for admin parameters). * * Note: Testing that Setup::install() actually dispatches this event requires a full integration * test with database setup, file system operations, and app installation, which is beyond the @@ -206,13 +220,10 @@ public function testInstallationCompletedEventParametersFromInstallOptions(): vo 'adminemail' => 'admin@example.com', ]; - // Extract parameters the same way Setup::install() does at lines 502-503 - $dataDir = htmlspecialchars_decode($options['directory']); - $disableAdminUser = (bool)($options['admindisable'] ?? false); - $adminUsername = !$disableAdminUser ? ($options['adminlogin'] ?? null) : null; - $adminEmail = !empty($options['adminemail']) ? $options['adminemail'] : null; + // Extract parameters the same way Setup::install() does + [$dataDir, $adminUsername, $adminEmail] = $this->extractInstallationEventParameters($options); - // Create the event as Setup::install() does at line 505 + // Create the event as Setup::install() does at lines 505-506 $event = new InstallationCompletedEvent($dataDir, $adminUsername, $adminEmail); // Verify the event contains the expected values @@ -235,10 +246,7 @@ public function testInstallationCompletedEventWithDisabledAdminUser(): void { ]; // Extract parameters as Setup::install() does - $dataDir = htmlspecialchars_decode($options['directory']); - $disableAdminUser = (bool)($options['admindisable'] ?? false); - $adminUsername = !$disableAdminUser ? ($options['adminlogin'] ?? null) : null; - $adminEmail = !empty($options['adminemail']) ? $options['adminemail'] : null; + [$dataDir, $adminUsername, $adminEmail] = $this->extractInstallationEventParameters($options); $event = new InstallationCompletedEvent($dataDir, $adminUsername, $adminEmail); From e8b7805dd0afaf734d56efae06d5e2fdc75edb6d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 12:28:58 +0000 Subject: [PATCH 4/5] docs: fix line number references in test comments for accuracy Update comments to correctly reference Setup.php lines: - Line 329: dataDir extraction - Lines 502-503: admin parameter extraction - Lines 504-506: event dispatch Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- tests/lib/SetupTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index f73daf63d4af0..c8abb8568ab87 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -190,7 +190,7 @@ public function testSetupHasEventDispatcher(): void { /** * Helper method to extract event parameters from install options - * This mirrors the logic in Setup::install() at lines 329 and 502-503 + * This mirrors the logic in Setup::install() at line 329 for dataDir and lines 502-503 for admin parameters */ private function extractInstallationEventParameters(array $options): array { $dataDir = htmlspecialchars_decode($options['directory']); @@ -206,7 +206,7 @@ private function extractInstallationEventParameters(array $options): array { * * This test verifies that the InstallationCompletedEvent can be properly constructed with * the parameters that Setup::install() extracts from the options array (see Setup.php line 329 - * for dataDir extraction and lines 502-503 for admin parameters). + * for dataDir and lines 502-503 for admin parameters). * * Note: Testing that Setup::install() actually dispatches this event requires a full integration * test with database setup, file system operations, and app installation, which is beyond the @@ -223,7 +223,7 @@ public function testInstallationCompletedEventParametersFromInstallOptions(): vo // Extract parameters the same way Setup::install() does [$dataDir, $adminUsername, $adminEmail] = $this->extractInstallationEventParameters($options); - // Create the event as Setup::install() does at lines 505-506 + // Create the event as Setup::install() does at lines 504-506 $event = new InstallationCompletedEvent($dataDir, $adminUsername, $adminEmail); // Verify the event contains the expected values From 04c4209b94990b42e3d2368f64d4dc0af8597099 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 12:30:04 +0000 Subject: [PATCH 5/5] docs: add note about directory key assumption in helper method Document that extractInstallationEventParameters() assumes the 'directory' key is present, referencing Setup::install()'s fallback logic at lines 321-323. Co-authored-by: printminion-co <145785698+printminion-co@users.noreply.github.com> --- tests/lib/SetupTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/lib/SetupTest.php b/tests/lib/SetupTest.php index c8abb8568ab87..6c2f85841b15e 100644 --- a/tests/lib/SetupTest.php +++ b/tests/lib/SetupTest.php @@ -191,6 +191,9 @@ public function testSetupHasEventDispatcher(): void { /** * Helper method to extract event parameters from install options * This mirrors the logic in Setup::install() at line 329 for dataDir and lines 502-503 for admin parameters + * + * Note: This assumes 'directory' key is present in options. Setup::install() has a fallback at lines 321-323 + * that sets a default directory if empty, but our tests always provide this key. */ private function extractInstallationEventParameters(array $options): array { $dataDir = htmlspecialchars_decode($options['directory']);