Repository: bmitch/churn-php (php)

Issue:
Make command fail if score is higher than a configurable threshold
It could be used in a CI pipeline.


Discussion:

Patch:
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -85,6 +85,11 @@ filesToShow: 10
 # Default: 0.1
 minScoreToShow: 0
 
+# The command returns an 1 exit code if the highest score is greater than the threshold.
+# Disabled if null.
+# Default: null
+maxScoreThreshold: 0.9
+
 # The number of parallel jobs to use when processing files.
 # Default: 10
 parallelJobs: 10
@@ -126,12 +131,12 @@ hooks:
 # The version control system used for your project.
 # Accepted values: fossil, git, mercurial, subversion, none
 # Default: git
- vcs: git
+vcs: git
 
 # The path of the cache file. It doesn't need to exist before running churn.
 # Disabled if null.
 # Default: null
- cachePath: .churn.cache
+cachePath: .churn.cache
  ```
 
 If a `churn.yml` file is omitted or an individual setting is omitted the default values above will be used.
diff --git a/src/Command/Helper/MaxScoreChecker.php b/src/Command/Helper/MaxScoreChecker.php
new file mode 100644
--- /dev/null
+++ b/src/Command/Helper/MaxScoreChecker.php
@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Churn\Command\Helper;
+
+use Churn\Result\ResultAccumulator;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+/**
+ * @internal
+ */
+class MaxScoreChecker
+{
+
+    /**
+     * @var float|null
+     */
+    private $maxScoreThreshold;
+
+    /**
+     * @param float|null $maxScoreThreshold The max score threshold.
+     */
+    public function __construct(?float $maxScoreThreshold)
+    {
+        $this->maxScoreThreshold = $maxScoreThreshold;
+    }
+
+    /**
+     * @param InputInterface $input Input.
+     * @param OutputInterface $output Output.
+     * @param ResultAccumulator $report The report containing the scores.
+     */
+    public function isOverThreshold(InputInterface $input, OutputInterface $output, ResultAccumulator $report): bool
+    {
+        $maxScore = $report->getMaxScore();
+
+        if (null === $this->maxScoreThreshold || null === $maxScore || $maxScore <= $this->maxScoreThreshold) {
+            return false;
+        }
+
+        if ('text' === $input->getOption('format') || !empty($input->getOption('output'))) {
+            $output->writeln('<error>Max score is over the threshold</>');
+        }
+
+        return true;
+    }
+}
diff --git a/src/Command/RunCommand.php b/src/Command/RunCommand.php
--- a/src/Command/RunCommand.php
+++ b/src/Command/RunCommand.php
@@ -4,6 +4,7 @@
 
 namespace Churn\Command;
 
+use Churn\Command\Helper\MaxScoreChecker;
 use Churn\Command\Helper\ProgressBarSubscriber;
 use Churn\Configuration\Config;
 use Churn\Configuration\Loader;
@@ -109,7 +110,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         }
         $config = $this->getConfiguration($input);
         $broker = new Broker();
-        $this->attachHooks($config, $broker);
+        (new HookLoader($config->getDirPath()))->attachHooks($config->getHooks(), $broker);
         $this->printLogo($input, $output);
         if (true === $input->getOption('progress')) {
             $broker->subscribe(new ProgressBarSubscriber($output));
@@ -117,7 +118,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $report = $this->analyze($input, $config, $broker);
         $this->writeResult($input, $output, $report);
 
-        return 0;
+        return (int) (new MaxScoreChecker($config->getMaxScoreThreshold()))->isOverThreshold($input, $output, $report);
     }
 
     /**
@@ -147,26 +148,6 @@ private function getConfiguration(InputInterface $input): Config
         return $config;
     }
 
-    /**
-     * @param Config $config The configuration object.
-     * @param Broker $broker The event broker.
-     * @throws InvalidArgumentException If a hook is invalid.
-     */
-    private function attachHooks(Config $config, Broker $broker): void
-    {
-        if ([] === $config->getHooks()) {
-            return;
-        }
-
-        $loader = new HookLoader($config->getDirPath());
-
-        foreach ($config->getHooks() as $hook) {
-            if (!$loader->attach($hook, $broker)) {
-                throw new InvalidArgumentException('Invalid hook: ' . $hook);
-            }
-        }
-    }
-
     /**
      * Run the actual analysis.
      *
diff --git a/src/Configuration/Config.php b/src/Configuration/Config.php
--- a/src/Configuration/Config.php
+++ b/src/Configuration/Config.php
@@ -13,6 +13,7 @@ class Config
     private const DIRECTORIES_TO_SCAN = [];
     private const FILES_TO_SHOW = 10;
     private const MINIMUM_SCORE_TO_SHOW = 0.1;
+    private const MAXIMUM_SCORE_THRESHOLD = null;
     private const AMOUNT_OF_PARALLEL_JOBS = 10;
     private const SHOW_COMMITS_SINCE = '10 years ago';
     private const FILES_TO_IGNORE = [];
@@ -112,6 +113,18 @@ public function getMinScoreToShow(): ?float
         return self::MINIMUM_SCORE_TO_SHOW;
     }
 
+    /**
+     * Get the maximum score threshold.
+     */
+    public function getMaxScoreThreshold(): ?float
+    {
+        if (\array_key_exists('maxScoreThreshold', $this->configuration)) {
+            return $this->configuration['maxScoreThreshold'];
+        }
+
+        return self::MAXIMUM_SCORE_THRESHOLD;
+    }
+
     /**
      * Get the number of parallel jobs to use to process the files.
      */
diff --git a/src/Configuration/Validator.php b/src/Configuration/Validator.php
--- a/src/Configuration/Validator.php
+++ b/src/Configuration/Validator.php
@@ -20,6 +20,7 @@ public function validateConfigurationValues(array $configuration): void
         $this->validateDirectoriesToScan($configuration);
         $this->validateFilesToShow($configuration);
         $this->validateMinScoreToShow($configuration);
+        $this->validateMaxScoreThreshold($configuration);
         $this->validateParallelJobs($configuration);
         $this->validateCommitsSince($configuration);
         $this->validateFilesToIgnore($configuration);
@@ -59,13 +60,25 @@ private function validateFilesToShow(array $configuration): void
      */
     private function validateMinScoreToShow(array $configuration): void
     {
-        if (!\array_key_exists('minScoreToShow', $configuration) || null === $configuration['minScoreToShow']) {
+        if (!isset($configuration['minScoreToShow'])) {
             return;
         }
 
         Assert::numeric($configuration['minScoreToShow'], 'Minimum score to show should be a number');
     }
 
+    /**
+     * @param array<mixed> $configuration The array containing the configuration values.
+     */
+    private function validateMaxScoreThreshold(array $configuration): void
+    {
+        if (!isset($configuration['maxScoreThreshold'])) {
+            return;
+        }
+
+        Assert::numeric($configuration['maxScoreThreshold'], 'Maximum score threshold should be a number');
+    }
+
     /**
      * @param array<mixed> $configuration The array containing the configuration values.
      */
diff --git a/src/Event/HookLoader.php b/src/Event/HookLoader.php
--- a/src/Event/HookLoader.php
+++ b/src/Event/HookLoader.php
@@ -11,6 +11,7 @@
 use Churn\Event\Subscriber\AfterFileAnalysisHookDecorator;
 use Churn\Event\Subscriber\BeforeAnalysisHookDecorator;
 use Churn\File\FileHelper;
+use InvalidArgumentException;
 
 /**
  * @internal
@@ -41,6 +42,24 @@ public function __construct(string $basePath)
         $this->basePath = $basePath;
     }
 
+    /**
+     * @param array<string> $hooks The list of hooks to attach.
+     * @param Broker $broker The event broker.
+     * @throws InvalidArgumentException If a hook is invalid.
+     */
+    public function attachHooks(array $hooks, Broker $broker): void
+    {
+        if ([] === $hooks) {
+            return;
+        }
+
+        foreach ($hooks as $hook) {
+            if (!$this->attach($hook, $broker)) {
+                throw new InvalidArgumentException('Invalid hook: ' . $hook);
+            }
+        }
+    }
+
     /**
      * @param string $hookPath The class name or the file path of the hook.
      * @param Broker $broker The event broker.


Test patch:
diff --git a/tests/Integration/Command/RunCommandTest.php b/tests/Integration/Command/RunCommandTest.php
--- a/tests/Integration/Command/RunCommandTest.php
+++ b/tests/Integration/Command/RunCommandTest.php
@@ -224,4 +224,17 @@ public function it_can_suppress_normal_output(): void
         $this->assertEquals(0, $exitCode);
         $this->assertEquals('Churn: DONE', $display);
     }
+
+    /** @test */
+    public function it_can_return_one_as_exit_code(): void
+    {
+        $exitCode = $this->commandTester->execute([
+            'paths' => [__FILE__],
+            '-c' => __DIR__ . '/config/test-threshold.yml',
+        ]);
+        $display = $this->commandTester->getDisplay();
+
+        $this->assertEquals(1, $exitCode);
+        $this->assertStringContainsString('Max score is over the threshold', $display);
+    }
 }
diff --git a/tests/Integration/Command/config/test-threshold.yml b/tests/Integration/Command/config/test-threshold.yml
new file mode 100644
--- /dev/null
+++ b/tests/Integration/Command/config/test-threshold.yml
@@ -0,0 +1 @@
+maxScoreThreshold: 0.9
\ No newline at end of file
diff --git a/tests/Unit/Command/Helper/MaxScoreCheckerTest.php b/tests/Unit/Command/Helper/MaxScoreCheckerTest.php
new file mode 100644
--- /dev/null
+++ b/tests/Unit/Command/Helper/MaxScoreCheckerTest.php
@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Churn\Tests\Unit\Command\Helper;
+
+use Churn\Command\Helper\MaxScoreChecker;
+use Churn\Result\ResultAccumulator;
+use Churn\Tests\BaseTestCase;
+use Mockery as m;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class MaxScoreCheckerTest extends BaseTestCase
+{
+
+    /**
+     * @test
+     * @dataProvider provide_arguments
+     */
+    public function it_can_check_the_max_score(
+        bool $expectedResult,
+        ?float $threshold,
+        ?float $maxScore,
+        string $format = 'text',
+        ?string $output = null 
+    ): void {
+        $input = m::mock(InputInterface::class);
+        $input->shouldReceive('getOption')->with('format')->andReturn($format);
+        $input->shouldReceive('getOption')->with('output')->andReturn($output);
+
+        $output = m::mock(OutputInterface::class);
+        $output->shouldReceive('writeln');
+
+        $report = m::mock(ResultAccumulator::class);
+        $report->shouldReceive('getMaxScore')->andReturn($maxScore);
+
+        $checker = new MaxScoreChecker($threshold);
+
+        $this->assertEquals(
+            $expectedResult,
+            $checker->isOverThreshold($input, $output, $report)
+        );
+    }
+
+    public function provide_arguments(): iterable
+    {
+        yield [false, null, null];
+        yield [false, 1, null];
+        yield [false, null, 1];
+        yield [false, 1, 0.1];
+        yield [true, 0.1, 1];
+        yield [true, 0.1, 1, 'text', '/tmp'];
+    }
+}
diff --git a/tests/Unit/Configuration/ConfigTest.php b/tests/Unit/Configuration/ConfigTest.php
--- a/tests/Unit/Configuration/ConfigTest.php
+++ b/tests/Unit/Configuration/ConfigTest.php
@@ -52,6 +52,7 @@ public function it_can_return_its_default_values_when_instantiated_without_any_p
         $this->assertSame([], $config->getDirectoriesToScan());
         $this->assertSame(10, $config->getFilesToShow());
         $this->assertSame(0.1, $config->getMinScoreToShow());
+        $this->assertSame(null, $config->getMaxScoreThreshold());
         $this->assertSame(10, $config->getParallelJobs());
         $this->assertSame('10 years ago', $config->getCommitsSince());
         $this->assertSame([], $config->getFilesToIgnore());
@@ -67,6 +68,7 @@ public function it_can_return_its_values_when_instantiated_parameters()
         $filesToShow = 13;
         $directoriesToScan = ['src', 'tests'];
         $minScoreToShow = 5;
+        $maxScoreThreshold = 9.5;
         $parallelJobs = 7;
         $commitsSince = '4 years ago';
         $filesToIgnore = ['foo.php', 'bar.php', 'baz.php'];
@@ -79,6 +81,7 @@ public function it_can_return_its_values_when_instantiated_parameters()
             'directoriesToScan' => $directoriesToScan,
             'filesToShow' => $filesToShow,
             'minScoreToShow' => $minScoreToShow,
+            'maxScoreThreshold' => $maxScoreThreshold,
             'parallelJobs' => $parallelJobs,
             'commitsSince' => $commitsSince,
             'filesToIgnore' => $filesToIgnore,
@@ -91,6 +94,7 @@ public function it_can_return_its_values_when_instantiated_parameters()
         $this->assertSame($directoriesToScan, $config->getDirectoriesToScan());
         $this->assertSame($filesToShow, $config->getFilesToShow());
         $this->assertEquals($minScoreToShow, $config->getMinScoreToShow());
+        $this->assertSame($maxScoreThreshold, $config->getMaxScoreThreshold());
         $this->assertSame($parallelJobs, $config->getParallelJobs());
         $this->assertSame($commitsSince, $config->getCommitsSince());
         $this->assertSame($filesToIgnore, $config->getFilesToIgnore());