test: improve InstallationCompletedEvent testing to verify parameter extraction from install options#181
Merged
printminion-co merged 5 commits intofeature/add_postinstall_eventfrom Jan 13, 2026
Conversation
…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>
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>
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>
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>
Copilot
AI
changed the title
[WIP] Update postinstall event feature implementation
test: improve InstallationCompletedEvent testing to verify parameter extraction from install options
Jan 13, 2026
printminion-co
merged commit bcdf047
into
feature/add_postinstall_event
4 of 12 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The original test
testEventDispatcherCanDispatchInstallationCompletedEventcreated a mock dispatcher and manually calleddispatchTyped(), which didn't verify thatSetup::install()correctly extracts parameters from the options array and dispatches the event.Changes
testInstallationCompletedEventParametersFromInstallOptions()- verifies event construction using parameters extracted from install options matchingSetup::install()logic (lines 329, 502-503, 504-506)testInstallationCompletedEventWithDisabledAdminUser()- testsadmindisable=truescenario resulting in null admin usernameextractInstallationEventParameters()helper to mirror parameter extraction logic and reduce duplicationNote: Full integration testing of
Setup::install()requires database setup, user creation, app installation, and Server dependency injection - impractical for unit tests. TheInstallationCompletedEventclass itself is comprehensively tested inInstallationCompletedEventTest.php.TODO
N/A
Checklist
3. to review, feature component)stable32)💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.