From 611d6a036403f5f8511a36953dc9d9b66358e6ed Mon Sep 17 00:00:00 2001 From: Rob Bast Date: Mon, 21 Nov 2016 10:00:05 +0100 Subject: [PATCH 1/3] use array as default value rather than assigning empty array in constructor. the latter can cause issues with mocks in some scenarios (if constructor does not get called) --- src/Composer/Package/BasePackage.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Composer/Package/BasePackage.php b/src/Composer/Package/BasePackage.php index 3dd390aa0..6106f8dda 100644 --- a/src/Composer/Package/BasePackage.php +++ b/src/Composer/Package/BasePackage.php @@ -49,12 +49,14 @@ abstract class BasePackage implements PackageInterface * @var int */ public $id; - + /** @var string */ protected $name; + /** @var string */ protected $prettyName; - + /** @var RepositoryInterface */ protected $repository; - protected $transportOptions; + /** @var array */ + protected $transportOptions = array(); /** * All descendants' constructors should call this parent constructor @@ -66,7 +68,6 @@ abstract class BasePackage implements PackageInterface $this->prettyName = $name; $this->name = strtolower($name); $this->id = -1; - $this->transportOptions = array(); } /** From 7d67da3ffa8710a269fb17323b07e90cd9e2f7c3 Mon Sep 17 00:00:00 2001 From: Rob Bast Date: Mon, 21 Nov 2016 10:45:26 +0100 Subject: [PATCH 2/3] guard against non array value --- src/Composer/Package/Dumper/ArrayDumper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Composer/Package/Dumper/ArrayDumper.php b/src/Composer/Package/Dumper/ArrayDumper.php index faea5666a..0ff02ff41 100644 --- a/src/Composer/Package/Dumper/ArrayDumper.php +++ b/src/Composer/Package/Dumper/ArrayDumper.php @@ -118,7 +118,7 @@ class ArrayDumper } } - if (count($package->getTransportOptions()) > 0) { + if (is_array($package->getTransportOptions()) && count($package->getTransportOptions()) > 0) { $data['transport-options'] = $package->getTransportOptions(); } From 873f17261c72d0331a32b58042c6c5068e7e3cde Mon Sep 17 00:00:00 2001 From: Rob Bast Date: Fri, 25 Nov 2016 08:28:49 +0100 Subject: [PATCH 3/3] try to fix test instead of guarding implementation --- src/Composer/Package/Dumper/ArrayDumper.php | 2 +- .../Test/Package/Dumper/ArrayDumperTest.php | 14 ++++++++++++-- tests/Composer/Test/Package/LockerTest.php | 7 +++++++ .../Test/Package/Version/VersionSelectorTest.php | 16 ++++++++++++---- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/Composer/Package/Dumper/ArrayDumper.php b/src/Composer/Package/Dumper/ArrayDumper.php index 0ff02ff41..faea5666a 100644 --- a/src/Composer/Package/Dumper/ArrayDumper.php +++ b/src/Composer/Package/Dumper/ArrayDumper.php @@ -118,7 +118,7 @@ class ArrayDumper } } - if (is_array($package->getTransportOptions()) && count($package->getTransportOptions()) > 0) { + if (count($package->getTransportOptions()) > 0) { $data['transport-options'] = $package->getTransportOptions(); } diff --git a/tests/Composer/Test/Package/Dumper/ArrayDumperTest.php b/tests/Composer/Test/Package/Dumper/ArrayDumperTest.php index bf4eef6b7..fa841a391 100644 --- a/tests/Composer/Test/Package/Dumper/ArrayDumperTest.php +++ b/tests/Composer/Test/Package/Dumper/ArrayDumperTest.php @@ -31,6 +31,7 @@ class ArrayDumperTest extends \PHPUnit_Framework_TestCase { $this->dumper = new ArrayDumper(); $this->package = $this->getMock('Composer\Package\CompletePackageInterface'); + $this->packageExpects('getTransportOptions', array()); } public function testRequiredInformation() @@ -38,7 +39,8 @@ class ArrayDumperTest extends \PHPUnit_Framework_TestCase $this ->packageExpects('getPrettyName', 'foo') ->packageExpects('getPrettyVersion', '1.0') - ->packageExpects('getVersion', '1.0.0.0'); + ->packageExpects('getVersion', '1.0.0.0') + ; $config = $this->dumper->dump($this->package); $this->assertEquals( @@ -56,7 +58,9 @@ class ArrayDumperTest extends \PHPUnit_Framework_TestCase $this->package = $this->getMock('Composer\Package\RootPackageInterface'); $this - ->packageExpects('getMinimumStability', 'dev'); + ->packageExpects('getMinimumStability', 'dev') + ->packageExpects('getTransportOptions', array()) + ; $config = $this->dumper->dump($this->package); $this->assertSame('dev', $config['minimum-stability']); @@ -87,9 +91,15 @@ class ArrayDumperTest extends \PHPUnit_Framework_TestCase */ public function testKeys($key, $value, $method = null, $expectedValue = null) { + $this->package = $this->getMock('Composer\Package\RootPackageInterface'); + $this->packageExpects('get'.ucfirst($method ?: $key), $value); $this->packageExpects('isAbandoned', $value); + if ($method !== 'transportOptions') { + $this->packageExpects('getTransportOptions', array()); + } + $config = $this->dumper->dump($this->package); $this->assertSame($expectedValue ?: $value, $config[$key]); diff --git a/tests/Composer/Test/Package/LockerTest.php b/tests/Composer/Test/Package/LockerTest.php index 65dcbb62b..e7f91bb98 100644 --- a/tests/Composer/Test/Package/LockerTest.php +++ b/tests/Composer/Test/Package/LockerTest.php @@ -118,6 +118,13 @@ class LockerTest extends \PHPUnit_Framework_TestCase ->method('getVersion') ->will($this->returnValue('0.1.10.0')); + foreach (array($package1, $package2) as $package) { + $package + ->expects($this->atLeastOnce()) + ->method('getTransportOptions') + ->will($this->returnValue(array())); + } + $contentHash = md5(trim($jsonContent)); $json diff --git a/tests/Composer/Test/Package/Version/VersionSelectorTest.php b/tests/Composer/Test/Package/Version/VersionSelectorTest.php index c28ac3c17..85c9791fe 100644 --- a/tests/Composer/Test/Package/Version/VersionSelectorTest.php +++ b/tests/Composer/Test/Package/Version/VersionSelectorTest.php @@ -198,18 +198,26 @@ class VersionSelectorTest extends \PHPUnit_Framework_TestCase $versionParser = new VersionParser(); $package = $this->getMock('\Composer\Package\PackageInterface'); - $package->expects($this->any()) + $package + ->expects($this->any()) ->method('getPrettyVersion') ->will($this->returnValue($prettyVersion)); - $package->expects($this->any()) + $package + ->expects($this->any()) ->method('getVersion') ->will($this->returnValue($versionParser->normalize($prettyVersion))); - $package->expects($this->any()) + $package + ->expects($this->any()) ->method('isDev') ->will($this->returnValue($isDev)); - $package->expects($this->any()) + $package + ->expects($this->any()) ->method('getStability') ->will($this->returnValue($stability)); + $package + ->expects($this->any()) + ->method('getTransportOptions') + ->will($this->returnValue(array())); $branchAlias = $branchAlias === null ? array() : array('branch-alias' => array($prettyVersion => $branchAlias)); $package->expects($this->any())