Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts Installer::update() to correctly handle upgrades where the currently installed package ($initial) has distType === 'path', avoiding Composer’s PathDownloader against an old path source that may no longer exist, and adds targeted tests to verify the new branching behavior.
Changes:
- Add a success-path guard in
Installer::update()to manually update the installed repository when upgrading from apathdist-type package (skippingparent::update()repo tracking delegation). - Extend
TestableInstallerwith invocation flags to assert whether base-download and repo-tracking delegation paths are used. - Add two unit tests covering the “initial is path” vs “initial is not path” update flows.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Installer.php | Adds manual installed-repo tracking when upgrading from distType=path to avoid invoking PathDownloader via parent::update(). |
| tests/ModuleInstallerTest.php | Adds invocation flags and two tests to assert the correct update branch behavior for path vs non-path initial packages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // update() merge-strategy guard for path-type initial packages | ||
| // ------------------------------------------------------------------------- | ||
|
|
||
| public function test_update_skips_base_download_and_delegates_repo_manually_when_initial_is_path_type(): void |
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.
This pull request improves the handling of package updates in the installer, specifically addressing scenarios where the initial package is of type
path. The main change ensures that when upgrading from a path-type package, the installer properly manages repository tracking without attempting to re-download files that may not exist. Additionally, comprehensive tests are added to verify the new behavior and to ensure correct delegation logic.Installer logic improvements:
update()method inInstaller.phpto manually track repository changes when the initial package is of typepath, avoiding unnecessary downloads and ensuring correct package removal and addition in the repository.Test coverage enhancements:
downloadBaseInvokedanddelegateRepoTrackingInvokedflags toTestableInstallerinModuleInstallerTest.phpto verify whether download and delegation methods are called during updates.test_update_skips_base_download_and_delegates_repo_manually_when_initial_is_path_typeto confirm that updating from apathpackage skips the download and manually manages repository tracking.test_update_downloads_base_and_delegates_repo_to_parent_when_initial_is_not_path_typeto confirm that for non-pathpackages, the base download and parent repository tracking are invoked as expected.