From 7bed8ba6524782345816336e623b71d786b0c20e Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 4 Apr 2024 16:06:01 +0200 Subject: [PATCH] Threshold report: add new `YOASTCS_THRESHOLD_EXACT_MATCH` constant The threshold report is used in the YoastCS repos in combination with a (Composer) script which overrules the exit code from PHPCS to only exit with a non-zero exit code when the _actual_ total number of errors/warnings exceeds the _expected_ number of errors/warnings. This could lead to situations where issues are being fixed in PR A, but the threshold has not been updated (i.e. errors/warnings _below_ threshold). This would not fail a build, which means this could go unnoticed and get merged. This, in turn, then allows for a subsequent PR B to introduce _new issues_ into a codebase without those new issues failing the PHPCS build, as long as the number of new issues being introduced in PR B did not exceed the number of previously _fixed_ issues from PR A. This is undesired. This commit adds a new global `YOASTCS_THRESHOLD_EXACT_MATCH` constant which can be used by the calling script to fail a build with a non-zero exit code when the _actual_ total number of errors/warnings doesn't **_exactly_** match the _expected_ number of errors/warnings. Notes: * The old constant is not being removed as that would be a breaking change and would require a major release. * I considered deprecating the old `YOASTCS_ABOVE_THRESHOLD` constant, but having both available provides the calling scripts flexibility to choose which logic to apply. If so desired, the old constant can still be deprecated in a later minor. Includes a minor tweak to make the values of the constants testable. --- Yoast/Reports/Threshold.php | 15 +++++++++++++-- Yoast/Tests/Reports/ThresholdReportTest.php | 12 ++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Yoast/Reports/Threshold.php b/Yoast/Reports/Threshold.php index 6894cd67..a1e26ba3 100644 --- a/Yoast/Reports/Threshold.php +++ b/Yoast/Reports/Threshold.php @@ -133,6 +133,7 @@ public function generate( echo \PHP_EOL; $above_threshold = false; + $below_threshold = false; if ( $totalErrors > $error_threshold ) { echo self::RED, 'Please fix any errors introduced in your code and run PHPCS again to verify.', self::RESET, \PHP_EOL; @@ -141,6 +142,7 @@ public function generate( elseif ( $totalErrors < $error_threshold ) { echo self::GREEN, 'Found less errors than the threshold, great job!', self::RESET, \PHP_EOL; echo 'Please update the ERRORS threshold in the composer.json file to ', self::GREEN, $totalErrors, '.', self::RESET, \PHP_EOL; + $below_threshold = true; } if ( $totalWarnings > $warning_threshold ) { @@ -150,6 +152,7 @@ public function generate( elseif ( $totalWarnings < $warning_threshold ) { echo self::GREEN, 'Found less warnings than the threshold, great job!', self::RESET, \PHP_EOL; echo 'Please update the WARNINGS threshold in the composer.json file to ', self::GREEN, $totalWarnings, '.', self::RESET, \PHP_EOL; + $below_threshold = true; } if ( $above_threshold === false ) { @@ -157,10 +160,18 @@ public function generate( echo 'Coding standards checks have passed!', \PHP_EOL; } - // Make the threshold comparison outcome available to the calling script. + // Make the threshold comparison outcomes available to the calling script. // The conditional define is only so as to make the method testable. - if ( \defined( 'YOASTCS_ABOVE_THRESHOLD' ) === false ) { + $exact_match = ( $above_threshold === false && $below_threshold === false ); + if ( \defined( 'PHP_CODESNIFFER_IN_TESTS' ) === false ) { \define( 'YOASTCS_ABOVE_THRESHOLD', $above_threshold ); + \define( 'YOASTCS_THRESHOLD_EXACT_MATCH', $exact_match ); + } + else { + // Only used in the tests to verify the above constants are being set correctly. + echo \PHP_EOL; + echo 'YOASTCS_ABOVE_THRESHOLD: ', ( $above_threshold === true ) ? 'true' : 'false', \PHP_EOL; + echo 'YOASTCS_THRESHOLD_EXACT_MATCH: ', ( $exact_match === true ) ? 'true' : 'false', \PHP_EOL; } } } diff --git a/Yoast/Tests/Reports/ThresholdReportTest.php b/Yoast/Tests/Reports/ThresholdReportTest.php index 138e9588..55bd29b3 100644 --- a/Yoast/Tests/Reports/ThresholdReportTest.php +++ b/Yoast/Tests/Reports/ThresholdReportTest.php @@ -121,6 +121,8 @@ public static function data_generate() { . '\\033\[33mCoding standards WARNINGS: 300/0\.\\033\[0m\s+' . '\\033\[31mPlease fix any errors introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' . '\\033\[33mPlease fix any warnings introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . 'YOASTCS_ABOVE_THRESHOLD: true\s+' + . 'YOASTCS_THRESHOLD_EXACT_MATCH: false\s+' . '`', ], 'Threshold: both errors and warnings below threshold' => [ @@ -136,6 +138,8 @@ public static function data_generate() { . '\\033\[32mFound less warnings than the threshold, great job!\\033\[0m\s+' . 'Please update the WARNINGS threshold in the composer.json file to \\033\[32m300\.\\033\[0m\s+' . 'Coding standards checks have passed!\s+' + . 'YOASTCS_ABOVE_THRESHOLD: false\s+' + . 'YOASTCS_THRESHOLD_EXACT_MATCH: false\s+' . '`', ], 'Threshold: both errors and warnings exactly at threshold' => [ @@ -147,6 +151,8 @@ public static function data_generate() { . '\\033\[32mCoding standards ERRORS: 150/150\.\\033\[0m\s+' . '\\033\[32mCoding standards WARNINGS: 300/300\.\\033\[0m\s+' . 'Coding standards checks have passed!\s+' + . 'YOASTCS_ABOVE_THRESHOLD: false\s+' + . 'YOASTCS_THRESHOLD_EXACT_MATCH: true\s+' . '`', ], 'Threshold: errors below threshold, warnings above' => [ @@ -160,6 +166,8 @@ public static function data_generate() { . '\\033\[32mFound less errors than the threshold, great job!\\033\[0m\s+' . 'Please update the ERRORS threshold in the composer\.json file to \\033\[32m150\.\\033\[0m\s+' . '\\033\[33mPlease fix any warnings introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . 'YOASTCS_ABOVE_THRESHOLD: true\s+' + . 'YOASTCS_THRESHOLD_EXACT_MATCH: false\s+' . '`', ], 'Threshold: errors above threshold, warnings below' => [ @@ -173,6 +181,8 @@ public static function data_generate() { . '\\033\[31mPlease fix any errors introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' . '\\033\[32mFound less warnings than the threshold, great job!\\033\[0m\s+' . 'Please update the WARNINGS threshold in the composer.json file to \\033\[32m300\.\\033\[0m\s+' + . 'YOASTCS_ABOVE_THRESHOLD: true\s+' + . 'YOASTCS_THRESHOLD_EXACT_MATCH: false\s+' . '`', ], 'Threshold: both errors and warnings above threshold' => [ @@ -185,6 +195,8 @@ public static function data_generate() { . '\\033\[33mCoding standards WARNINGS: 300/200\.\\033\[0m\s+' . '\\033\[31mPlease fix any errors introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' . '\\033\[33mPlease fix any warnings introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . 'YOASTCS_ABOVE_THRESHOLD: true\s+' + . 'YOASTCS_THRESHOLD_EXACT_MATCH: false\s+' . '`', ], ];