Merge pull request #304 from stronk7/better_detect_exit_codes

Exit on error when something was wrong executing the command
moodlehq · Jun 3, 2024 · 35a982b · 35a982b
2 parents 783ec09 + 84bcf2c
commit 35a982b
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 1 deletion.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -294,7 +294,7 @@ jobs:
         coverage: none
 
     - name: Download base ${{ env.lowest_release }} PHAR artifact
-      uses: robinraju/release-downloader@v1.8
+      uses: robinraju/release-downloader@v1.10
       with:
         repository: moodlehq/moodle-plugin-ci
         tag: ${{ env.lowest_release }}

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -15,6 +15,7 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt
 
 ### Fixed
 - Solved a problem with the validation of `dataformat` plugin lang strings.
+- Fixed a problem with the `phpcs` command returning with success when some (configuration, installation, ...) problem was causing it not to be executed at all.
 
 ## [4.4.5] - 2024-04-03
 ### Changed

diff --git a/src/Command/CodeCheckerCommand.php b/src/Command/CodeCheckerCommand.php
@@ -178,6 +178,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 
         // Arrived here, we are playing with max-warnings, so we have to decide the exit code
         // based on the existence of errors and the number of warnings compared with the threshold.
+
+        // Verify that the summary file was created. If not, something went wrong with the execution.
+        if (!file_exists($this->tempFile)) {
+            return 1;
+        }
+
+        // Let's inspect the summary file to get the total number of errors and warnings.
         $totalErrors   = 0;
         $totalWarnings = 0;
         $jsonFile      = trim(file_get_contents($this->tempFile));

diff --git a/tests/Command/CodeCheckerCommandTest.php b/tests/Command/CodeCheckerCommandTest.php
@@ -324,4 +324,43 @@ public function testExecuteNoPlugin()
         $this->expectException(\InvalidArgumentException::class);
         $this->executeCommand('/path/to/no/plugin');
     }
+
+    /**
+     * Data provider for testCommandFailedSomethingIsWrong.
+     *
+     * @return array of options to use for the command, all them known to fail
+     */
+    public function commandFailedSomethingIsWrongProvider()
+    {
+        return [
+            'default' => [
+                ['--standard' => 'chocolate', '--max-warnings' => -1],
+                'ERROR: the "chocolate" coding standard is not installed',
+            ],
+            'zero' => [
+                ['--standard' => 'chocolate', '--max-warnings' => 0],
+                'ERROR: the "chocolate" coding standard is not installed',
+            ],
+            'positive' => [
+                ['--standard' => 'chocolate', '--max-warnings' => 1],
+                'ERROR: the "chocolate" coding standard is not installed',
+            ],
+        ];
+    }
+
+    /**
+     * Verify that if anything goes wrong, the command fails, no matter the max-warnings.
+     *
+     * @param array  $options configuration to use for the command
+     * @param string $output  Expected output (substring matching is enough)
+     *
+     * @dataProvider commandFailedSomethingIsWrongProvider
+     */
+    public function testCommandFailedSomethingIsWrong(array $options, string $output)
+    {
+        // Verify that with incorrect configurations we are getting the command always failed.
+        $commandTester = $this->executeCommand($this->pluginDir, $options);
+        $this->assertSame(1, $commandTester->getStatusCode());
+        $this->assertStringContainsString($output, $commandTester->getDisplay());
+    }
 }