From 0f7b078d6ccb555348ade0abe1a854e9bd53e841 Mon Sep 17 00:00:00 2001 From: Clark Stuth Date: Fri, 14 Mar 2014 14:45:31 -0500 Subject: [PATCH] added new dependency to Perforce object, updating some tests. --- .../Downloader/PerforceDownloader.php | 4 +- .../Repository/Vcs/PerforceDriver.php | 2 +- src/Composer/Util/Perforce.php | 21 ++- .../Downloader/PerforceDownloaderTest.php | 160 +++++++++++------- tests/Composer/Test/Util/PerforceTest.php | 11 +- 5 files changed, 122 insertions(+), 76 deletions(-) diff --git a/src/Composer/Downloader/PerforceDownloader.php b/src/Composer/Downloader/PerforceDownloader.php index 2bb1ba619..632ac648c 100644 --- a/src/Composer/Downloader/PerforceDownloader.php +++ b/src/Composer/Downloader/PerforceDownloader.php @@ -22,7 +22,7 @@ use Composer\Util\Perforce; class PerforceDownloader extends VcsDownloader { protected $perforce; - protected $perforceInjected = false; +// protected $perforceInjected = false; /** * {@inheritDoc} @@ -54,7 +54,7 @@ class PerforceDownloader extends VcsDownloader if ($repository instanceof VcsRepository) { $repoConfig = $this->getRepoConfig($repository); } - $this->perforce = Perforce::create($repoConfig, $package->getSourceUrl(), $path); + $this->perforce = Perforce::create($repoConfig, $package->getSourceUrl(), $path, $this->process, $this->io); } private function getRepoConfig(VcsRepository $repository) diff --git a/src/Composer/Repository/Vcs/PerforceDriver.php b/src/Composer/Repository/Vcs/PerforceDriver.php index 79500f1d6..8a1b40623 100644 --- a/src/Composer/Repository/Vcs/PerforceDriver.php +++ b/src/Composer/Repository/Vcs/PerforceDriver.php @@ -56,7 +56,7 @@ class PerforceDriver extends VcsDriver } $repoDir = $this->config->get('cache-vcs-dir') . '/' . $this->depot; - $this->perforce = Perforce::create($repoConfig, $this->getUrl(), $repoDir, $this->process); + $this->perforce = Perforce::create($repoConfig, $this->getUrl(), $repoDir, $this->process, $this->io); } /** diff --git a/src/Composer/Util/Perforce.php b/src/Composer/Util/Perforce.php index 94b9730eb..c3b66329f 100644 --- a/src/Composer/Util/Perforce.php +++ b/src/Composer/Util/Perforce.php @@ -35,27 +35,36 @@ class Perforce protected $windowsFlag; protected $commandResult; - public function __construct($repoConfig, $port, $path, ProcessExecutor $process, $isWindows) + protected $io; + + public function __construct($repoConfig, $port, $path, ProcessExecutor $process, $isWindows, IOInterface $io) { $this->windowsFlag = $isWindows; $this->p4Port = $port; $this->initializePath($path); $this->process = $process; $this->initialize($repoConfig); + $this->io = $io; } - public static function create($repoConfig, $port, $path, ProcessExecutor $process = null) + public static function create($repoConfig, $port, $path, ProcessExecutor $process = null, IOInterface $io) { if (!isset($process)) { $process = new ProcessExecutor; } $isWindows = defined('PHP_WINDOWS_VERSION_BUILD'); - $perforce = new Perforce($repoConfig, $port, $path, $process, $isWindows); + $perforce = new Perforce($repoConfig, $port, $path, $process, $isWindows, $io); return $perforce; } + public static function checkServerExists($url, ProcessExecutor $processExecutor) + { + $output = null; + return 0 === $processExecutor->execute('p4 -p ' . $url . ' info -s', $output); + } + public function initialize($repoConfig) { $this->uniquePerforceClientName = $this->generateUniquePerforceClientName(); @@ -382,12 +391,6 @@ class Perforce } } - public static function checkServerExists($url, ProcessExecutor $processExecutor) - { - $output = null; - return 0 === $processExecutor->execute('p4 -p ' . $url . ' info -s', $output); - } - public function getComposerInformation($identifier) { $index = strpos($identifier, '@'); diff --git a/tests/Composer/Test/Downloader/PerforceDownloaderTest.php b/tests/Composer/Test/Downloader/PerforceDownloaderTest.php index 8a20f67cc..0eb981362 100644 --- a/tests/Composer/Test/Downloader/PerforceDownloaderTest.php +++ b/tests/Composer/Test/Downloader/PerforceDownloaderTest.php @@ -15,86 +15,120 @@ namespace Composer\Test\Downloader; use Composer\Downloader\PerforceDownloader; use Composer\Config; use Composer\Repository\VcsRepository; +use Composer\IO\IOInterface; /** * @author Matt Whittom */ class PerforceDownloaderTest extends \PHPUnit_Framework_TestCase { - private $io; - private $config; - private $testPath; - public static $repository; + + protected $config; + protected $downloader; + protected $io; + protected $package; + protected $processExecutor; + protected $repoConfig; + protected $repository; + protected $testPath; - protected function setUp() + public function setUp() { - $this->testPath = sys_get_temp_dir() . '/composer-test'; - $this->config = new Config(); - $this->config->merge( - array( - 'config' => array( - 'home' => $this->testPath, - ), - ) - ); - $this->io = $this->getMock('Composer\IO\IOInterface'); + $this->testPath = sys_get_temp_dir() . '/composer-test'; + $this->repoConfig = $this->getRepoConfig(); + $this->config = $this->getConfig(); + $this->io = $this->getMockIoInterface(); + $this->processExecutor = $this->getMockProcessExecutor(); + $this->repository = $this->getMockRepository($this->repoConfig, $this->io, $this->config); + $this->package = $this->getMockPackageInterface($this->repository); + $this->downloader = new PerforceDownloader($this->io, $this->config, $this->processExecutor); } - public function testInitPerforceGetRepoConfig() + public function tearDown() + { + $this->downloader = null; + $this->package = null; + $this->repository = null; + $this->io = null; + $this->config = null; + $this->repoConfig = null; + $this->testPath = null; + } + + protected function getMockProcessExecutor() + { + return $this->getMock('Composer\Util\ProcessExecutor'); + } + + protected function getConfig() + { + $config = new Config(); + $settings = array('config' => array('home' => $this->testPath)); + $config->merge($settings); + return $config; + } + + protected function getMockIoInterface() + { + return $this->getMock('Composer\IO\IOInterface'); + } + + protected function getMockPackageInterface(VcsRepository $repository) { - $downloader = new PerforceDownloader($this->io, $this->config); $package = $this->getMock('Composer\Package\PackageInterface'); - $repoConfig = array('url' => 'TEST_URL', 'p4user' => 'TEST_USER'); - $repository = $this->getMock( - 'Composer\Repository\VcsRepository', - array('getRepoConfig'), - array($repoConfig, $this->io, $this->config) - ); - $package->expects($this->at(0)) - ->method('getRepository') - ->will($this->returnValue($repository)); - $repository->expects($this->at(0)) - ->method('getRepoConfig'); - $path = $this->testPath; - $downloader->initPerforce($package, $path, 'SOURCE_REF'); + $package->expects($this->any())->method('getRepository')->will($this->returnValue($repository)); + return $package; } + protected function getRepoConfig() + { + return array('url' => 'TEST_URL', 'p4user' => 'TEST_USER'); + } + + protected function getMockRepository(array $repoConfig, IOInterface $io, Config $config) + { + $class = 'Composer\Repository\VcsRepository'; + $methods = array('getRepoConfig'); + $args = array($repoConfig, $io, $config); + $repository = $this->getMock($class, $methods, $args); + $repository->expects($this->any())->method('getRepoConfig')->will($this->returnValue($repoConfig)); + return $repository; + } + + public function testInitPerforceInstantiatesANewPerforceObject() + { + $this->downloader->initPerforce($this->package, $this->testPath, 'SOURCE_REF'); + } + + public function testInitPerforceDoesNothingIfPerforceAlreadySet() + { + $perforce = $this->getMockBuilder('Composer\Util\Perforce')->disableOriginalConstructor()->getMock(); + $this->downloader->setPerforce($perforce); + $this->repository->expects($this->never())->method('getRepoConfig'); + $this->downloader->initPerforce($this->package, $this->testPath, 'SOURCE_REF'); + } + + /** + * @depends testInitPerforceInstantiatesANewPerforceObject + * @depends testInitPerforceDoesNothingIfPerforceAlreadySet + */ public function testDoDownload() { - $downloader = new PerforceDownloader($this->io, $this->config); - $repoConfig = array('depot' => 'TEST_DEPOT', 'branch' => 'TEST_BRANCH', 'p4user' => 'TEST_USER'); - $port = 'TEST_PORT'; - $path = 'TEST_PATH'; - $process = $this->getmock('Composer\Util\ProcessExecutor'); - $perforce = $this->getMock( - 'Composer\Util\Perforce', - array('setStream', 'queryP4User', 'writeP4ClientSpec', 'connectClient', 'syncCodeBase'), - array($repoConfig, $port, $path, $process, true, 'TEST') - ); + //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'; $label = 'LABEL'; - $perforce->expects($this->at(0)) - ->method('setStream') - ->with($this->equalTo($ref)); - $perforce->expects($this->at(1)) - ->method('queryP4User') - ->with($this->io); - $perforce->expects($this->at(2)) - ->method('writeP4ClientSpec'); - $perforce->expects($this->at(3)) - ->method('connectClient'); - $perforce->expects($this->at(4)) - ->method('syncCodeBase') - ->with($this->equalTo($label)); - $downloader->setPerforce($perforce); - $package = $this->getMock('Composer\Package\PackageInterface'); - $package->expects($this->at(0)) - ->method('getSourceReference') - ->will($this->returnValue($ref)); - $package->expects($this->at(1)) - ->method('getPrettyVersion') - ->will($this->returnValue($label)); - $path = $this->testPath; - $downloader->doDownload($package, $path); + $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(); + $perforce->expects($this->at(0))->method('setStream')->with($this->equalTo($ref)); + $perforce->expects($this->at(1))->method('p4Login')->with($this->identicalTo($this->io)); + $perforce->expects($this->at(2))->method('writeP4ClientSpec'); + $perforce->expects($this->at(3))->method('connectClient'); + $perforce->expects($this->at(4))->method('syncCodeBase'); + $perforce->expects($this->at(5))->method('cleanupClientSpec'); + $this->downloader->setPerforce($perforce); + $this->downloader->doDownload($this->package, $this->testPath); } } diff --git a/tests/Composer/Test/Util/PerforceTest.php b/tests/Composer/Test/Util/PerforceTest.php index f2eeb1964..56a234536 100644 --- a/tests/Composer/Test/Util/PerforceTest.php +++ b/tests/Composer/Test/Util/PerforceTest.php @@ -22,6 +22,7 @@ class PerforceTest extends \PHPUnit_Framework_TestCase { protected $perforce; protected $processExecutor; + protected $io; public function setUp() { @@ -32,7 +33,15 @@ class PerforceTest extends \PHPUnit_Framework_TestCase 'p4user' => 'user', 'unique_perforce_client_name' => 'TEST' ); - $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, true); + $io = $this->getMock('Composer\IO\IOInterface'); + $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, true, $io); + } + + public function tearDown() + { + $this->perforce = null; + $this->io = null; + $this->processExecutor = null; } public function testGetClientWithoutStream()