From baa18f4c22317662365cc02f7bffc5918d073b7e Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 21 Jul 2020 23:12:07 +0200 Subject: [PATCH 1/3] Ruleset test script: fail the build on failing ruleset tests The builds on PHP 5.4, 5.5 and 5.6 should have been failing on the ruleset test for a while now, but aren't. Example: https://travis-ci.org/github/Automattic/VIP-Coding-Standards/jobs/710513865#L311-L315 What is happening here is that when several commands are run via a script, Travis will only look at the exit code of the last script to decide whether the build should be failed or not. This means that failures from the `WordPressVIPMinimum` ruleset test were never failing the build as they were hidden by a passing ruleset test for the `WordPress-VIP-Go` ruleset. By chaining the commands together, the exit codes of both commands are taken into account when determining whether the build should be failed or not. --- bin/ruleset-tests | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bin/ruleset-tests b/bin/ruleset-tests index 2da29def..6f0f5aa9 100755 --- a/bin/ruleset-tests +++ b/bin/ruleset-tests @@ -17,5 +17,4 @@ PHPCS_BIN="$(pwd)/vendor/bin/phpcs" export PHPCS_BIN -php ./WordPressVIPMinimum/ruleset-test.php -php ./WordPress-VIP-Go/ruleset-test.php +php ./WordPressVIPMinimum/ruleset-test.php && php ./WordPress-VIP-Go/ruleset-test.php From 1a421d49deb149e9f0caaf5eac98286f0a550d47 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 21 Jul 2020 23:26:52 +0200 Subject: [PATCH 2/3] WordPressVIPMinimum: fix the failing ruleset test Ok, this is a fun one. Line 5 of the ruleset-test is not supposed to error. It is just setting up the variables for the VariableAnalysis test. Line 7, however, is supposed to error on a PHP parse error (`Generic.PHP.Syntax`). The above is true and working for PHP 7. However on PHP 5, line 7 will not throw a parse error, while line 5 actually will - `Cannot re-assign $this`. To fix this, I've: * Replaced the `$this` variable assignment on line 5 with an assignment to `$stdClass`. * Replaced the PHP 7 parse error on line 7 with a parse error which will error on all currently known PHP versions. Fun times ;-) --- WordPressVIPMinimum/ruleset-test.inc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc index 56df28dc..78d46ea9 100644 --- a/WordPressVIPMinimum/ruleset-test.inc +++ b/WordPressVIPMinimum/ruleset-test.inc @@ -2,9 +2,9 @@ endpoint, array( +wp_remote_post( $stdClass->endpoint, array( 'method' => 'POST', 'timeout' => 45, // Error. 'httpversion' => '1.1', 'blocking' => false, - 'body' => wp_json_encode( $this->logs, JSON_UNESCAPED_SLASHES ), + 'body' => wp_json_encode( $stdClass->logs, JSON_UNESCAPED_SLASHES ), ) ); From 4aed540d928aae6030042142b57636ebfddd0268 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 21 Jul 2020 23:39:21 +0200 Subject: [PATCH 3/3] RulesetTest: fix the test script to be able to handle 0 values Turns out the test script was not set up to be able to deal with `0` values and would throw a very informative "_Expected 0 errors, found 0 on line 7._" error in such a case. Fixed now. --- tests/RulesetTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/RulesetTest.php b/tests/RulesetTest.php index 0272ca5b..bf21dd7c 100644 --- a/tests/RulesetTest.php +++ b/tests/RulesetTest.php @@ -230,6 +230,10 @@ private function check_missing_expected_values() { } foreach ( $lines as $line_number => $expected_count_of_type_violations ) { + if ( 0 === $expected_count_of_type_violations ) { + continue; + } + if ( ! isset( $this->{$type}[ $line_number ] ) ) { $this->error_warning_message( $expected_count_of_type_violations, $type, 0, $line_number ); } elseif ( $this->{$type}[ $line_number ] !== $expected_count_of_type_violations ) { @@ -247,6 +251,10 @@ private function check_missing_expected_values() { private function check_unexpected_values() { foreach ( [ 'errors', 'warnings' ] as $type ) { foreach ( $this->$type as $line_number => $actual_count_of_type_violations ) { + if ( 0 === $actual_count_of_type_violations ) { + continue; + } + if ( ! isset( $this->expected[ $type ][ $line_number ] ) ) { $this->error_warning_message( 0, $type, $actual_count_of_type_violations, $line_number ); } elseif ( $actual_count_of_type_violations !== $this->expected[ $type ][ $line_number ] ) {