Repository: bmitch/churn-php (php)

Issue:
Option to disable parallelization
Disabling parallelization will force to better structure the code.
It will also be the occasion to improve how to handle certain values (eg: when `parallelJobs` < 1).


Discussion:

Patch:
diff --git a/src/Commands/ChurnCommand.php b/src/Commands/ChurnCommand.php
--- a/src/Commands/ChurnCommand.php
+++ b/src/Commands/ChurnCommand.php
@@ -4,9 +4,9 @@
 
 use Churn\Factories\ResultsRendererFactory;
 use Churn\Logic\ResultsLogic;
-use Churn\Managers\ProcessManager;
 use Churn\Factories\ProcessFactory;
 use Churn\Managers\FileManager;
+use Churn\Processes\ProcessHandlerFactory;
 use function count;
 use function file_get_contents;
 use Symfony\Component\Console\Command\Command;
@@ -30,10 +30,10 @@ class ChurnCommand extends Command
     private $resultsLogic;
 
     /**
-     * The process manager.
-     * @var ProcessManager
+     * The process handler factory.
+     * @var ProcessHandlerFactory
      */
-    private $processManager;
+    private $processHandlerFactory;
 
     /**
      * The renderer factory.
@@ -43,18 +43,18 @@ class ChurnCommand extends Command
 
     /**
      * ChurnCommand constructor.
-     * @param ResultsLogic           $resultsLogic   The results logic.
-     * @param ProcessManager         $processManager The process manager.
-     * @param ResultsRendererFactory $renderFactory  The Results Renderer Factory.
+     * @param ResultsLogic           $resultsLogic          The results logic.
+     * @param ProcessHandlerFactory  $processHandlerFactory The process handler factory.
+     * @param ResultsRendererFactory $renderFactory         The Results Renderer Factory.
      */
     public function __construct(
         ResultsLogic $resultsLogic,
-        ProcessManager $processManager,
+        ProcessHandlerFactory $processHandlerFactory,
         ResultsRendererFactory $renderFactory
     ) {
         parent::__construct();
         $this->resultsLogic = $resultsLogic;
-        $this->processManager = $processManager;
+        $this->processHandlerFactory = $processHandlerFactory;
         $this->renderFactory = $renderFactory;
     }
 
@@ -85,7 +85,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $filesCollection = (new FileManager($config->getFileExtensions(), $config->getFilesToIgnore()))
             ->getPhpFiles($this->getDirectoriesToScan($input, $config->getDirectoriesToScan()));
 
-        $completedProcesses = $this->processManager->process(
+        $completedProcesses = $this->processHandlerFactory->getProcessHandler($config)->process(
             $filesCollection,
             new ProcessFactory($config->getCommitsSince()),
             $config->getParallelJobs()
diff --git a/src/Managers/ProcessManager.php b/src/Processes/Handler/ParallelProcessHandler.php
similarity index 72%
rename from src/Managers/ProcessManager.php
rename to src/Processes/Handler/ParallelProcessHandler.php
--- a/src/Managers/ProcessManager.php
+++ b/src/Processes/Handler/ParallelProcessHandler.php
@@ -1,12 +1,12 @@
 <?php declare(strict_types = 1);
 
-namespace Churn\Managers;
+namespace Churn\Processes\Handler;
 
 use Churn\Collections\FileCollection;
 use Churn\Factories\ProcessFactory;
 use Illuminate\Support\Collection;
 
-class ProcessManager
+class ParallelProcessHandler implements ProcessHandler
 {
     /**
      * Collection of running processes.
@@ -32,24 +32,37 @@ class ProcessManager
      */
     private $processFactory;
 
+    /**
+     * Number of parallel jobs to run.
+     * @var integer
+     */
+    private $numberOfParallelJobs;
+
+    /**
+     * ProcessManager constructor.
+     * @param int $numberOfParallelJobs Number of parallel jobs to run.
+     */
+    public function __construct(int $numberOfParallelJobs)
+    {
+        $this->numberOfParallelJobs = $numberOfParallelJobs;
+    }
+
     /**
      * Run the processes to gather information.
-     * @param FileCollection $filesCollection      Collection of files.
-     * @param ProcessFactory $processFactory       Process Factory.
-     * @param integer        $numberOfParallelJobs Number of parallel jobs to run.
+     * @param FileCollection $filesCollection Collection of files.
+     * @param ProcessFactory $processFactory  Process Factory.
      * @return Collection
      */
     public function process(
         FileCollection $filesCollection,
-        ProcessFactory $processFactory,
-        int $numberOfParallelJobs
+        ProcessFactory $processFactory
     ): Collection {
         $this->filesCollection = $filesCollection;
         $this->processFactory = $processFactory;
         $this->runningProcesses = new Collection;
         $this->completedProcessesArray = [];
         while ($filesCollection->hasFiles() || $this->runningProcesses->count()) {
-            $this->getProcessResults($numberOfParallelJobs);
+            $this->getProcessResults($this->numberOfParallelJobs);
         }
         return new Collection($this->completedProcessesArray);
     }
@@ -62,7 +75,7 @@ public function process(
     private function getProcessResults(int $numberOfParallelJobs): void
     {
         $index = $this->runningProcesses->count();
-        for (; $this->filesCollection->hasFiles() > 0 && $index < $numberOfParallelJobs; $index++) {
+        for (; $index < $numberOfParallelJobs && $this->filesCollection->hasFiles() > 0; $index++) {
             $file = $this->filesCollection->getNextFile();
             $process = $this->processFactory->createGitCommitProcess($file);
             $process->start();
diff --git a/src/Processes/Handler/ProcessHandler.php b/src/Processes/Handler/ProcessHandler.php
new file mode 100644
--- /dev/null
+++ b/src/Processes/Handler/ProcessHandler.php
@@ -0,0 +1,21 @@
+<?php declare(strict_types = 1);
+
+namespace Churn\Processes\Handler;
+
+use Churn\Collections\FileCollection;
+use Churn\Factories\ProcessFactory;
+use Illuminate\Support\Collection;
+
+interface ProcessHandler
+{
+    /**
+     * Run the processes to gather information.
+     * @param FileCollection $filesCollection Collection of files.
+     * @param ProcessFactory $processFactory  Process Factory.
+     * @return Collection
+     */
+    public function process(
+        FileCollection $filesCollection,
+        ProcessFactory $processFactory
+    ): Collection;
+}
diff --git a/src/Processes/Handler/SequentialProcessHandler.php b/src/Processes/Handler/SequentialProcessHandler.php
new file mode 100644
--- /dev/null
+++ b/src/Processes/Handler/SequentialProcessHandler.php
@@ -0,0 +1,35 @@
+<?php declare(strict_types = 1);
+
+namespace Churn\Processes\Handler;
+
+use Churn\Collections\FileCollection;
+use Churn\Factories\ProcessFactory;
+use Illuminate\Support\Collection;
+
+class SequentialProcessHandler implements ProcessHandler
+{
+    /**
+     * Run the processes sequentially to gather information.
+     * @param FileCollection $filesCollection Collection of files.
+     * @param ProcessFactory $processFactory  Process Factory.
+     * @return Collection
+     */
+    public function process(
+        FileCollection $filesCollection,
+        ProcessFactory $processFactory
+    ): Collection {
+        $completedProcessesArray = [];
+        while ($filesCollection->hasFiles()) {
+            $file = $filesCollection->getNextFile();
+            $process = $processFactory->createGitCommitProcess($file);
+            $process->start();
+            while (!$process->isSuccessful());
+            $completedProcessesArray[$process->getFileName()][$process->getType()] = $process;
+            $process = $processFactory->createCyclomaticComplexityProcess($file);
+            $process->start();
+            while (!$process->isSuccessful());
+            $completedProcessesArray[$process->getFileName()][$process->getType()] = $process;
+        }
+        return new Collection($completedProcessesArray);
+    }
+}
diff --git a/src/Processes/ProcessHandlerFactory.php b/src/Processes/ProcessHandlerFactory.php
new file mode 100644
--- /dev/null
+++ b/src/Processes/ProcessHandlerFactory.php
@@ -0,0 +1,25 @@
+<?php declare(strict_types = 1);
+
+namespace Churn\Processes;
+
+use Churn\Configuration\Config;
+use Churn\Processes\Handler\ParallelProcessHandler;
+use Churn\Processes\Handler\ProcessHandler;
+use Churn\Processes\Handler\SequentialProcessHandler;
+
+class ProcessHandlerFactory
+{
+    /**
+     * Returns a process handler depending on the configuration.
+     * @param Config $config The application configuration.
+     * @return ProcessHandler
+     */
+    public function getProcessHandler(Config $config): ProcessHandler
+    {
+        if ($config->getParallelJobs() > 1) {
+            return new ParallelProcessHandler($config->getParallelJobs());
+        }
+
+        return new SequentialProcessHandler();
+    }
+}


Test patch:
diff --git a/tests/Integration/Managers/FileManagerTest.php b/tests/Integration/Managers/FileManagerTest.php
--- a/tests/Integration/Managers/FileManagerTest.php
+++ b/tests/Integration/Managers/FileManagerTest.php
@@ -1,4 +1,4 @@
-<?php
+<?php declare(strict_types = 1);
 
 namespace Churn\Tests\Integration\Managers;
 
@@ -25,7 +25,7 @@ public function it_can_be_instantiated()
     /** @test */
     public function it_can_recursively_get_the_php_files_in_a_path()
     {
-        $paths = [__DIR__ . ''];
+        $paths = [__DIR__];
         $results = $this->fileManager->getPhpFiles($paths);
         $this->assertInstanceOf(Collection::class, $results);
         $this->assertCount(1, $results);
diff --git a/tests/Unit/Processes/ChurnProcessTest.php b/tests/Unit/Processes/ChurnProcessTest.php
--- a/tests/Unit/Processes/ChurnProcessTest.php
+++ b/tests/Unit/Processes/ChurnProcessTest.php
@@ -1,4 +1,6 @@
-<?php
+<?php declare(strict_types = 1);
+
+namespace Churn\Tests\Unit\Processes;
 
 use Mockery as m;
 use Churn\Tests\BaseTestCase;
diff --git a/tests/Unit/Managers/ProcessManagerTest.php b/tests/Unit/Processes/Handler/ParallelProcessHandlerTest.php
similarity index 61%
rename from tests/Unit/Managers/ProcessManagerTest.php
rename to tests/Unit/Processes/Handler/ParallelProcessHandlerTest.php
--- a/tests/Unit/Managers/ProcessManagerTest.php
+++ b/tests/Unit/Processes/Handler/ParallelProcessHandlerTest.php
@@ -1,32 +1,31 @@
 <?php declare(strict_types = 1);
 
-namespace Churn\Tests\Unit\Managers;
+namespace Churn\Tests\Unit\Processes\Handler;
 
 use Churn\Collections\FileCollection;
 use Churn\Configuration\Config;
 use Churn\Factories\ProcessFactory;
-use Churn\Managers\ProcessManager;
+use Churn\Processes\Handler\ParallelProcessHandler;
 use Churn\Tests\BaseTestCase;
 use Churn\Values\File;
 
-class ProcessManagerTest extends BaseTestCase
+class ParallelProcessHandlerTest extends BaseTestCase
 {
     /** @test */
     public function it_can_be_instantiated() 
     {
-        $this->assertInstanceOf(ProcessManager::class, new ProcessManager);
+        $this->assertInstanceOf(ParallelProcessHandler::class, new ParallelProcessHandler(2));
     }
 
     /** @test */
     public function it_returns_empty_collection_when_no_files() 
     {
-        $numParallelJobs = 3;
-        $processManager = new ProcessManager();
+        $processHandler = new ParallelProcessHandler(3);
         $config = Config::createFromDefaultValues();
         $processFactory = new ProcessFactory($config->getCommitsSince());
         $fileCollection = new FileCollection();
         
-        $collection = $processManager->process($fileCollection, $processFactory, $numParallelJobs);
+        $collection = $processHandler->process($fileCollection, $processFactory);
         $this->assertEquals($collection->count(), 0);        
     }
 }
diff --git a/tests/Unit/Processes/Handler/SequentialProcessHandlerTest.php b/tests/Unit/Processes/Handler/SequentialProcessHandlerTest.php
new file mode 100644
--- /dev/null
+++ b/tests/Unit/Processes/Handler/SequentialProcessHandlerTest.php
@@ -0,0 +1,47 @@
+<?php declare(strict_types = 1);
+
+namespace Churn\Tests\Unit\Processes\Handler;
+
+use Churn\Collections\FileCollection;
+use Churn\Factories\ProcessFactory;
+use Churn\Processes\ChurnProcess;
+use Churn\Processes\Handler\SequentialProcessHandler;
+use Churn\Tests\BaseTestCase;
+use Churn\Values\File;
+use Illuminate\Support\Collection;
+use Mockery as m;
+
+class SequentialProcessHandlerTest extends BaseTestCase
+{
+    /** @test */
+    public function it_can_be_instantiated()
+    {
+        $this->assertInstanceOf(SequentialProcessHandler::class, new SequentialProcessHandler());
+    }
+
+    /** @test */
+    public function it_returns_a_collection_of_results()
+    {
+        $process1 = m::mock(ChurnProcess::class);
+        $process1->shouldReceive('start');
+        $process1->shouldReceive('isSuccessful')->andReturn(true);
+        $process1->shouldReceive('getFileName')->andReturn(__FILE__);
+        $process1->shouldReceive('getType')->andReturn('GitCommitProcess');
+        
+        $process2 = m::mock(ChurnProcess::class);
+        $process2->shouldReceive('start');
+        $process2->shouldReceive('isSuccessful')->andReturn(true);
+        $process2->shouldReceive('getFileName')->andReturn(__FILE__);
+        $process2->shouldReceive('getType')->andReturn('CyclomaticComplexityProcess');
+        
+        $fileCollection = new FileCollection([new File(['fullPath' => __FILE__, 'displayPath' => __FILE__])]);
+        $processFactory = m::mock(ProcessFactory::class);
+        $processFactory->shouldReceive('createGitCommitProcess')->andReturn($process1);
+        $processFactory->shouldReceive('createCyclomaticComplexityProcess')->andReturn($process2);
+
+        $processHandler = new SequentialProcessHandler();
+        $results = $processHandler->process($fileCollection, $processFactory);
+
+        $this->assertInstanceOf(Collection::class, $results);
+    }
+}
diff --git a/tests/Unit/Processes/ProcessHandlerFactoryTest.php b/tests/Unit/Processes/ProcessHandlerFactoryTest.php
new file mode 100644
--- /dev/null
+++ b/tests/Unit/Processes/ProcessHandlerFactoryTest.php
@@ -0,0 +1,45 @@
+<?php declare(strict_types = 1);
+
+namespace Churn\Tests\Unit\Processes;
+
+use Churn\Configuration\Config;
+use Churn\Processes\Handler\ParallelProcessHandler;
+use Churn\Processes\Handler\SequentialProcessHandler;
+use Churn\Processes\ProcessHandlerFactory;
+use Churn\Tests\BaseTestCase;
+use Mockery as m;
+
+class ProcessHandlerFactoryTest extends BaseTestCase
+{
+    /** @test */
+    public function it_can_be_instantiated()
+    {
+        $this->assertInstanceOf(ProcessHandlerFactory::class, new ProcessHandlerFactory());
+    }
+
+    /**
+     * @test
+     * @dataProvider provideConfigWithCorrespondingProcessHandler
+     */
+    public function it_returns_the_right_process_handler(Config $config, string $expectedClassName)
+    {
+        $factory = new ProcessHandlerFactory();
+        $processHandler = $factory->getProcessHandler($config);
+        $this->assertEquals($expectedClassName, get_class($processHandler));
+    }
+
+    public function provideConfigWithCorrespondingProcessHandler(): iterable
+    {
+        $config = m::mock(Config::class);
+        $config->shouldReceive('getParallelJobs')->andReturn(0);
+        yield [$config, SequentialProcessHandler::class];
+        
+        $config = m::mock(Config::class);
+        $config->shouldReceive('getParallelJobs')->andReturn(1);
+        yield [$config, SequentialProcessHandler::class];
+        
+        $config = m::mock(Config::class);
+        $config->shouldReceive('getParallelJobs')->andReturn(2);
+        yield [$config, ParallelProcessHandler::class];
+    }
+}