From 5939b056f9a7bc8660fe850f53581d12edd7b1d0 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 12 Apr 2025 21:15:50 +0200 Subject: [PATCH 1/2] Runner::printProgress(): add tests --- tests/Core/Runner/PrintProgressDotsTest.php | 222 ++++++++++++++++++ tests/Core/Runner/PrintProgressTest.php | 235 ++++++++++++++++++++ 2 files changed, 457 insertions(+) create mode 100644 tests/Core/Runner/PrintProgressDotsTest.php create mode 100644 tests/Core/Runner/PrintProgressTest.php diff --git a/tests/Core/Runner/PrintProgressDotsTest.php b/tests/Core/Runner/PrintProgressDotsTest.php new file mode 100644 index 0000000000..d4630bb8e5 --- /dev/null +++ b/tests/Core/Runner/PrintProgressDotsTest.php @@ -0,0 +1,222 @@ +markTestSkipped('This test needs CS mode to run'); + } + + $this->checkProgressDot($colors, $code, $sniffs, $expected); + + }//end testProgressDotCs() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataProgressDotCs() + { + return [ + 'No colors: Dot: no errors, no warnings' => [ + 'colors' => false, + 'code' => ' 'Generic.PHP.LowerCaseConstant', + 'expected' => '.', + ], + 'No colors: E: has error' => [ + 'colors' => false, + 'code' => ' 'Generic.CodeAnalysis.RequireExplicitBooleanOperatorPrecedence', + 'expected' => 'E', + ], + 'No colors: W: has warning' => [ + 'colors' => false, + 'code' => ' 'Generic.Commenting.Todo', + 'expected' => 'W', + ], + + 'Colors: Dot: no errors, no warnings' => [ + 'colors' => true, + 'code' => ' 'Generic.PHP.LowerCaseConstant', + 'expected' => '.', + ], + 'Colors: E: has error (red)' => [ + 'colors' => true, + 'code' => ' 'Generic.CodeAnalysis.RequireExplicitBooleanOperatorPrecedence', + 'expected' => "\033[31m".'E'."\033[0m", + ], + 'Colors: E: has fixable error (green)' => [ + 'colors' => true, + 'code' => ' 'Generic.Arrays.DisallowLongArraySyntax', + 'expected' => "\033[32m".'E'."\033[0m", + ], + 'Colors: W: has warning (yellow)' => [ + 'colors' => true, + 'code' => ' 'Generic.Commenting.Todo', + 'expected' => "\033[33m".'W'."\033[0m", + ], + 'Colors: W: has fixable warning (green)' => [ + 'colors' => true, + 'code' => ' 'Generic.CodeAnalysis.EmptyPHPStatement', + 'expected' => "\033[32m".'W'."\033[0m", + ], + ]; + + }//end dataProgressDotCs() + + + /** + * Verify the correct progress indicator is used for a file in CBF mode. + * + * @param bool $colors Whether to enable colors or not. + * @param string $code Code snippet to process. + * @param string $sniffs Comma-separated list of sniff(s) to run against the code snippet. + * @param string $expected Expected output of the progress printer. + * + * @dataProvider dataProgressDotCbf + * + * @group CBF + * + * @return void + */ + public function testProgressDotCbf($colors, $code, $sniffs, $expected) + { + if (PHP_CODESNIFFER_CBF === false) { + $this->markTestSkipped('This test needs CBF mode to run'); + } + + $this->checkProgressDot($colors, $code, $sniffs, $expected, true); + + }//end testProgressDotCbf() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataProgressDotCbf() + { + return [ + 'No colors: Dot: no errors, no warnings' => [ + 'colors' => false, + 'code' => ' 'Generic.PHP.LowerCaseConstant', + 'expected' => '.', + ], + 'No colors: F: fixes made' => [ + 'colors' => false, + 'code' => ' 'Generic.Arrays.DisallowLongArraySyntax', + 'expected' => 'F', + ], + 'No colors: E: has fixer conflict' => [ + 'colors' => false, + 'code' => ' 'Generic.Arrays.DisallowLongArraySyntax,Generic.Arrays.DisallowShortArraySyntax', + 'expected' => 'E', + ], + + 'Colors: Dot: no errors, no warnings (no color)' => [ + 'colors' => true, + 'code' => ' 'Generic.PHP.LowerCaseConstant', + 'expected' => '.', + ], + 'Colors: F: fixes made (green)' => [ + 'colors' => true, + 'code' => ' 'Generic.Arrays.DisallowLongArraySyntax', + 'expected' => "\033[32m".'F'."\033[0m", + ], + 'Colors: E: has fixer conflict (red)' => [ + 'colors' => true, + 'code' => ' 'Generic.Arrays.DisallowLongArraySyntax,Generic.Arrays.DisallowShortArraySyntax', + 'expected' => "\033[31m".'E'."\033[0m", + ], + ]; + + }//end dataProgressDotCbf() + + + /** + * Verify the correct progress indicator is used for a file in CBF mode. + * + * @param bool $colors Whether to enable colors or not. + * @param string $code Code snippet to process. + * @param string $sniffs Comma-separated list of sniff(s) to run against the code snippet. + * @param string $expected Expected output of the progress printer. + * @param bool $enableFixer Whether to fix the code or not. + * + * @return void + */ + private function checkProgressDot($colors, $code, $sniffs, $expected, $enableFixer=false) + { + $this->expectOutputString($expected); + + $config = new ConfigDouble(['-p']); + $config->colors = $colors; + $config->standards = ['Generic']; + $config->sniffs = explode(',', $sniffs); + $ruleset = new Ruleset($config); + + $runner = new Runner(); + $runner->config = $config; + + $file = new DummyFile($code, $ruleset, $config); + $file->process(); + + if ($enableFixer === true) { + $file->fixer->fixFile(); + } + + $runner->printProgress($file, 2, 1); + + }//end checkProgressDot() + + +}//end class diff --git a/tests/Core/Runner/PrintProgressTest.php b/tests/Core/Runner/PrintProgressTest.php new file mode 100644 index 0000000000..93535417e0 --- /dev/null +++ b/tests/Core/Runner/PrintProgressTest.php @@ -0,0 +1,235 @@ +standards = ['Generic']; + self::$config->sniffs = ['Generic.PHP.LowerCaseConstant']; + self::$ruleset = new Ruleset(self::$config); + + self::$runner = new Runner(); + self::$runner->config = self::$config; + + // Simple file which won't have any errors against the above sniff. + $content = 'process(); + + }//end initializeConfigRulesetRunner() + + + /** + * Reset some flags between tests. + * + * @after + * + * @return void + */ + protected function resetObjectFlags() + { + self::$config->showProgress = true; + self::$fileWithoutErrorsOrWarnings->ignored = false; + + }//end resetObjectFlags() + + + /** + * Destroy the Config object after the test to reset statics. + * + * @afterClass + * + * @return void + */ + public static function reset() + { + // Explicitly trigger __destruct() on the ConfigDouble to reset the Config statics. + // The explicit method call prevents potential stray test-local references to the $config object + // preventing the destructor from running the clean up (which without stray references would be + // automagically triggered when this object is destroyed, but we can't definitively rely on that). + self::$config->__destruct(); + + }//end reset() + + + /** + * Verify that if progress reporting is disabled, no progress dots will show. + * + * @return void + */ + public function testNoProgressIsShownWhenDisabled() + { + $this->expectOutputString(''); + + self::$config->showProgress = false; + + for ($i = 1; $i <= 10; $i++) { + self::$runner->printProgress(self::$fileWithoutErrorsOrWarnings, 3, $i); + } + + }//end testNoProgressIsShownWhenDisabled() + + + /** + * Verify ignored files will be marked with an "S" for "skipped". + * + * @return void + */ + public function testProgressDotSkippedFiles() + { + $nrOfFiles = 10; + $this->expectOutputString('.S.S.S.S.S 10 / 10 (100%)'.PHP_EOL); + + for ($i = 1; $i <= $nrOfFiles; $i++) { + if (($i % 2) === 0) { + self::$fileWithoutErrorsOrWarnings->ignored = true; + } else { + self::$fileWithoutErrorsOrWarnings->ignored = false; + } + + self::$runner->printProgress(self::$fileWithoutErrorsOrWarnings, $nrOfFiles, $i); + } + + }//end testProgressDotSkippedFiles() + + + /** + * Verify the handling of the summary at the end of each line. + * + * @param int $nrOfFiles The number of files in the scan. + * @param string $expected The expected progress information output. + * + * @dataProvider dataEndOfLineSummary + * + * @return void + */ + public function testEndOfLineSummary($nrOfFiles, $expected) + { + $this->expectOutputString($expected); + + for ($i = 1; $i <= $nrOfFiles; $i++) { + self::$runner->printProgress(self::$fileWithoutErrorsOrWarnings, $nrOfFiles, $i); + } + + }//end testEndOfLineSummary() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataEndOfLineSummary() + { + $fullLineOfDots = str_repeat('.', 60); + + // phpcs:disable Squiz.Strings.ConcatenationSpacing.PaddingFound -- Favour test readability. + return [ + 'Less than 60 files (23)' => [ + 'nrOfFiles' => 23, + 'expected' => str_repeat('.', 23).' 23 / 23 (100%)'.PHP_EOL, + ], + 'Exactly 60 files' => [ + 'nrOfFiles' => 60, + 'expected' => $fullLineOfDots.' 60 / 60 (100%)'.PHP_EOL, + ], + 'Between 60 and 120 files (71)' => [ + 'nrOfFiles' => 71, + 'expected' => $fullLineOfDots.' 60 / 71 (85%)'.PHP_EOL + .str_repeat('.', 11).str_repeat(' ', 49).' 71 / 71 (100%)'.PHP_EOL, + ], + 'More than 120 files (162)' => [ + 'nrOfFiles' => 162, + 'expected' => $fullLineOfDots.' 60 / 162 (37%)'.PHP_EOL + .$fullLineOfDots.' 120 / 162 (74%)'.PHP_EOL + .str_repeat('.', 42).str_repeat(' ', 18).' 162 / 162 (100%)'.PHP_EOL, + ], + // More than anything, this tests that the padding of the numbers is handled correctly. + 'More than 1000 files (1234)' => [ + 'nrOfFiles' => 1234, + 'expected' => $fullLineOfDots.' 60 / 1234 (5%)'.PHP_EOL + .$fullLineOfDots.' 120 / 1234 (10%)'.PHP_EOL + .$fullLineOfDots.' 180 / 1234 (15%)'.PHP_EOL + .$fullLineOfDots.' 240 / 1234 (19%)'.PHP_EOL + .$fullLineOfDots.' 300 / 1234 (24%)'.PHP_EOL + .$fullLineOfDots.' 360 / 1234 (29%)'.PHP_EOL + .$fullLineOfDots.' 420 / 1234 (34%)'.PHP_EOL + .$fullLineOfDots.' 480 / 1234 (39%)'.PHP_EOL + .$fullLineOfDots.' 540 / 1234 (44%)'.PHP_EOL + .$fullLineOfDots.' 600 / 1234 (49%)'.PHP_EOL + .$fullLineOfDots.' 660 / 1234 (53%)'.PHP_EOL + .$fullLineOfDots.' 720 / 1234 (58%)'.PHP_EOL + .$fullLineOfDots.' 780 / 1234 (63%)'.PHP_EOL + .$fullLineOfDots.' 840 / 1234 (68%)'.PHP_EOL + .$fullLineOfDots.' 900 / 1234 (73%)'.PHP_EOL + .$fullLineOfDots.' 960 / 1234 (78%)'.PHP_EOL + .$fullLineOfDots.' 1020 / 1234 (83%)'.PHP_EOL + .$fullLineOfDots.' 1080 / 1234 (88%)'.PHP_EOL + .$fullLineOfDots.' 1140 / 1234 (92%)'.PHP_EOL + .$fullLineOfDots.' 1200 / 1234 (97%)'.PHP_EOL + .str_repeat('.', 34).str_repeat(' ', 26).' 1234 / 1234 (100%)'.PHP_EOL, + ], + ]; + // phpcs:enable + + }//end dataEndOfLineSummary() + + +}//end class From 0632c4c2a91dc7072a8318e21e82807d49d6b34b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 12 Apr 2025 18:44:55 +0200 Subject: [PATCH 2/2] Runner::printProgress(): minor refactor Reduce the complexity of the method somewhat and improve the readability. --- src/Runner.php | 63 ++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/src/Runner.php b/src/Runner.php index d8b17c25f3..d527ea575e 100644 --- a/src/Runner.php +++ b/src/Runner.php @@ -856,9 +856,14 @@ public function printProgress(File $file, $numFiles, $numProcessed) return; } + $showColors = $this->config->colors; + $colorOpen = ''; + $progressDot = '.'; + $colorClose = ''; + // Show progress information. if ($file->ignored === true) { - echo 'S'; + $progressDot = 'S'; } else { $errors = $file->getErrorCount(); $warnings = $file->getWarningCount(); @@ -870,27 +875,19 @@ public function printProgress(File $file, $numFiles, $numProcessed) // Files with unfixable errors or warnings are E (red). // Files with no errors or warnings are . (black). if ($fixable > 0) { - if ($this->config->colors === true) { - echo "\033[31m"; - } + $progressDot = 'E'; - echo 'E'; - - if ($this->config->colors === true) { - echo "\033[0m"; + if ($showColors === true) { + $colorOpen = "\033[31m"; + $colorClose = "\033[0m"; } } else if ($fixed > 0) { - if ($this->config->colors === true) { - echo "\033[32m"; - } + $progressDot = 'F'; - echo 'F'; - - if ($this->config->colors === true) { - echo "\033[0m"; + if ($showColors === true) { + $colorOpen = "\033[32m"; + $colorClose = "\033[0m"; } - } else { - echo '.'; }//end if } else { // Files with errors are E (red). @@ -899,39 +896,35 @@ public function printProgress(File $file, $numFiles, $numProcessed) // Files with fixable warnings are W (green). // Files with no errors or warnings are . (black). if ($errors > 0) { - if ($this->config->colors === true) { + $progressDot = 'E'; + + if ($showColors === true) { if ($fixable > 0) { - echo "\033[32m"; + $colorOpen = "\033[32m"; } else { - echo "\033[31m"; + $colorOpen = "\033[31m"; } - } - - echo 'E'; - if ($this->config->colors === true) { - echo "\033[0m"; + $colorClose = "\033[0m"; } } else if ($warnings > 0) { - if ($this->config->colors === true) { + $progressDot = 'W'; + + if ($showColors === true) { if ($fixable > 0) { - echo "\033[32m"; + $colorOpen = "\033[32m"; } else { - echo "\033[33m"; + $colorOpen = "\033[33m"; } - } - echo 'W'; - - if ($this->config->colors === true) { - echo "\033[0m"; + $colorClose = "\033[0m"; } - } else { - echo '.'; }//end if }//end if }//end if + echo $colorOpen.$progressDot.$colorClose; + $numPerLine = 60; if ($numProcessed !== $numFiles && ($numProcessed % $numPerLine) !== 0) { return;