Skip to content

Commit

Permalink
Threshold report: add new YOASTCS_THRESHOLD_EXACT_MATCH constant
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jrfnl committed Apr 4, 2024
1 parent 081c3f5 commit 7bed8ba
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
15 changes: 13 additions & 2 deletions Yoast/Reports/Threshold.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 ) {
Expand All @@ -150,17 +152,26 @@ 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 ) {
echo \PHP_EOL;
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;
}
}
}
12 changes: 12 additions & 0 deletions Yoast/Tests/Reports/ThresholdReportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' => [
Expand All @@ -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' => [
Expand All @@ -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' => [
Expand All @@ -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' => [
Expand All @@ -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' => [
Expand All @@ -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+'
. '`',
],
];
Expand Down

0 comments on commit 7bed8ba

Please sign in to comment.