From dd1fd0e306dbad073fa8b32c0b23eea52ada8dea Mon Sep 17 00:00:00 2001 From: Clark Stuth Date: Mon, 24 Mar 2014 15:19:35 -0500 Subject: [PATCH 1/2] fixed perforce to reference labels instead of invalid tags --- .../Downloader/PerforceDownloader.php | 12 ++++++- src/Composer/Util/Perforce.php | 8 ++--- .../Downloader/PerforceDownloaderTest.php | 31 ++++++++++++++++--- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/src/Composer/Downloader/PerforceDownloader.php b/src/Composer/Downloader/PerforceDownloader.php index c2def4ee2..8892e4b74 100644 --- a/src/Composer/Downloader/PerforceDownloader.php +++ b/src/Composer/Downloader/PerforceDownloader.php @@ -29,7 +29,7 @@ class PerforceDownloader extends VcsDownloader public function doDownload(PackageInterface $package, $path) { $ref = $package->getSourceReference(); - $label = $package->getPrettyVersion(); + $label = $this->getLabelFromSourceReference($ref); $this->io->write(' Cloning ' . $ref); $this->initPerforce($package, $path); @@ -41,6 +41,16 @@ class PerforceDownloader extends VcsDownloader $this->perforce->cleanupClientSpec(); } + private function getLabelFromSourceReference($ref) + { + $pos = strpos($ref,'@'); + if (false !== $pos) + { + return substr($ref, $pos + 1); + } + return null; + } + public function initPerforce($package, $path) { if (!empty($this->perforce)) { diff --git a/src/Composer/Util/Perforce.php b/src/Composer/Util/Perforce.php index ae6f44d04..7801f966b 100644 --- a/src/Composer/Util/Perforce.php +++ b/src/Composer/Util/Perforce.php @@ -309,15 +309,15 @@ class Perforce $this->executeCommand($p4CreateClientCommand); } - public function syncCodeBase($label) + public function syncCodeBase($sourceReference) { $prevDir = getcwd(); chdir($this->path); - $p4SyncCommand = $this->generateP4Command('sync -f '); - $p4SyncCommand = $p4SyncCommand . '@' . $label; + if (null != $sourceReference) { + $p4SyncCommand = $p4SyncCommand . '@' . $sourceReference; + } $this->executeCommand($p4SyncCommand); - chdir($prevDir); } diff --git a/tests/Composer/Test/Downloader/PerforceDownloaderTest.php b/tests/Composer/Test/Downloader/PerforceDownloaderTest.php index c78f1ed13..da93db767 100644 --- a/tests/Composer/Test/Downloader/PerforceDownloaderTest.php +++ b/tests/Composer/Test/Downloader/PerforceDownloaderTest.php @@ -112,13 +112,35 @@ class PerforceDownloaderTest extends \PHPUnit_Framework_TestCase * @depends testInitPerforceInstantiatesANewPerforceObject * @depends testInitPerforceDoesNothingIfPerforceAlreadySet */ - public function testDoDownload() + public function testDoDownloadWithTag() { //I really don't like this test but the logic of each Perforce method is tested in the Perforce class. Really I am just enforcing workflow. + $ref = 'SOURCE_REF@123'; + $label = 123; + $this->package->expects($this->once())->method('getSourceReference')->will($this->returnValue($ref)); + $this->io->expects($this->once())->method('write')->with($this->stringContains('Cloning '.$ref)); + $perforceMethods = array('setStream', 'p4Login', 'writeP4ClientSpec', 'connectClient', 'syncCodeBase', 'cleanupClientSpec'); + $perforce = $this->getMockBuilder('Composer\Util\Perforce', $perforceMethods)->disableOriginalConstructor()->getMock(); + $perforce->expects($this->at(0))->method('initializePath')->with($this->equalTo($this->testPath)); + $perforce->expects($this->at(1))->method('setStream')->with($this->equalTo($ref)); + $perforce->expects($this->at(2))->method('p4Login')->with($this->identicalTo($this->io)); + $perforce->expects($this->at(3))->method('writeP4ClientSpec'); + $perforce->expects($this->at(4))->method('connectClient'); + $perforce->expects($this->at(5))->method('syncCodeBase')->with($label); + $perforce->expects($this->at(6))->method('cleanupClientSpec'); + $this->downloader->setPerforce($perforce); + $this->downloader->doDownload($this->package, $this->testPath); + } + + /** + * @depends testInitPerforceInstantiatesANewPerforceObject + * @depends testInitPerforceDoesNothingIfPerforceAlreadySet + */ + public function testDoDownloadWithNoTag() + { $ref = 'SOURCE_REF'; - $label = 'LABEL'; + $label = null; $this->package->expects($this->once())->method('getSourceReference')->will($this->returnValue($ref)); - $this->package->expects($this->once())->method('getPrettyVersion')->will($this->returnValue($label)); $this->io->expects($this->once())->method('write')->with($this->stringContains('Cloning '.$ref)); $perforceMethods = array('setStream', 'p4Login', 'writeP4ClientSpec', 'connectClient', 'syncCodeBase', 'cleanupClientSpec'); $perforce = $this->getMockBuilder('Composer\Util\Perforce', $perforceMethods)->disableOriginalConstructor()->getMock(); @@ -127,9 +149,10 @@ class PerforceDownloaderTest extends \PHPUnit_Framework_TestCase $perforce->expects($this->at(2))->method('p4Login')->with($this->identicalTo($this->io)); $perforce->expects($this->at(3))->method('writeP4ClientSpec'); $perforce->expects($this->at(4))->method('connectClient'); - $perforce->expects($this->at(5))->method('syncCodeBase'); + $perforce->expects($this->at(5))->method('syncCodeBase')->with($label); $perforce->expects($this->at(6))->method('cleanupClientSpec'); $this->downloader->setPerforce($perforce); $this->downloader->doDownload($this->package, $this->testPath); } + } From a12c4e2a1722730c199b32e90b3be5240f2f4698 Mon Sep 17 00:00:00 2001 From: Clark Stuth Date: Tue, 25 Mar 2014 08:30:44 -0500 Subject: [PATCH 2/2] Removed getWindowsFlag and setWindowsFlag methods from Perforce object. --- .../Repository/Vcs/PerforceDriver.php | 11 --------- src/Composer/Util/Perforce.php | 10 -------- .../Downloader/PerforceDownloaderTest.php | 4 ++-- .../Repository/Vcs/PerforceDriverTest.php | 20 +++++++++------- tests/Composer/Test/Util/PerforceTest.php | 23 +++++++++++-------- 5 files changed, 28 insertions(+), 40 deletions(-) diff --git a/src/Composer/Repository/Vcs/PerforceDriver.php b/src/Composer/Repository/Vcs/PerforceDriver.php index 9599dd964..e928e5835 100644 --- a/src/Composer/Repository/Vcs/PerforceDriver.php +++ b/src/Composer/Repository/Vcs/PerforceDriver.php @@ -140,7 +140,6 @@ class PerforceDriver extends VcsDriver { $this->composerInfo = $this->perforce->getComposerInformation('//' . $this->depot . '/' . $identifier); $this->composerInfoIdentifier = $identifier; - $result = false; return !empty($this->composerInfo); } @@ -183,14 +182,4 @@ class PerforceDriver extends VcsDriver return $this->branch; } - public function setPerforce(Perforce $perforce) - { - $this->perforce = $perforce; - } - - public function getPerforce() - { - return $this->perforce; - } - } diff --git a/src/Composer/Util/Perforce.php b/src/Composer/Util/Perforce.php index 7801f966b..e812fbce3 100644 --- a/src/Composer/Util/Perforce.php +++ b/src/Composer/Util/Perforce.php @@ -370,16 +370,6 @@ class Perforce return; } - public function getWindowsFlag() - { - return $this->windowsFlag; - } - - public function setWindowsFlag($flag) - { - $this->windowsFlag = $flag; - } - public function windowsLogin($password) { $command = $this->generateP4Command(' login -a'); diff --git a/tests/Composer/Test/Downloader/PerforceDownloaderTest.php b/tests/Composer/Test/Downloader/PerforceDownloaderTest.php index da93db767..7562f7fca 100644 --- a/tests/Composer/Test/Downloader/PerforceDownloaderTest.php +++ b/tests/Composer/Test/Downloader/PerforceDownloaderTest.php @@ -32,7 +32,7 @@ class PerforceDownloaderTest extends \PHPUnit_Framework_TestCase protected $repository; protected $testPath; - public function setUp() + protected function setUp() { $this->testPath = sys_get_temp_dir() . '/composer-test'; $this->repoConfig = $this->getRepoConfig(); @@ -44,7 +44,7 @@ class PerforceDownloaderTest extends \PHPUnit_Framework_TestCase $this->downloader = new PerforceDownloader($this->io, $this->config, $this->processExecutor); } - public function tearDown() + protected function tearDown() { $this->downloader = null; $this->package = null; diff --git a/tests/Composer/Test/Repository/Vcs/PerforceDriverTest.php b/tests/Composer/Test/Repository/Vcs/PerforceDriverTest.php index 9afc57d30..dcb50244a 100644 --- a/tests/Composer/Test/Repository/Vcs/PerforceDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/PerforceDriverTest.php @@ -15,6 +15,7 @@ namespace Composer\Test\Repository\Vcs; use Composer\Repository\Vcs\PerforceDriver; use Composer\Util\Filesystem; use Composer\Config; +use Composer\Util\Perforce; /** * @author Matt Whittom @@ -33,7 +34,7 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase const TEST_DEPOT = 'TEST_DEPOT_CONFIG'; const TEST_BRANCH = 'TEST_BRANCH_CONFIG'; - public function setUp() + protected function setUp() { $this->testPath = sys_get_temp_dir() . '/composer-test'; $this->config = $this->getTestConfig($this->testPath); @@ -43,9 +44,10 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase $this->remoteFileSystem = $this->getMockRemoteFilesystem(); $this->perforce = $this->getMockPerforce(); $this->driver = new PerforceDriver($this->repoConfig, $this->io, $this->config, $this->process, $this->remoteFileSystem); + $this->overrideDriverInternalPerforce($this->perforce); } - public function tearDown() + protected function tearDown() { //cleanup directory under test path $fs = new Filesystem; @@ -60,6 +62,14 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase $this->testPath = null; } + protected function overrideDriverInternalPerforce(Perforce $perforce) + { + $reflectionClass = new \ReflectionClass($this->driver); + $property = $reflectionClass->getProperty('perforce'); + $property->setAccessible(true); + $property->setValue($this->driver, $perforce); + } + protected function getTestConfig($testPath) { $config = new Config(); @@ -100,7 +110,6 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase public function testInitializeCapturesVariablesFromRepoConfig() { $driver = new PerforceDriver($this->repoConfig, $this->io, $this->config, $this->process, $this->remoteFileSystem); - $driver->setPerforce($this->perforce); $driver->initialize(); $this->assertEquals(self::TEST_URL, $driver->getUrl()); $this->assertEquals(self::TEST_DEPOT, $driver->getDepot()); @@ -109,7 +118,6 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase public function testInitializeLogsInAndConnectsClient() { - $this->driver->setPerforce($this->perforce); $this->perforce->expects($this->at(0))->method('p4Login')->with($this->identicalTo($this->io)); $this->perforce->expects($this->at(1))->method('checkStream')->with($this->equalTo(self::TEST_DEPOT)); $this->perforce->expects($this->at(2))->method('writeP4ClientSpec'); @@ -125,7 +133,6 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase { $identifier = 'TEST_IDENTIFIER'; $formatted_depot_path = '//' . self::TEST_DEPOT . '/' . $identifier; - $this->driver->setPerforce($this->perforce); $this->perforce->expects($this->any())->method('getComposerInformation')->with($this->equalTo($formatted_depot_path))->will($this->returnValue(array())); $this->driver->initialize(); $result = $this->driver->hasComposerFile($identifier); @@ -140,7 +147,6 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase { $identifier = 'TEST_IDENTIFIER'; $formatted_depot_path = '//' . self::TEST_DEPOT . '/' . $identifier; - $this->driver->setPerforce($this->perforce); $this->perforce->expects($this->any())->method('getComposerInformation')->with($this->equalTo($formatted_depot_path))->will($this->returnValue(array(''))); $this->driver->initialize(); $result = $this->driver->hasComposerFile($identifier); @@ -163,9 +169,7 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase public function testCleanup() { $this->perforce->expects($this->once())->method('cleanupClientSpec'); - $this->driver->setPerforce($this->perforce); $this->driver->cleanup(); - $this->assertNull($this->driver->getPerforce()); } } diff --git a/tests/Composer/Test/Util/PerforceTest.php b/tests/Composer/Test/Util/PerforceTest.php index 1b8063258..a5f3e60fa 100644 --- a/tests/Composer/Test/Util/PerforceTest.php +++ b/tests/Composer/Test/Util/PerforceTest.php @@ -31,15 +31,15 @@ class PerforceTest extends \PHPUnit_Framework_TestCase const TEST_PORT = 'port'; const TEST_PATH = 'path'; - public function setUp() + protected function setUp() { $this->processExecutor = $this->getMock('Composer\Util\ProcessExecutor'); $this->repoConfig = $this->getTestRepoConfig(); $this->io = $this->getMockIOInterface(); - $this->perforce = new Perforce($this->repoConfig, self::TEST_PORT, self::TEST_PATH, $this->processExecutor, true, $this->io); + $this->createNewPerforceWithWindowsFlag(true); } - public function tearDown() + protected function tearDown() { $this->perforce = null; $this->io = null; @@ -62,6 +62,11 @@ class PerforceTest extends \PHPUnit_Framework_TestCase return $this->getMock('Composer\IO\IOInterface'); } + protected function createNewPerforceWithWindowsFlag($flag) + { + $this->perforce = new Perforce($this->repoConfig, self::TEST_PORT, self::TEST_PATH, $this->processExecutor, $flag, $this->io); + } + public function testGetClientWithoutStream() { $client = $this->perforce->getClient(); @@ -131,8 +136,8 @@ class PerforceTest extends \PHPUnit_Framework_TestCase public function testQueryP4UserWithUserSetInP4VariablesWithWindowsOS() { + $this->createNewPerforceWithWindowsFlag(true); $this->perforce->setUser(null); - $this->perforce->setWindowsFlag(true); $expectedCommand = 'p4 set'; $callback = function($command, &$output) { @@ -149,8 +154,8 @@ class PerforceTest extends \PHPUnit_Framework_TestCase public function testQueryP4UserWithUserSetInP4VariablesNotWindowsOS() { + $this->createNewPerforceWithWindowsFlag(false); $this->perforce->setUser(null); - $this->perforce->setWindowsFlag(false); $expectedCommand = 'echo $P4USER'; $callback = function($command, &$output) { @@ -179,8 +184,8 @@ class PerforceTest extends \PHPUnit_Framework_TestCase public function testQueryP4UserStoresResponseToQueryForUserWithWindows() { + $this->createNewPerforceWithWindowsFlag(true); $this->perforce->setUser(null); - $this->perforce->setWindowsFlag(true); $expectedQuestion = 'Enter P4 User:'; $expectedCommand = 'p4 set P4USER=TEST_QUERY_USER'; $this->io->expects($this->at(0)) @@ -196,8 +201,8 @@ class PerforceTest extends \PHPUnit_Framework_TestCase public function testQueryP4UserStoresResponseToQueryForUserWithoutWindows() { + $this->createNewPerforceWithWindowsFlag(false); $this->perforce->setUser(null); - $this->perforce->setWindowsFlag(false); $expectedQuestion = 'Enter P4 User:'; $expectedCommand = 'export P4USER=TEST_QUERY_USER'; $this->io->expects($this->at(0)) @@ -226,7 +231,7 @@ class PerforceTest extends \PHPUnit_Framework_TestCase public function testQueryP4PasswordWithPasswordSetInP4VariablesWithWindowsOS() { - $this->perforce->setWindowsFlag(true); + $this->createNewPerforceWithWindowsFlag(true); $expectedCommand = 'p4 set'; $callback = function($command, &$output) { @@ -243,7 +248,7 @@ class PerforceTest extends \PHPUnit_Framework_TestCase public function testQueryP4PasswordWithPasswordSetInP4VariablesNotWindowsOS() { - $this->perforce->setWindowsFlag(false); + $this->createNewPerforceWithWindowsFlag(false); $expectedCommand = 'echo $P4PASSWD'; $callback = function($command, &$output) {