Merge pull request #62 from alleyinteractive/feature/issue-46/compare-stub-errors-with-expected

Issue-46: Improve tests to match the error reported for the stub files with the expected error

Author: anubisthejackle

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 This project adheres to [Keep a CHANGELOG](https://keepachangelog.com/en/1.0.0/).
 
+### 2.3.3
+- Changed: Improves tests to ensure tests expected to fail only fail for expected reasons.
+
 ### 2.3.2
 
 - Lock `wp-coding-standards/wpcs` to 3.1.0 to avoid issues with the latest version.

README.md

@@ -73,11 +73,19 @@ references these rules, like this:
 When contributing to this project, modifications to the ruleset should have a
 corresponding test in the `tests` directory. For the most part, this takes the
 form of a passing test in `tests/fixtures/pass` and a failing one in
-`tests/fixtures/fail`. You can run the tests with `composer phpunit`. If you
+`tests/fixtures/fail`.
+
+You can run the tests with `composer phpunit`. If you
 want to run PHPCS against the test fixtures, you can run
 `composer phpcs:fixtures` to ensure that what is passing/failing matches your
-expectations. For failing fixtures in `tests/fixtures/fail`, we recommend
+expectations.
+
+### Special Considerations for failure expectations
+For failing fixtures in `tests/fixtures/fail`, we recommend
 keeping the files smaller and focused on the specific sniff being tested.
+A corresponding file should be created in `tests/fixtures/fail/expectations`
+referencing the expected source of the failures for that test file. Name the file
+the same as you do the fixture (e.g. `tests/fixtures/fail/foo.php` and `tests/fixtures/fail/expectations/foo.php`).
 
 ## Changelog
 

composer.json

@@ -30,6 +30,7 @@
     "phpcbf": "phpcbf --standard=Alley-Interactive tests/*Test.php",
     "phpcs": "phpcs --standard=Alley-Interactive tests/*Test.php -sn",
     "phpcs:fixtures": "phpcs -sn --standard=Alley-Interactive tests/fixtures",
+    "phpcs:expectations": "phpcs -sn --standard=Alley-Interactive tests/fixtures/fail/expectations",
     "phpunit": "phpunit",
     "test": [
       "@phpcs",

tests/FixtureTest.php

@@ -32,10 +32,22 @@ public function test_passing_fixtures( string $file ): void {
 	 * Test that fixtures fail.
 	 *
 	 * @param string $file The file to test.
+	 * @param string $expectation The expectations file for this test.
 	 */
 	#[DataProvider( 'failing_fixture_data_provider' )]
-	public function test_failing_fixtures( string $file ): void {
-		$this->process_phpcs_output( $this->run_phpcs( $file ), expect_to_fail: true );
+	public function test_failing_fixtures( string $file, ?string $expectation = null ): void {
+
+		$expectations = [];
+
+		if ( ! empty( $expectation ) && file_exists( $expectation ) ) {
+			$expectations = include $expectation;
+		}
+
+		$this->process_phpcs_output(
+			$this->run_phpcs( $file ),
+			ignored_errors: $expectations,
+			expect_to_fail: true
+		);
 	}
 
 	/**
@@ -53,7 +65,19 @@ public static function passing_fixture_data_provider(): array {
 	 * @return array<array<string|array<string>>>
 	 */
 	public static function failing_fixture_data_provider(): array {
-		return self::get_files_in_directory( __DIR__ . '/fixtures/fail' );
+		$data         = self::get_files_in_directory( __DIR__ . '/fixtures/fail' );
+		$expectations = self::get_files_in_directory( __DIR__ . '/fixtures/fail/expectations' );
+
+		foreach ( $data as $key => $data_set ) {
+			if ( ! isset( $expectations[ $key ] ) ) {
+				$data[ $key ][] = null;
+				continue;
+			}
+
+			$data[ $key ][] = $expectations[ $key ][0];
+		}
+
+		return $data;
 	}
 
 	/**
@@ -76,6 +100,10 @@ protected static function get_files_in_directory( string $directory ): array {
 		$data = [];
 
 		foreach ( $files as $file ) {
+			if ( is_dir( $file ) ) {
+				continue;
+			}
+
 			$data[ basename( $file ) ] = [ $file ];
 		}
 

tests/PhpcsHelper.php

@@ -76,13 +76,6 @@ protected function process_phpcs_output(
 			);
 
 			if ( ! empty( $unexpected_messages ) ) {
-				// Bail if we expect the test to fail and there are errors.
-				if ( $expect_to_fail ) {
-					$this->assertTrue( true );
-
-					return;
-				}
-
 				$this->fail(
 					sprintf(
 						'Unexpected errors found in %s: %s',
@@ -104,8 +97,12 @@ protected function process_phpcs_output(
 			),
 		);
 
-		if ( $expect_to_fail ) {
+		if ( $expect_to_fail && empty( $errors ) ) {
 			$this->fail( 'Test did not fail as expected' );
+		} else if ( $expect_to_fail && ! empty( $errors ) ) {
+			$this->assertNotEmpty( $errors );
+		} else {
+			$this->assertEmpty( $errors );
 		}
 	}
 }

tests/fixtures/fail/expectations/array.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * Array expectations file.
+ *
+ * @package Alley\WP\Coding_Standards
+ */
+
+/**
+ * Return the array of expected failures that this test file should generate.
+ */
+return [
+	'Generic.Arrays.DisallowLongArraySyntax.Found',
+];

tests/fixtures/fail/expectations/functions-empty-arguments.php

@@ -0,0 +1,14 @@
+<?php
+/**
+ * Function Empty Arguments expectations file.
+ *
+ * @package Alley\WP\Coding_Standards
+ */
+
+/**
+ * Return the array of expected failures that this test file should generate.
+ */
+return [
+	'PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket',
+	'PSR2.Methods.FunctionCallSignature.SpaceBeforeCloseBracket',
+];

tests/fixtures/fail/expectations/functions-space-arguments.php

@@ -0,0 +1,14 @@
+<?php
+/**
+ * Functions expectations file.
+ *
+ * @package Alley\WP\Coding_Standards
+ */
+
+/**
+ * Return the array of expected failures that this test file should generate.
+ */
+return [
+	'PSR2.Methods.FunctionCallSignature.SpaceAfterOpenBracket',
+	'PSR2.Methods.FunctionCallSignature.SpaceBeforeCloseBracket',
+];

tests/fixtures/fail/expectations/one-object-structure.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * Function Empty Arguments expectations file.
+ *
+ * @package Alley\WP\Coding_Standards
+ */
+
+/**
+ * Return the array of expected failures that this test file should generate.
+ */
+return [
+	'Generic.Files.OneObjectStructurePerFile.MultipleFound',
+];

tests/fixtures/fail/expectations/shorthand-tag.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * Shorthand Tag expectations file.
+ *
+ * @package Alley\WP\Coding_Standards
+ */
+
+/**
+ * Return the array of expected failures that this test file should generate.
+ */
+return [
+	'Alley.PHP.DisallowShortEchoTag.Found',
+];