Merge pull request #2834 from vuhl/master

Fixing perforce driver/downloader bugs

src/Composer/Downloader/PerforceDownloader.php

@@ -22,7 +22,6 @@ use Composer\Util\Perforce;
 class PerforceDownloader extends VcsDownloader
 {
     protected $perforce;
-    protected $perforceInjected = false;
 
     /**
      * {@inheritDoc}
@@ -30,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);
@@ -42,9 +41,19 @@ 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 ($this->perforce) {
+        if (!empty($this->perforce)) {
             $this->perforce->initializePath($path);
             return;
         }
@@ -54,7 +63,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)

src/Composer/Repository/Vcs/PerforceDriver.php

@@ -35,7 +35,7 @@ class PerforceDriver extends VcsDriver
     {
         $this->depot = $this->repoConfig['depot'];
         $this->branch = '';
-        if (isset($this->repoConfig['branch'])) {
+        if (!empty($this->repoConfig['branch'])) {
             $this->branch = $this->repoConfig['branch'];
         }
 
@@ -51,12 +51,12 @@ class PerforceDriver extends VcsDriver
 
     private function initPerforce($repoConfig)
     {
-        if (isset($this->perforce)) {
+        if (!empty($this->perforce)) {
             return;
         }
 
         $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);
     }
 
     /**
@@ -64,7 +64,7 @@ class PerforceDriver extends VcsDriver
      */
     public function getComposerInformation($identifier)
     {
-        if (isset($this->composerInfoIdentifier)) {
+        if (!empty($this->composerInfoIdentifier)) {
             if (strcmp($identifier, $this->composerInfoIdentifier) === 0) {
                 return $this->composerInfo;
             }
@@ -140,12 +140,7 @@ class PerforceDriver extends VcsDriver
     {
         $this->composerInfo = $this->perforce->getComposerInformation('//' . $this->depot . '/' . $identifier);
         $this->composerInfoIdentifier = $identifier;
-        $result = false;
+        return !empty($this->composerInfo);
-        if (isset($this->composerInfo)) {
-            $result = count($this->composerInfo) > 0;
-        }
-
-        return $result;
     }
 
     /**
@@ -187,8 +182,4 @@ class PerforceDriver extends VcsDriver
         return $this->branch;
     }
 
-    public function setPerforce(Perforce $perforce)
-    {
-        $this->perforce = $perforce;
-    }
 }

src/Composer/Util/Perforce.php

@@ -35,27 +35,33 @@ class Perforce
     protected $windowsFlag;
     protected $commandResult;
 
-    public function __construct($repoConfig, $port, $path, ProcessExecutor $process, $isWindows)
+    protected $io;
+
+    protected $filesystem;
+
+    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, IOInterface $io)
     {
-        if (!isset($process)) {
-            $process = new ProcessExecutor;
-        }
         $isWindows = defined('PHP_WINDOWS_VERSION_BUILD');
-
+        $perforce = new Perforce($repoConfig, $port, $path, $process, $isWindows, $io);
-        $perforce = new Perforce($repoConfig, $port, $path, $process, $isWindows);
-
         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();
@@ -100,10 +106,12 @@ class Perforce
     public function cleanupClientSpec()
     {
         $client = $this->getClient();
-        $command = 'p4 client -d $client';
+        $task = 'client -d ' . $client;
+        $useP4Client = false;
+        $command = $this->generateP4Command($task, $useP4Client);
         $this->executeCommand($command);
         $clientSpec = $this->getP4ClientSpec();
-        $fileSystem = new FileSystem($this->process);
+        $fileSystem = $this->getFilesystem();
         $fileSystem->remove($clientSpec);
     }
 
@@ -132,7 +140,7 @@ class Perforce
     public function initializePath($path)
     {
         $this->path = $path;
-        $fs = new Filesystem();
+        $fs = $this->getFilesystem();
         $fs->ensureDirectoryExists($path);
     }
 
@@ -191,7 +199,12 @@ class Perforce
         return $this->p4User;
     }
 
-    public function queryP4User(IOInterface $io)
+    public function setUser($user)
+    {
+        $this->p4User = $user;
+    }
+
+    public function queryP4User()
     {
         $this->getUser();
         if (strlen($this->p4User) > 0) {
@@ -201,7 +214,7 @@ class Perforce
         if (strlen($this->p4User) > 0) {
             return;
         }
-        $this->p4User = $io->ask('Enter P4 User:');
+        $this->p4User = $this->io->ask('Enter P4 User:');
         if ($this->windowsFlag) {
             $command = 'p4 set P4USER=' . $this->p4User;
         } else {
@@ -239,14 +252,14 @@ class Perforce
         }
     }
 
-    public function queryP4Password(IOInterface $io)
+    public function queryP4Password()
     {
         if (isset($this->p4Password)) {
             return $this->p4Password;
         }
         $password = $this->getP4variable('P4PASSWD');
         if (strlen($password) <= 0) {
-            $password = $io->askAndHideAnswer('Enter password for Perforce user ' . $this->getUser() . ': ');
+            $password = $this->io->askAndHideAnswer('Enter password for Perforce user ' . $this->getUser() . ': ');
         }
         $this->p4Password = $password;
 
@@ -291,19 +304,15 @@ class Perforce
         $this->executeCommand($p4CreateClientCommand);
     }
 
-    public function syncCodeBase($label)
+    public function syncCodeBase($sourceReference)
     {
         $prevDir = getcwd();
         chdir($this->path);
-
         $p4SyncCommand = $this->generateP4Command('sync -f ');
-        if (isset($label)) {
+        if (null != $sourceReference) {
-            if (strcmp($label, 'dev-master') != 0) {
+            $p4SyncCommand = $p4SyncCommand . '@' . $sourceReference;
-                $p4SyncCommand = $p4SyncCommand . '@' . $label;
-            }
         }
         $this->executeCommand($p4SyncCommand);
-
         chdir($prevDir);
     }
 
@@ -364,11 +373,11 @@ class Perforce
         return $process->run();
     }
 
-    public function p4Login(IOInterface $io)
+    public function p4Login()
     {
-        $this->queryP4User($io);
+        $this->queryP4User();
         if (!$this->isLoggedIn()) {
-            $password = $this->queryP4Password($io);
+            $password = $this->queryP4Password();
             if ($this->windowsFlag) {
                 $this->windowsLogin($password);
             } else {
@@ -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, '@');
@@ -459,9 +462,15 @@ class Perforce
                 }
             }
         }
-        $branches = array();
+        $command = $this->generateP4Command('changes '. $this->getStream() . '/...', false);
-        $branches['master'] = $possibleBranches[$this->p4Branch];
+        $this->executeCommand($command);
+        $result = $this->commandResult;
+        $resArray = explode(PHP_EOL, $result);
+        $lastCommit = $resArray[0];
+        $lastCommitArr = explode(' ', $lastCommit);
+        $lastCommitNum = $lastCommitArr[1];
 
+        $branches = array('master' => $possibleBranches[$this->p4Branch] . '@'. $lastCommitNum);
         return $branches;
     }
 
@@ -479,7 +488,6 @@ class Perforce
                 $tags[$fields[1]] = $this->getStream() . '@' . $fields[1];
             }
         }
-
         return $tags;
     }
 
@@ -541,4 +549,20 @@ class Perforce
 
         return $result;
     }
+
+    public function getFilesystem()
+    {
+        if (empty($this->filesystem))
+        {
+            $this->filesystem = new Filesystem($this->process);
+        }
+        return $this->filesystem;
+    }
+
+
+    public function setFilesystem(Filesystem $fs)
+    {
+        $this->filesystem = $fs;
+    }
+
 }

tests/Composer/Test/Downloader/PerforceDownloaderTest.php

@@ -15,86 +15,144 @@ namespace Composer\Test\Downloader;
 use Composer\Downloader\PerforceDownloader;
 use Composer\Config;
 use Composer\Repository\VcsRepository;
+use Composer\IO\IOInterface;
 
 /**
  * @author Matt Whittom <Matt.Whittom@veteransunited.com>
  */
 class PerforceDownloaderTest extends \PHPUnit_Framework_TestCase
 {
-    private $io;
+    
-    private $config;
+    protected $config;
-    private $testPath;
+    protected $downloader;
-    public static $repository;
+    protected $io;
+    protected $package;
+    protected $processExecutor;
+    protected $repoConfig;
+    protected $repository;
+    protected $testPath;
 
     protected function setUp()
     {
-        $this->testPath = sys_get_temp_dir() . '/composer-test';
+        $this->testPath        = sys_get_temp_dir() . '/composer-test';
-        $this->config = new Config();
+        $this->repoConfig      = $this->getRepoConfig();
-        $this->config->merge(
+        $this->config          = $this->getConfig();
-            array(
+        $this->io              = $this->getMockIoInterface();
-                 'config' => array(
+        $this->processExecutor = $this->getMockProcessExecutor();
-                     'home' => $this->testPath,
+        $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);
-        );
-        $this->io = $this->getMock('Composer\IO\IOInterface');
     }
 
-    public function testInitPerforceGetRepoConfig()
+    protected 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');
+        $package->expects($this->any())->method('getRepository')->will($this->returnValue($repository));
-        $repository = $this->getMock(
+        return $package;
-            'Composer\Repository\VcsRepository',
+    }
-            array('getRepoConfig'),
+
-            array($repoConfig, $this->io, $this->config)
+    protected function getRepoConfig()
-        );
+    {
-        $package->expects($this->at(0))
+        return array('url' => 'TEST_URL', 'p4user' => 'TEST_USER');   
-            ->method('getRepository')
-            ->will($this->returnValue($repository));
-        $repository->expects($this->at(0))
-            ->method('getRepoConfig');
-        $path = $this->testPath;
-        $downloader->initPerforce($package, $path, 'SOURCE_REF');
     }
 
-    public function testDoDownload()
+    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 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()
     {
-        $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')
-        );
         $ref = 'SOURCE_REF';
-        $label = 'LABEL';
+        $label = null;
-        $perforce->expects($this->at(0))
+        $this->package->expects($this->once())->method('getSourceReference')->will($this->returnValue($ref));
-            ->method('setStream')
+        $this->io->expects($this->once())->method('write')->with($this->stringContains('Cloning '.$ref));
-            ->with($this->equalTo($ref));
+        $perforceMethods = array('setStream', 'p4Login', 'writeP4ClientSpec', 'connectClient', 'syncCodeBase', 'cleanupClientSpec');
-        $perforce->expects($this->at(1))
+        $perforce = $this->getMockBuilder('Composer\Util\Perforce', $perforceMethods)->disableOriginalConstructor()->getMock();
-            ->method('queryP4User')
+        $perforce->expects($this->at(0))->method('initializePath')->with($this->equalTo($this->testPath));
-            ->with($this->io);
+        $perforce->expects($this->at(1))->method('setStream')->with($this->equalTo($ref));
-        $perforce->expects($this->at(2))
+        $perforce->expects($this->at(2))->method('p4Login')->with($this->identicalTo($this->io));
-            ->method('writeP4ClientSpec');
+        $perforce->expects($this->at(3))->method('writeP4ClientSpec');
-        $perforce->expects($this->at(3))
+        $perforce->expects($this->at(4))->method('connectClient');
-            ->method('connectClient');
+        $perforce->expects($this->at(5))->method('syncCodeBase')->with($label);
-        $perforce->expects($this->at(4))
+        $perforce->expects($this->at(6))->method('cleanupClientSpec');
-            ->method('syncCodeBase')
+        $this->downloader->setPerforce($perforce);
-            ->with($this->equalTo($label));
+        $this->downloader->doDownload($this->package, $this->testPath);
-        $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);
     }
+
 }

tests/Composer/Test/Repository/Vcs/PerforceDriverTest.php

@@ -15,119 +15,141 @@ namespace Composer\Test\Repository\Vcs;
 use Composer\Repository\Vcs\PerforceDriver;
 use Composer\Util\Filesystem;
 use Composer\Config;
+use Composer\Util\Perforce;
 
 /**
  * @author Matt Whittom <Matt.Whittom@veteransunited.com>
  */
 class PerforceDriverTest extends \PHPUnit_Framework_TestCase
 {
-    private $config;
+    protected $config;
-    private $io;
+    protected $io;
-    private $process;
+    protected $process;
-    private $remoteFileSystem;
+    protected $remoteFileSystem;
-    private $testPath;
+    protected $testPath;
+    protected $driver;
+    protected $repoConfig;
 
-    public function setUp()
+    const TEST_URL    = 'TEST_PERFORCE_URL';
-    {
+    const TEST_DEPOT  = 'TEST_DEPOT_CONFIG';
-        $this->testPath = sys_get_temp_dir() . '/composer-test';
+    const TEST_BRANCH = 'TEST_BRANCH_CONFIG';
-        $this->config = new Config();
-        $this->config->merge(
-            array(
-                 'config' => array(
-                     'home' => $this->testPath,
-                 ),
-            )
-        );
 
-        $this->io = $this->getMock('Composer\IO\IOInterface');
+    protected function setUp()
-        $this->process = $this->getMock('Composer\Util\ProcessExecutor');
+    {
-        $this->remoteFileSystem = $this->getMockBuilder('Composer\Util\RemoteFilesystem')->disableOriginalConstructor()
+        $this->testPath         = sys_get_temp_dir() . '/composer-test';
-                                  ->getMock();
+        $this->config           = $this->getTestConfig($this->testPath);
+        $this->repoConfig       = $this->getTestRepoConfig();
+        $this->io               = $this->getMockIOInterface();
+        $this->process          = $this->getMockProcessExecutor();
+        $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;
         $fs->removeDirectory($this->testPath);
+        $this->driver           = null;
+        $this->perforce         = null;
+        $this->remoteFileSystem = null;
+        $this->process          = null;
+        $this->io               = null;
+        $this->repoConfig       = null;
+        $this->config           = null;
+        $this->testPath         = null;
     }
 
-    public function testInitializeCapturesVariablesFromRepoConfig()
+    protected function overrideDriverInternalPerforce(Perforce $perforce)
     {
-        $this->setUp();
+        $reflectionClass = new \ReflectionClass($this->driver);
-        $repoConfig = array(
+        $property = $reflectionClass->getProperty('perforce');
-            'url'    => 'TEST_PERFORCE_URL',
+        $property->setAccessible(true);
-            'depot'  => 'TEST_DEPOT_CONFIG',
+        $property->setValue($this->driver, $perforce);
-            'branch' => 'TEST_BRANCH_CONFIG'
+    }
-        );
+
-        $driver = new PerforceDriver($repoConfig, $this->io, $this->config, $this->process, $this->remoteFileSystem);
+    protected function getTestConfig($testPath)
-        $process = $this->getMock('Composer\Util\ProcessExecutor');
+    {
-        $arguments = array(
+        $config = new Config();
-            array('depot' => 'TEST_DEPOT', 'branch' => 'TEST_BRANCH'),
+        $config->merge(array('config'=>array('home'=>$testPath)));
-            'port' => 'TEST_PORT',
+        return $config;
-            'path' => $this->testPath,
+    }
-            $process,
+
-            true,
+    protected function getTestRepoConfig()
-            'TEST'
+    {
+        return array(
+            'url'    => self::TEST_URL,
+            'depot'  => self::TEST_DEPOT,
+            'branch' => self::TEST_BRANCH,
         );
-        $perforce = $this->getMock('Composer\Util\Perforce', null, $arguments);
+    }
-        $driver->setPerforce($perforce);
+
+    protected function getMockIOInterface()
+    {
+        return $this->getMock('Composer\IO\IOInterface');
+    }
+
+    protected function getMockProcessExecutor()
+    {
+        return $this->getMock('Composer\Util\ProcessExecutor');
+    }
+
+    protected function getMockRemoteFilesystem()
+    {
+        return $this->getMockBuilder('Composer\Util\RemoteFilesystem')->disableOriginalConstructor()->getMock();
+    }
+
+    protected function getMockPerforce()
+    {
+        $methods = array('p4login', 'checkStream', 'writeP4ClientSpec', 'connectClient', 'getComposerInformation', 'cleanupClientSpec');
+        return $this->getMockBuilder('Composer\Util\Perforce', $methods)->disableOriginalConstructor()->getMock();
+    }
+
+    public function testInitializeCapturesVariablesFromRepoConfig()
+    {
+        $driver = new PerforceDriver($this->repoConfig, $this->io, $this->config, $this->process, $this->remoteFileSystem);
         $driver->initialize();
-        $this->assertEquals('TEST_PERFORCE_URL', $driver->getUrl());
+        $this->assertEquals(self::TEST_URL, $driver->getUrl());
-        $this->assertEquals('TEST_DEPOT_CONFIG', $driver->getDepot());
+        $this->assertEquals(self::TEST_DEPOT, $driver->getDepot());
-        $this->assertEquals('TEST_BRANCH_CONFIG', $driver->getBranch());
+        $this->assertEquals(self::TEST_BRANCH, $driver->getBranch());
     }
 
     public function testInitializeLogsInAndConnectsClient()
     {
-        $this->setUp();
+        $this->perforce->expects($this->at(0))->method('p4Login')->with($this->identicalTo($this->io));
-        $repoConfig = array(
+        $this->perforce->expects($this->at(1))->method('checkStream')->with($this->equalTo(self::TEST_DEPOT));
-            'url'    => 'TEST_PERFORCE_URL',
+        $this->perforce->expects($this->at(2))->method('writeP4ClientSpec');
-            'depot'  => 'TEST_DEPOT_CONFIG',
+        $this->perforce->expects($this->at(3))->method('connectClient');
-            'branch' => 'TEST_BRANCH_CONFIG'
+        $this->driver->initialize();
-        );
-        $driver = new PerforceDriver($repoConfig, $this->io, $this->config, $this->process, $this->remoteFileSystem);
-        $perforce = $this->getMockBuilder('Composer\Util\Perforce')->disableOriginalConstructor()->getMock();
-        $perforce->expects($this->at(0))
-            ->method('p4Login')
-            ->with($this->io);
-        $perforce->expects($this->at(1))
-            ->method('checkStream')
-            ->with($this->equalTo('TEST_DEPOT_CONFIG'));
-        $perforce->expects($this->at(2))
-            ->method('writeP4ClientSpec');
-        $perforce->expects($this->at(3))
-            ->method('connectClient');
-
-        $driver->setPerforce($perforce);
-        $driver->initialize();
     }
 
-    public function testHasComposerFile()
+    /**
+     * @depends testInitializeCapturesVariablesFromRepoConfig
+     * @depends testInitializeLogsInAndConnectsClient
+     */
+    public function testHasComposerFileReturnsFalseOnNoComposerFile()
     {
-        $repoConfig = array(
-            'url'    => 'TEST_PERFORCE_URL',
-            'depot'  => 'TEST_DEPOT_CONFIG',
-            'branch' => 'TEST_BRANCH_CONFIG'
-        );
-        $driver = new PerforceDriver($repoConfig, $this->io, $this->config, $this->process, $this->remoteFileSystem);
-        $process = $this->getMock('Composer\Util\ProcessExecutor');
-        $arguments = array(
-            array('depot' => 'TEST_DEPOT', 'branch' => 'TEST_BRANCH'),
-            'port' => 'TEST_PORT',
-            'path' => $this->testPath,
-            $process,
-            true,
-            'TEST'
-        );
-        $perforce = $this->getMock('Composer\Util\Perforce', array('getComposerInformation'), $arguments);
-        $perforce->expects($this->at(0))
-            ->method('getComposerInformation')
-            ->with($this->equalTo('//TEST_DEPOT_CONFIG/TEST_IDENTIFIER'))
-            ->will($this->returnValue('Some json stuff'));
-        $driver->setPerforce($perforce);
-        $driver->initialize();
         $identifier = 'TEST_IDENTIFIER';
-        $result = $driver->hasComposerFile($identifier);
+        $formatted_depot_path = '//' . self::TEST_DEPOT . '/' . $identifier;
+        $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);
+        $this->assertFalse($result);
+    }
+
+    /**
+     * @depends testInitializeCapturesVariablesFromRepoConfig
+     * @depends testInitializeLogsInAndConnectsClient
+     */
+    public function testHasComposerFileReturnsTrueWithOneOrMoreComposerFiles()
+    {
+        $identifier = 'TEST_IDENTIFIER';
+        $formatted_depot_path = '//' . self::TEST_DEPOT . '/' . $identifier;
+        $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);
         $this->assertTrue($result);
     }
 
@@ -143,4 +165,11 @@ class PerforceDriverTest extends \PHPUnit_Framework_TestCase
         $this->expectOutputString('');
         $this->assertFalse(PerforceDriver::supports($this->io, $this->config, 'existing.url'));
     }
+
+    public function testCleanup()
+    {
+        $this->perforce->expects($this->once())->method('cleanupClientSpec');
+        $this->driver->cleanup();
+    }
+
 }

tests/Composer/Test/Util/PerforceTest.php

@@ -22,17 +22,49 @@ class PerforceTest extends \PHPUnit_Framework_TestCase
 {
     protected $perforce;
     protected $processExecutor;
+    protected $io;
 
-    public function setUp()
+    const TEST_DEPOT       = 'depot';
+    const TEST_BRANCH      = 'branch';
+    const TEST_P4USER      = 'user';
+    const TEST_CLIENT_NAME = 'TEST';
+    const TEST_PORT        = 'port';
+    const TEST_PATH        = 'path';
+
+    protected function setUp()
     {
         $this->processExecutor = $this->getMock('Composer\Util\ProcessExecutor');
-        $repoConfig = array(
+        $this->repoConfig = $this->getTestRepoConfig();
-            'depot'                       => 'depot',
+        $this->io = $this->getMockIOInterface();
-            'branch'                      => 'branch',
+        $this->createNewPerforceWithWindowsFlag(true);
-            'p4user'                      => 'user',
+    }
-            'unique_perforce_client_name' => 'TEST'
+
+    protected function tearDown()
+    {
+        $this->perforce        = null;
+        $this->io              = null;
+        $this->repoConfig      = null;
+        $this->processExecutor = null;
+    }
+
+    public function getTestRepoConfig()
+    {
+        return array(
+            'depot'                       => self::TEST_DEPOT,
+            'branch'                      => self::TEST_BRANCH,
+            'p4user'                      => self::TEST_P4USER,
+            'unique_perforce_client_name' => self::TEST_CLIENT_NAME
         );
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, true);
+    }
+
+    public function getMockIOInterface()
+    {
+        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()
@@ -98,116 +130,90 @@ class PerforceTest extends \PHPUnit_Framework_TestCase
 
     public function testQueryP4UserWithUserAlreadySet()
     {
-        $io = $this->getMock('Composer\IO\IOInterface');
+        $this->perforce->queryP4user();
-
+        $this->assertEquals(self::TEST_P4USER, $this->perforce->getUser());
-        $repoConfig = array('depot' => 'depot', 'branch' => 'branch', 'p4user' => 'TEST_USER');
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, true, 'TEST');
-
-        $this->perforce->queryP4user($io);
-        $this->assertEquals('TEST_USER', $this->perforce->getUser());
     }
 
     public function testQueryP4UserWithUserSetInP4VariablesWithWindowsOS()
     {
-        $repoConfig = array('depot' => 'depot', 'branch' => 'branch');
+        $this->createNewPerforceWithWindowsFlag(true);
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, true, 'TEST');
+        $this->perforce->setUser(null);
-
-        $io = $this->getMock('Composer\IO\IOInterface');
         $expectedCommand = 'p4 set';
+        $callback = function($command, &$output)
+            {
+                $output = 'P4USER=TEST_P4VARIABLE_USER' . PHP_EOL;
+                return true;
+            };
         $this->processExecutor->expects($this->at(0))
-            ->method('execute')
+                            ->method('execute')
-            ->with($this->equalTo($expectedCommand))
+                            ->with($this->equalTo($expectedCommand))
-            ->will(
+                            ->will($this->returnCallback($callback));
-                $this->returnCallback(
+        $this->perforce->queryP4user();
-                    function ($command, &$output) {
-                        $output = 'P4USER=TEST_P4VARIABLE_USER' . PHP_EOL ;
-
-                        return true;
-                    }
-                )
-            );
-
-        $this->perforce->queryP4user($io);
         $this->assertEquals('TEST_P4VARIABLE_USER', $this->perforce->getUser());
     }
 
     public function testQueryP4UserWithUserSetInP4VariablesNotWindowsOS()
     {
-        $repoConfig = array('depot' => 'depot', 'branch' => 'branch');
+        $this->createNewPerforceWithWindowsFlag(false);
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, false, 'TEST');
+        $this->perforce->setUser(null);
-
-        $io = $this->getMock('Composer\IO\IOInterface');
         $expectedCommand = 'echo $P4USER';
+        $callback = function($command, &$output)
+            {
+                $output = 'TEST_P4VARIABLE_USER' . PHP_EOL;
+                return true;
+            };
         $this->processExecutor->expects($this->at(0))
-            ->method('execute')
+                              ->method('execute')
-            ->with($this->equalTo($expectedCommand))
+                              ->with($this->equalTo($expectedCommand))
-            ->will(
+                              ->will($this->returnCallback($callback));
-                $this->returnCallback(
+        $this->perforce->queryP4user();
-                    function ($command, &$output) {
-                        $output = 'TEST_P4VARIABLE_USER' . PHP_EOL;
-
-                        return true;
-                    }
-                )
-            );
-
-        $this->perforce->queryP4user($io);
         $this->assertEquals('TEST_P4VARIABLE_USER', $this->perforce->getUser());
     }
 
     public function testQueryP4UserQueriesForUser()
     {
-        $repoConfig = array('depot' => 'depot', 'branch' => 'branch');
+        $this->perforce->setUser(null);
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, false, 'TEST');
-        $io = $this->getMock('Composer\IO\IOInterface');
         $expectedQuestion = 'Enter P4 User:';
-        $io->expects($this->at(0))
+        $this->io->expects($this->at(0))
-            ->method('ask')
+                 ->method('ask')
-            ->with($this->equalTo($expectedQuestion))
+                 ->with($this->equalTo($expectedQuestion))
-            ->will($this->returnValue('TEST_QUERY_USER'));
+                 ->will($this->returnValue('TEST_QUERY_USER'));
-
+        $this->perforce->queryP4user();
-        $this->perforce->queryP4user($io);
         $this->assertEquals('TEST_QUERY_USER', $this->perforce->getUser());
     }
 
     public function testQueryP4UserStoresResponseToQueryForUserWithWindows()
     {
-        $repoConfig = array('depot' => 'depot', 'branch' => 'branch');
+        $this->createNewPerforceWithWindowsFlag(true);
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, true, 'TEST');
+        $this->perforce->setUser(null);
-
-        $io = $this->getMock('Composer\IO\IOInterface');
         $expectedQuestion = 'Enter P4 User:';
-        $io->expects($this->at(0))
+        $expectedCommand  = 'p4 set P4USER=TEST_QUERY_USER';
-            ->method('ask')
+        $this->io->expects($this->at(0))
-            ->with($this->equalTo($expectedQuestion))
+                 ->method('ask')
-            ->will($this->returnValue('TEST_QUERY_USER'));
+                 ->with($this->equalTo($expectedQuestion))
-        $expectedCommand = 'p4 set P4USER=TEST_QUERY_USER';
+                 ->will($this->returnValue('TEST_QUERY_USER'));
         $this->processExecutor->expects($this->at(1))
-            ->method('execute')
+                              ->method('execute')
-            ->with($this->equalTo($expectedCommand))
+                              ->with($this->equalTo($expectedCommand))
-            ->will($this->returnValue(0));
+                              ->will($this->returnValue(0));
-
+        $this->perforce->queryP4user();
-        $this->perforce->queryP4user($io);
     }
 
     public function testQueryP4UserStoresResponseToQueryForUserWithoutWindows()
     {
-        $repoConfig = array('depot' => 'depot', 'branch' => 'branch');
+        $this->createNewPerforceWithWindowsFlag(false);
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, false, 'TEST');
+        $this->perforce->setUser(null);
-
-        $io = $this->getMock('Composer\IO\IOInterface');
         $expectedQuestion = 'Enter P4 User:';
-        $io->expects($this->at(0))
+        $expectedCommand  = 'export P4USER=TEST_QUERY_USER';
-            ->method('ask')
+        $this->io->expects($this->at(0))
-            ->with($this->equalTo($expectedQuestion))
+                 ->method('ask')
-            ->will($this->returnValue('TEST_QUERY_USER'));
+                 ->with($this->equalTo($expectedQuestion))
-        $expectedCommand = 'export P4USER=TEST_QUERY_USER';
+                 ->will($this->returnValue('TEST_QUERY_USER'));
         $this->processExecutor->expects($this->at(1))
-            ->method('execute')
+                              ->method('execute')
-            ->with($this->equalTo($expectedCommand))
+                              ->with($this->equalTo($expectedCommand))
-            ->will($this->returnValue(0));
+                              ->will($this->returnValue(0));
-
+        $this->perforce->queryP4user();
-        $this->perforce->queryP4user($io);
     }
 
     public function testQueryP4PasswordWithPasswordAlreadySet()
@@ -218,69 +224,55 @@ class PerforceTest extends \PHPUnit_Framework_TestCase
             'p4user'     => 'user',
             'p4password' => 'TEST_PASSWORD'
         );
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, false, 'TEST');
+        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, false,  $this->getMockIOInterface(), 'TEST');
-        $io = $this->getMock('Composer\IO\IOInterface');
+        $password = $this->perforce->queryP4Password();
-
-        $password = $this->perforce->queryP4Password($io);
         $this->assertEquals('TEST_PASSWORD', $password);
     }
 
     public function testQueryP4PasswordWithPasswordSetInP4VariablesWithWindowsOS()
     {
-        $io = $this->getMock('Composer\IO\IOInterface');
+        $this->createNewPerforceWithWindowsFlag(true);
-
         $expectedCommand = 'p4 set';
+        $callback = function($command, &$output)
+            {
+                $output = 'P4PASSWD=TEST_P4VARIABLE_PASSWORD' . PHP_EOL;
+                return true;
+            };
         $this->processExecutor->expects($this->at(0))
-            ->method('execute')
+                              ->method('execute')
-            ->with($this->equalTo($expectedCommand))
+                              ->with($this->equalTo($expectedCommand))
-            ->will(
+                              ->will($this->returnCallback($callback));
-                $this->returnCallback(
+        $password = $this->perforce->queryP4Password();
-                    function ($command, &$output) {
-                        $output = 'P4PASSWD=TEST_P4VARIABLE_PASSWORD' . PHP_EOL;
-
-                        return true;
-                    }
-                )
-            );
-
-        $password = $this->perforce->queryP4Password($io);
         $this->assertEquals('TEST_P4VARIABLE_PASSWORD', $password);
     }
 
     public function testQueryP4PasswordWithPasswordSetInP4VariablesNotWindowsOS()
     {
-        $repoConfig = array('depot' => 'depot', 'branch' => 'branch', 'p4user' => 'user');
+        $this->createNewPerforceWithWindowsFlag(false);
-        $this->perforce = new Perforce($repoConfig, 'port', 'path', $this->processExecutor, false, 'TEST');
-
-        $io = $this->getMock('Composer\IO\IOInterface');
         $expectedCommand = 'echo $P4PASSWD';
+        $callback = function($command, &$output) 
+            {
+                $output = 'TEST_P4VARIABLE_PASSWORD' . PHP_EOL;
+                return true;
+            };
         $this->processExecutor->expects($this->at(0))
-            ->method('execute')
+                              ->method('execute')
-            ->with($this->equalTo($expectedCommand))
+                              ->with($this->equalTo($expectedCommand))
-            ->will(
+                              ->will($this->returnCallback($callback));
-                $this->returnCallback(
-                    function ($command, &$output) {
-                        $output = 'TEST_P4VARIABLE_PASSWORD' . PHP_EOL;
-
-                        return true;
-                    }
-                )
-            );
 
-        $password = $this->perforce->queryP4Password($io);
+        $password = $this->perforce->queryP4Password();
         $this->assertEquals('TEST_P4VARIABLE_PASSWORD', $password);
     }
 
     public function testQueryP4PasswordQueriesForPassword()
     {
-        $io = $this->getMock('Composer\IO\IOInterface');
         $expectedQuestion = 'Enter password for Perforce user user: ';
-        $io->expects($this->at(0))
+        $this->io->expects($this->at(0))
             ->method('askAndHideAnswer')
             ->with($this->equalTo($expectedQuestion))
             ->will($this->returnValue('TEST_QUERY_PASSWORD'));
 
-        $password = $this->perforce->queryP4Password($io);
+        $password = $this->perforce->queryP4Password();
         $this->assertEquals('TEST_QUERY_PASSWORD', $password);
     }
 
@@ -364,15 +356,35 @@ class PerforceTest extends \PHPUnit_Framework_TestCase
                     }
                 )
             );
+        $expectedCommand2 = 'p4 -u user -p port changes //depot/branch/...';
+        $expectedCallback = function($command, &$output)
+            {
+                $output = 'Change 1234 on 2014/03/19 by Clark.Stuth@Clark.Stuth_test_client \'test changelist\'';
+                return true;
+            };
+        $this->processExecutor->expects($this->at(1))
+                              ->method('execute')
+                              ->with($this->equalTo($expectedCommand2))
+                              ->will($this->returnCallback($expectedCallback));
 
         $branches = $this->perforce->getBranches();
-        $this->assertEquals('//depot/branch', $branches['master']);
+        $this->assertEquals('//depot/branch@1234', $branches['master']);
     }
 
     public function testGetBranchesWithoutStream()
     {
+        $expectedCommand = 'p4 -u user -p port changes //depot/...';
+        $expectedCallback = function($command, &$output)
+            {
+                $output = 'Change 5678 on 2014/03/19 by Clark.Stuth@Clark.Stuth_test_client \'test changelist\'';
+                return true;
+            };
+        $this->processExecutor->expects($this->once())
+            ->method('execute')
+            ->with($this->equalTo($expectedCommand))
+            ->will($this->returnCallback($expectedCallback));
         $branches = $this->perforce->getBranches();
-        $this->assertEquals('//depot', $branches['master']);
+        $this->assertEquals('//depot@5678', $branches['master']);
     }
 
     public function testGetTagsWithoutStream()
@@ -692,4 +704,19 @@ class PerforceTest extends \PHPUnit_Framework_TestCase
     {
         $this->perforce->setStream('//depot/branch');
     }
+
+    public function testCleanupClientSpecShouldDeleteClient()
+    {
+        $fs = $this->getMock('Composer\Util\Filesystem');
+        $this->perforce->setFilesystem($fs);
+
+        $testClient = $this->perforce->getClient();
+        $expectedCommand = 'p4 -u ' . self::TEST_P4USER . ' -p ' . self::TEST_PORT . ' client -d ' . $testClient;
+        $this->processExecutor->expects($this->once())->method('execute')->with($this->equalTo($expectedCommand));
+
+        $fs->expects($this->once())->method('remove')->with($this->perforce->getP4ClientSpec());
+
+        $this->perforce->cleanupClientSpec();
+    }
+
 }