Skip to content

Commit 3f3cfb3

Browse files
committed
Exit on error when something was wrong executing the command
When something abnormal happens (wrong configuration, missing stuff, connectivity problems, etc., the phpcs execution can end with error. But, if max-warnings is enabled (>=0), then the count of errors and warnings (that is always 0 on error) gets precedence, hiding the real exit status. Hence it passes, when really it was not executed. Now we detect those abnormal situations and exit with error when something in the execution goes wrong. Covered with tests, checking all the possible max-warnings combinations.
1 parent 783ec09 commit 3f3cfb3

File tree

3 files changed

+47
-0
lines changed

3 files changed

+47
-0
lines changed

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt
1515

1616
### Fixed
1717
- Solved a problem with the validation of `dataformat` plugin lang strings.
18+
- Fixed a problem with the `phpcs` command returning with success when some (configuration, installation, ...) problem was causing it not to be executed at all.
1819

1920
## [4.4.5] - 2024-04-03
2021
### Changed

src/Command/CodeCheckerCommand.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int
178178

179179
// Arrived here, we are playing with max-warnings, so we have to decide the exit code
180180
// based on the existence of errors and the number of warnings compared with the threshold.
181+
182+
// Verify that the summary file was created. If not, something went wrong with the execution.
183+
if (!file_exists($this->tempFile)) {
184+
return 1;
185+
}
186+
187+
// Let's inspect the summary file to get the total number of errors and warnings.
181188
$totalErrors = 0;
182189
$totalWarnings = 0;
183190
$jsonFile = trim(file_get_contents($this->tempFile));

tests/Command/CodeCheckerCommandTest.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,4 +324,43 @@ public function testExecuteNoPlugin()
324324
$this->expectException(\InvalidArgumentException::class);
325325
$this->executeCommand('/path/to/no/plugin');
326326
}
327+
328+
/**
329+
* Data provider for testCommandFailedSomethingIsWrong.
330+
*
331+
* @return array of options to use for the command, all them known to fail
332+
*/
333+
public function commandFailedSomethingIsWrongProvider()
334+
{
335+
return [
336+
'default' => [
337+
['--standard' => 'chocolate', '--max-warnings' => -1],
338+
'ERROR: the "chocolate" coding standard is not installed',
339+
],
340+
'zero' => [
341+
['--standard' => 'chocolate', '--max-warnings' => 0],
342+
'ERROR: the "chocolate" coding standard is not installed',
343+
],
344+
'positive' => [
345+
['--standard' => 'chocolate', '--max-warnings' => 1],
346+
'ERROR: the "chocolate" coding standard is not installed',
347+
],
348+
];
349+
}
350+
351+
/**
352+
* Verify that if anything goes wrong, the command fails, no matter the max-warnings.
353+
*
354+
* @param array $options configuration to use for the command
355+
* @param string $output Expected output (substring matching is enough)
356+
*
357+
* @dataProvider commandFailedSomethingIsWrongProvider
358+
*/
359+
public function testCommandFailedSomethingIsWrong(array $options, string $output)
360+
{
361+
// Verify that with incorrect configurations we are getting the command always failed.
362+
$commandTester = $this->executeCommand($this->pluginDir, $options);
363+
$this->assertSame(1, $commandTester->getStatusCode());
364+
$this->assertStringContainsString($output, $commandTester->getDisplay());
365+
}
327366
}

0 commit comments

Comments
 (0)