From f181dc84e2348315389890b1cf5280a2a40679bc Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Wed, 5 Sep 2012 20:39:45 +0200 Subject: [PATCH 1/3] Added tests for the suggestions The test about replaced packages is failing because of #752. --- .../Fixtures/installer/suggest-installed.test | 29 +++++++++++++++++++ .../Fixtures/installer/suggest-replaced.test | 29 +++++++++++++++++++ .../installer/suggest-uninstalled.test | 27 +++++++++++++++++ tests/Composer/Test/InstallerTest.php | 10 +++++-- 4 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 tests/Composer/Test/Fixtures/installer/suggest-installed.test create mode 100644 tests/Composer/Test/Fixtures/installer/suggest-replaced.test create mode 100644 tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test diff --git a/tests/Composer/Test/Fixtures/installer/suggest-installed.test b/tests/Composer/Test/Fixtures/installer/suggest-installed.test new file mode 100644 index 000000000..b84f0a252 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/suggest-installed.test @@ -0,0 +1,29 @@ +--TEST-- +Suggestions are not displayed for installed packages if they are also installed +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "a/a", "version": "1.0.0", "suggest": { "b/b": "an obscure reason" } }, + { "name": "b/b", "version": "1.0.0" } + ] + } + ], + "require": { + "a/a": "1.0.0", + "b/b": "1.0.0" + } +} +--RUN-- +install +--EXPECT-OUTPUT-- +Loading composer repositories with package information +Installing dependencies +Writing lock file +Generating autoload files + +--EXPECT-- +Installing a/a (1.0.0) +Installing b/b (1.0.0) \ No newline at end of file diff --git a/tests/Composer/Test/Fixtures/installer/suggest-replaced.test b/tests/Composer/Test/Fixtures/installer/suggest-replaced.test new file mode 100644 index 000000000..3691e6070 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/suggest-replaced.test @@ -0,0 +1,29 @@ +--TEST-- +Suggestions are not displayed for installed packages if they are replaced +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "a/a", "version": "1.0.0", "suggest": { "b/b": "an obscure reason" } }, + { "name": "c/c", "version": "1.0.0", "replace": { "b/b": "1.0.0" } } + ] + } + ], + "require": { + "a/a": "1.0.0", + "b/b": "1.0.0" + } +} +--RUN-- +install +--EXPECT-OUTPUT-- +Loading composer repositories with package information +Installing dependencies +Writing lock file +Generating autoload files + +--EXPECT-- +Installing a/a (1.0.0) +Installing c/c (1.0.0) \ No newline at end of file diff --git a/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test b/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test new file mode 100644 index 000000000..a4ad14fd7 --- /dev/null +++ b/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test @@ -0,0 +1,27 @@ +--TEST-- +Suggestions are displayed for installed packages +--COMPOSER-- +{ + "repositories": [ + { + "type": "package", + "package": [ + { "name": "a/a", "version": "1.0.0", "suggest": { "b/b": "an obscure reason" } } + ] + } + ], + "require": { + "a/a": "1.0.0" + } +} +--RUN-- +install +--EXPECT-OUTPUT-- +Loading composer repositories with package information +Installing dependencies +a/a suggests installing b/b (an obscure reason) +Writing lock file +Generating autoload files + +--EXPECT-- +Installing a/a (1.0.0) \ No newline at end of file diff --git a/tests/Composer/Test/InstallerTest.php b/tests/Composer/Test/InstallerTest.php index d4635f246..cca2fd95f 100644 --- a/tests/Composer/Test/InstallerTest.php +++ b/tests/Composer/Test/InstallerTest.php @@ -123,7 +123,7 @@ class InstallerTest extends TestCase /** * @dataProvider getIntegrationTests */ - public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $installedDev, $run, $expectLock, $expect) + public function testIntegration($file, $message, $condition, $composerConfig, $lock, $installed, $installedDev, $run, $expectLock, $expectOutput, $expect) { if ($condition) { eval('$res = '.$condition.';'); @@ -226,6 +226,10 @@ class InstallerTest extends TestCase $installationManager = $composer->getInstallationManager(); $this->assertSame($expect, implode("\n", $installationManager->getTrace())); + + if ($expectOutput) { + $this->assertEquals($expectOutput, $output); + } } public function getIntegrationTests() @@ -250,6 +254,7 @@ class InstallerTest extends TestCase (?:--INSTALLED-DEV--\s*(?P'.$content.'))?\s* --RUN--\s*(?P.*?)\s* (?:--EXPECT-LOCK--\s*(?P'.$content.'))?\s* + (?:--EXPECT-OUTPUT--\s*(?P'.$content.'))?\s* --EXPECT--\s*(?P.*?)\s* $}xs'; @@ -279,6 +284,7 @@ class InstallerTest extends TestCase if (!empty($match['expectLock'])) { $expectLock = JsonFile::parseJson($match['expectLock']); } + $expectOutput = $match['expectOutput']; $expect = $match['expect']; } catch (\Exception $e) { die(sprintf('Test "%s" is not valid: '.$e->getMessage(), str_replace($fixturesDir.'/', '', $file))); @@ -287,7 +293,7 @@ class InstallerTest extends TestCase die(sprintf('Test "%s" is not valid, did not match the expected format.', str_replace($fixturesDir.'/', '', $file))); } - $tests[] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $installedDev, $run, $expectLock, $expect); + $tests[] = array(str_replace($fixturesDir.'/', '', $file), $message, $condition, $composer, $lock, $installed, $installedDev, $run, $expectLock, $expectOutput, $expect); } return $tests; From d0faa016c18eca66f81bd1682338a3ca8d211a1a Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Wed, 5 Sep 2012 21:00:24 +0200 Subject: [PATCH 2/3] Refactored the search of suggested packages to support replacements Fixes #752 --- src/Composer/Installer.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index d14662518..0fc8185b8 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -188,7 +188,16 @@ class Installer // output suggestions foreach ($this->suggestedPackages as $suggestion) { - if (!$installedRepo->findPackages($suggestion['target'])) { + $target = $suggestion['target']; + if ($installedRepo->filterPackages(function (PackageInterface $package) use ($target) { + // check the name first as it is the common case + if ($package->getName() === $target) { + return false; + } + if (in_array($target, $package->getNames())) { + return false; + } + })) { $this->io->write($suggestion['source'].' suggests installing '.$suggestion['target'].' ('.$suggestion['reason'].')'); } } From 9e372b1d8bd32772f1baf71dc2c88fdf3535ce7d Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Wed, 5 Sep 2012 22:56:59 +0200 Subject: [PATCH 3/3] Simplified the code --- src/Composer/Installer.php | 4 ---- tests/Composer/Test/Fixtures/installer/suggest-installed.test | 2 +- tests/Composer/Test/Fixtures/installer/suggest-replaced.test | 2 +- .../Composer/Test/Fixtures/installer/suggest-uninstalled.test | 2 +- 4 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/Composer/Installer.php b/src/Composer/Installer.php index 0fc8185b8..db26fdfed 100644 --- a/src/Composer/Installer.php +++ b/src/Composer/Installer.php @@ -190,10 +190,6 @@ class Installer foreach ($this->suggestedPackages as $suggestion) { $target = $suggestion['target']; if ($installedRepo->filterPackages(function (PackageInterface $package) use ($target) { - // check the name first as it is the common case - if ($package->getName() === $target) { - return false; - } if (in_array($target, $package->getNames())) { return false; } diff --git a/tests/Composer/Test/Fixtures/installer/suggest-installed.test b/tests/Composer/Test/Fixtures/installer/suggest-installed.test index b84f0a252..f46102d0a 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-installed.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-installed.test @@ -1,5 +1,5 @@ --TEST-- -Suggestions are not displayed for installed packages if they are also installed +Suggestions are not displayed for installed packages --COMPOSER-- { "repositories": [ diff --git a/tests/Composer/Test/Fixtures/installer/suggest-replaced.test b/tests/Composer/Test/Fixtures/installer/suggest-replaced.test index 3691e6070..d1e8f6102 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-replaced.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-replaced.test @@ -1,5 +1,5 @@ --TEST-- -Suggestions are not displayed for installed packages if they are replaced +Suggestions are not displayed for packages if they are replaced --COMPOSER-- { "repositories": [ diff --git a/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test b/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test index a4ad14fd7..d2ea37766 100644 --- a/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test +++ b/tests/Composer/Test/Fixtures/installer/suggest-uninstalled.test @@ -1,5 +1,5 @@ --TEST-- -Suggestions are displayed for installed packages +Suggestions are displayed --COMPOSER-- { "repositories": [