From 82502684b21bb179186ec392a2d86c8877636da3 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 20 Apr 2020 21:44:15 +0200 Subject: [PATCH 1/3] Allow testing for installed repo state --- tests/Composer/Test/InstallerTest.php | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/Composer/Test/InstallerTest.php b/tests/Composer/Test/InstallerTest.php index 03d301164..f9e1b1793 100644 --- a/tests/Composer/Test/InstallerTest.php +++ b/tests/Composer/Test/InstallerTest.php @@ -184,7 +184,7 @@ class InstallerTest extends TestCase /** * @dataProvider getIntegrationTests */ - public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $run, $expectLock, $expectOutput, $expect, $expectResult) + public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $run, $expectLock, $expectInstalled, $expectOutput, $expect, $expectResult) { if ($condition) { eval('$res = '.$condition.';'); @@ -327,6 +327,23 @@ class InstallerTest extends TestCase $this->assertEquals($expectLock, $actualLock); } + if ($expectInstalled !== null) { + $actualInstalled = array(); + $dumper = new ArrayDumper(); + + foreach ($repositoryManager->getLocalRepository()->getCanonicalPackages() as $package) { + $package = $dumper->dump($package); + unset($package['version_normalized']); + $actualInstalled[] = $package; + } + + usort($actualInstalled, function ($a, $b) { + return strcmp($a['name'], $b['name']); + }); + + $this->assertSame($expectInstalled, $actualInstalled); + } + $installationManager = $composer->getInstallationManager(); $this->assertSame(rtrim($expect), implode("\n", $installationManager->getTrace())); @@ -355,6 +372,7 @@ class InstallerTest extends TestCase $installedDev = array(); $lock = array(); $expectLock = array(); + $expectInstalled = null; $expectResult = 0; $message = $testData['TEST']; @@ -393,6 +411,9 @@ class InstallerTest extends TestCase $expectLock = JsonFile::parseJson($testData['EXPECT-LOCK']); } } + if (!empty($testData['EXPECT-INSTALLED'])) { + $expectInstalled = JsonFile::parseJson($testData['EXPECT-INSTALLED']); + } $expectOutput = isset($testData['EXPECT-OUTPUT']) ? $testData['EXPECT-OUTPUT'] : null; $expect = $testData['EXPECT']; if (!empty($testData['EXPECT-EXCEPTION'])) { @@ -409,7 +430,7 @@ class InstallerTest extends TestCase die(sprintf('Test "%s" is not valid: '.$e->getMessage(), str_replace($fixturesDir.'/', '', $file))); } - $tests[basename($file)] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $run, $expectLock, $expectOutput, $expect, $expectResult); + $tests[basename($file)] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $run, $expectLock, $expectInstalled, $expectOutput, $expect, $expectResult); } return $tests; @@ -427,6 +448,7 @@ class InstallerTest extends TestCase 'INSTALLED' => false, 'RUN' => true, 'EXPECT-LOCK' => false, + 'EXPECT-INSTALLED' => false, 'EXPECT-OUTPUT' => false, 'EXPECT-EXIT-CODE' => false, 'EXPECT-EXCEPTION' => false, From 17ed09be2ee33afaf9838f82ed01eccf6fd3c7db Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 20 Apr 2020 21:46:56 +0200 Subject: [PATCH 2/3] Add failing test showing that packages fail to be installed if they match a previous alias which was not removed yet --- .../installer/update-alias-lock2.test | 84 +++++++++++++++++++ .../Test/Mock/InstallationManagerMock.php | 4 +- 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 tests/Composer/Test/Fixtures/installer/update-alias-lock2.test diff --git a/tests/Composer/Test/Fixtures/installer/update-alias-lock2.test b/tests/Composer/Test/Fixtures/installer/update-alias-lock2.test new file mode 100644 index 000000000..b06ce3119 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/update-alias-lock2.test @@ -0,0 +1,84 @@ +--TEST-- +Updating an aliased package where the old alias matches the new package should not fail +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { + "name": "a/a", "version": "1.10.x-dev", + "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } }, + "source": { "type": "git", "url": "", "reference": "downgradedref" } + }, + { + "name": "a/a", "version": "dev-master", + "extra": { "branch-alias": { "dev-master": "2.x-dev" } }, + "source": { "type": "git", "url": "", "reference": "newref" } + } + ] + } + ], + "require": { + "a/a": "^1.0" + }, + "minimum-stability": "dev" +} +--LOCK-- +{ + "_": "outdated lock file, should not have to be loaded in an update", + "packages": [ + { + "name": "a/a", "version": "dev-master", + "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } }, + "source": { "type": "git", "url": "", "reference": "installedref" } + } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "dev", + "stability-flags": [], + "prefer-stable": false, + "prefer-lowest": false +} +--INSTALLED-- +[ + { + "name": "a/a", "version": "dev-master", + "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } }, + "source": { "type": "git", "url": "", "reference": "installedref" } + } +] +--RUN-- +update +--EXPECT-LOCK-- +{ + "packages": [ + { + "name": "a/a", "version": "1.10.x-dev", + "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } }, + "source": { "type": "git", "url": "", "reference": "downgradedref" }, + "type": "library" + } + ], + "packages-dev": [], + "aliases": [], + "minimum-stability": "dev", + "stability-flags": [], + "prefer-stable": false, + "prefer-lowest": false, + "platform": [], + "platform-dev": [] +} +--EXPECT-INSTALLED-- +[ + { + "name": "a/a", "version": "1.10.x-dev", + "source": { "type": "git", "url": "", "reference": "downgradedref" }, + "type": "library", + "extra": { "branch-alias": { "dev-master": "1.10.x-dev" } } + } +] +--EXPECT-- +Marking a/a (1.10.x-dev installedref) as uninstalled, alias of a/a (dev-master installedref) +Downgrading a/a (dev-master installedref => 1.10.x-dev downgradedref) diff --git a/tests/Composer/Test/Mock/InstallationManagerMock.php b/tests/Composer/Test/Mock/InstallationManagerMock.php index 2d4a8781b..1d1dfef8c 100644 --- a/tests/Composer/Test/Mock/InstallationManagerMock.php +++ b/tests/Composer/Test/Mock/InstallationManagerMock.php @@ -66,7 +66,9 @@ class InstallationManagerMock extends InstallationManager $this->updated[] = array($operation->getInitialPackage(), $operation->getTargetPackage()); $this->trace[] = strip_tags((string) $operation); $repo->removePackage($operation->getInitialPackage()); - $repo->addPackage(clone $operation->getTargetPackage()); + if (!$repo->hasPackage($operation->getTargetPackage())) { + $repo->addPackage(clone $operation->getTargetPackage()); + } } public function uninstall(RepositoryInterface $repo, UninstallOperation $operation) From ba9d4793bc31d36c16fe275bc9197a780a031ed6 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Mon, 20 Apr 2020 22:02:07 +0200 Subject: [PATCH 3/3] Fix transaction order --- src/Composer/DependencyResolver/Transaction.php | 2 +- tests/Composer/Test/DependencyResolver/TransactionTest.php | 2 +- .../partial-update-installs-from-lock-even-missing.test | 4 ++-- tests/Composer/Test/Fixtures/installer/update-alias.test | 2 +- .../installer/update-allow-list-with-dependencies-alias.test | 2 +- .../installer/update-downgrades-unstable-packages.test | 2 +- .../Fixtures/installer/update-no-dev-still-resolves-dev.test | 2 +- .../installer/updating-dev-from-lock-removes-old-deps.test | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Composer/DependencyResolver/Transaction.php b/src/Composer/DependencyResolver/Transaction.php index 6f7cc79fb..1e681a04f 100644 --- a/src/Composer/DependencyResolver/Transaction.php +++ b/src/Composer/DependencyResolver/Transaction.php @@ -295,7 +295,7 @@ class Transaction { $uninstOps = array(); foreach ($operations as $idx => $op) { - if ($op instanceof Operation\UninstallOperation) { + if ($op instanceof Operation\UninstallOperation || $op instanceof Operation\MarkAliasUninstalledOperation) { $uninstOps[] = $op; unset($operations[$idx]); } diff --git a/tests/Composer/Test/DependencyResolver/TransactionTest.php b/tests/Composer/Test/DependencyResolver/TransactionTest.php index 8b3e66b68..b5d209041 100644 --- a/tests/Composer/Test/DependencyResolver/TransactionTest.php +++ b/tests/Composer/Test/DependencyResolver/TransactionTest.php @@ -53,6 +53,7 @@ class TransactionTest extends TestCase $expectedOperations = array( array('job' => 'uninstall', 'package' => $packageC), array('job' => 'uninstall', 'package' => $packageE), + array('job' => 'markAliasUninstalled', 'package' => $packageEalias), array('job' => 'install', 'package' => $packageA0first), array('job' => 'update', 'from' => $packageB, 'to' => $packageBnew), array('job' => 'install', 'package' => $packageG), @@ -60,7 +61,6 @@ class TransactionTest extends TestCase array('job' => 'markAliasInstalled', 'package' => $packageFalias1), array('job' => 'markAliasInstalled', 'package' => $packageFalias2), array('job' => 'install', 'package' => $packageD), - array('job' => 'markAliasUninstalled', 'package' => $packageEalias), ); $transaction = new Transaction($presentPackages, $resultPackages); diff --git a/tests/Composer/Test/Fixtures/installer/partial-update-installs-from-lock-even-missing.test b/tests/Composer/Test/Fixtures/installer/partial-update-installs-from-lock-even-missing.test index 7530ca862..9d971b384 100644 --- a/tests/Composer/Test/Fixtures/installer/partial-update-installs-from-lock-even-missing.test +++ b/tests/Composer/Test/Fixtures/installer/partial-update-installs-from-lock-even-missing.test @@ -97,9 +97,9 @@ update b/b "platform-dev": [] } --EXPECT-- +Marking a/a (2.1.x-dev oldmaster-a) as uninstalled, alias of a/a (dev-master oldmaster-a) +Marking b/b (2.1.x-dev oldmaster-b) as uninstalled, alias of b/b (dev-master oldmaster-b) Upgrading a/a (dev-master oldmaster-a => dev-master newmaster-a) Marking a/a (2.2.x-dev newmaster-a) as installed, alias of a/a (dev-master newmaster-a) Upgrading b/b (dev-master oldmaster-b => dev-master newmaster-b2) Marking b/b (2.3.x-dev newmaster-b2) as installed, alias of b/b (dev-master newmaster-b2) -Marking a/a (2.1.x-dev oldmaster-a) as uninstalled, alias of a/a (dev-master oldmaster-a) -Marking b/b (2.1.x-dev oldmaster-b) as uninstalled, alias of b/b (dev-master oldmaster-b) diff --git a/tests/Composer/Test/Fixtures/installer/update-alias.test b/tests/Composer/Test/Fixtures/installer/update-alias.test index 8da3d4d23..944eed259 100644 --- a/tests/Composer/Test/Fixtures/installer/update-alias.test +++ b/tests/Composer/Test/Fixtures/installer/update-alias.test @@ -33,5 +33,5 @@ Update aliased package to non-aliased version --RUN-- update --EXPECT-- -Upgrading a/a (dev-master master => dev-foo foo) Marking a/a (1.0.x-dev master) as uninstalled, alias of a/a (dev-master master) +Upgrading a/a (dev-master master => dev-foo foo) diff --git a/tests/Composer/Test/Fixtures/installer/update-allow-list-with-dependencies-alias.test b/tests/Composer/Test/Fixtures/installer/update-allow-list-with-dependencies-alias.test index 2b68c6c69..193a5d85d 100644 --- a/tests/Composer/Test/Fixtures/installer/update-allow-list-with-dependencies-alias.test +++ b/tests/Composer/Test/Fixtures/installer/update-allow-list-with-dependencies-alias.test @@ -91,9 +91,9 @@ update new/pkg --with-all-dependencies "platform-dev": [] } --EXPECT-- +Marking current/dep2 (1.0.x-dev) as uninstalled, alias of current/dep2 (dev-foo) Marking current/dep (1.1.0) as installed, alias of current/dep (dev-master) Upgrading current/dep2 (dev-foo => dev-master) Marking current/dep2 (1.1.2) as installed, alias of current/dep2 (dev-master) Marking current/dep2 (2.x-dev) as installed, alias of current/dep2 (dev-master) Installing new/pkg (1.0.0) -Marking current/dep2 (1.0.x-dev) as uninstalled, alias of current/dep2 (dev-foo) diff --git a/tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test b/tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test index f0755d0d0..b213e480d 100644 --- a/tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test +++ b/tests/Composer/Test/Fixtures/installer/update-downgrades-unstable-packages.test @@ -46,5 +46,5 @@ Downgrading from unstable to more stable package should work even if already ins --RUN-- update --EXPECT-- -Downgrading a/a (dev-master abcd => 1.0.0) Marking a/a (9999999-dev abcd) as uninstalled, alias of a/a (dev-master abcd) +Downgrading a/a (dev-master abcd => 1.0.0) diff --git a/tests/Composer/Test/Fixtures/installer/update-no-dev-still-resolves-dev.test b/tests/Composer/Test/Fixtures/installer/update-no-dev-still-resolves-dev.test index 06cbc27c7..9844616f4 100644 --- a/tests/Composer/Test/Fixtures/installer/update-no-dev-still-resolves-dev.test +++ b/tests/Composer/Test/Fixtures/installer/update-no-dev-still-resolves-dev.test @@ -61,8 +61,8 @@ Updates with --no-dev but we still end up with a complete lock file including de update --no-dev --EXPECT-- Removing a/b (1.0.0) +Marking dev/pkg (1.0.x-dev old) as uninstalled, alias of dev/pkg (dev-master old) Upgrading a/a (1.0.0 => 1.0.1) Installing a/c (1.0.0) Upgrading dev/pkg (dev-master old => dev-master new) Marking dev/pkg (1.1.x-dev new) as installed, alias of dev/pkg (dev-master new) -Marking dev/pkg (1.0.x-dev old) as uninstalled, alias of dev/pkg (dev-master old) diff --git a/tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test b/tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test index 3fb6654ab..ec92ea607 100644 --- a/tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test +++ b/tests/Composer/Test/Fixtures/installer/updating-dev-from-lock-removes-old-deps.test @@ -42,5 +42,5 @@ Installing locked dev packages should remove old dependencies install --EXPECT-- Removing a/dependency (dev-master ref) -Upgrading a/devpackage (dev-master oldref => dev-master newref) Marking a/dependency (9999999-dev ref) as uninstalled, alias of a/dependency (dev-master ref) +Upgrading a/devpackage (dev-master oldref => dev-master newref)