From 2b368bf7df359013e969a9bcecb85da588c509fb Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 27 Jun 2023 23:45:14 +0200 Subject: [PATCH] Helpers\IsUnitTestTrait: bug fix - allow for custom test classes passed as FQN While looking at an old PR (1960), I realized that custom test classes passed as FQN were not handled correctly by the trait. This code was not adjusted in that PR, just moved, so this bug has existed for a while. The problem was in this code snippet: ```php $known_test_classes = RulesetPropertyHelper::merge_custom_array( $this->custom_test_classes, $this->known_test_classes ); /* * Show some tolerance for user input. * The custom test class names should be passed as FQN without a prefixing `\`. */ foreach ( $known_test_classes as $k => $v ) { $known_test_classes[ $k ] = ltrim( $v, '\\' ); } ``` After the merge of the custom classes, the `$known_test_classes` array is in `string` => `bool` format, so the `foreach()` loop is absolutely not having the intended effect. The only thing it does, is change the boolean value to strings... I also noticed that the array merge was being done every single time the `is_test_class()` method was being called. We can make that a bit more efficient as it will be rare that the list of custom tests classes changes during a run. So, I have made the following changes: * Introduce a new `get_all_test_classes()` method which will handle the cleaning of the custom names and the merge. This new class uses two new properties `$added_custom_test_classes` and `$all_test_classes` to prevent having to do the same merge over and over again. * The `$known_test_classes` property has now been made `private`. * The logic to trim namespace separators of custom test class names has been moved to before the merge as we don't need to trim the names from the `$known_test_classes`. * The logic to trim namespace separators will now work correctly as the value of the `$custom_test_classes` will be `string[]` and hasn't been "flipped" yet (keys/values reversed). Includes additional unit tests. --- WordPress/Helpers/IsUnitTestTrait.php | 74 +++++++++++++++---- WordPress/Tests/Files/FileNameUnitTest.php | 6 +- ...unit.inc => test-sample-custom-unit.1.inc} | 0 .../TestFiles/test-sample-custom-unit.2.inc | 6 ++ .../TestFiles/test-sample-custom-unit.3.inc | 8 ++ .../TestFiles/test-sample-custom-unit.4.inc | 6 ++ .../TestFiles/test-sample-custom-unit.5.inc | 8 ++ 7 files changed, 92 insertions(+), 16 deletions(-) rename WordPress/Tests/Files/FileNameUnitTests/TestFiles/{test-sample-custom-unit.inc => test-sample-custom-unit.1.inc} (100%) create mode 100644 WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.2.inc create mode 100644 WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.3.inc create mode 100644 WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.4.inc create mode 100644 WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.5.inc diff --git a/WordPress/Helpers/IsUnitTestTrait.php b/WordPress/Helpers/IsUnitTestTrait.php index e4a0869b92..85439c40ee 100644 --- a/WordPress/Helpers/IsUnitTestTrait.php +++ b/WordPress/Helpers/IsUnitTestTrait.php @@ -73,12 +73,13 @@ trait IsUnitTestTrait { * directory of WP Core.} * * @since 0.11.0 - * @since 3.0.0 Moved from the Sniff class to this dedicated Trait. - * Renamed from `$test_class_whitelist` to `$known_test_classes`. + * @since 3.0.0 - Moved from the Sniff class to this dedicated Trait. + * - Renamed from `$test_class_whitelist` to `$known_test_classes`. + * - Visibility changed from protected to private. * * @var string[] */ - protected $known_test_classes = array( + private $known_test_classes = array( // Base test cases. 'WP_UnitTestCase' => true, 'WP_UnitTestCase_Base' => true, @@ -100,6 +101,60 @@ trait IsUnitTestTrait { 'TestCase' => true, ); + /** + * Cache of previously added custom test classes. + * + * Prevents having to do the same merges over and over again. + * + * @since 3.0.0 + * + * @var string[] + */ + private $added_custom_test_classes = array(); + + /** + * Combined list of WP/PHPUnit native and custom test classes. + * + * @since 3.0.0 + * + * @var array + */ + private $all_test_classes = array(); + + /** + * Retrieve a list of all registered test classes, both WP/PHPUnit native as well as custom. + * + * @since 3.0.0 + * + * @var array + */ + protected function get_all_test_classes() { + if ( array() === $this->all_test_classes + || $this->custom_test_classes !== $this->added_custom_test_classes + ) { + /* + * Show some tolerance for user input. + * The custom test class names should be passed as FQN without a prefixing `\`. + */ + $custom_test_classes = array(); + if ( ! empty( $this->custom_test_classes ) ) { + foreach ( $this->custom_test_classes as $v ) { + $custom_test_classes[] = ltrim( $v, '\\' ); + } + } + + $this->all_test_classes = RulesetPropertyHelper::merge_custom_array( + $custom_test_classes, + $this->known_test_classes + ); + + // Store the original value so the comparison can succeed. + $this->added_custom_test_classes = $this->custom_test_classes; + } + + return $this->all_test_classes; + } + /** * Check if a class token is part of a unit test suite. * @@ -127,18 +182,7 @@ protected function is_test_class( File $phpcsFile, $stackPtr ) { } // Add any potentially extra custom test classes to the known test classes list. - $known_test_classes = RulesetPropertyHelper::merge_custom_array( - $this->custom_test_classes, - $this->known_test_classes - ); - - /* - * Show some tolerance for user input. - * The custom test class names should be passed as FQN without a prefixing `\`. - */ - foreach ( $known_test_classes as $k => $v ) { - $known_test_classes[ $k ] = ltrim( $v, '\\' ); - } + $known_test_classes = $this->get_all_test_classes(); // Is the class/trait one of the known test classes ? $namespace = Namespaces::determineNamespace( $phpcsFile, $stackPtr ); diff --git a/WordPress/Tests/Files/FileNameUnitTest.php b/WordPress/Tests/Files/FileNameUnitTest.php index 13909316cf..0e8c48579f 100644 --- a/WordPress/Tests/Files/FileNameUnitTest.php +++ b/WordPress/Tests/Files/FileNameUnitTest.php @@ -75,7 +75,11 @@ final class FileNameUnitTest extends AbstractSniffUnitTest { 'test-sample-phpunit.inc' => 0, 'test-sample-phpunit6.inc' => 0, 'test-sample-wpunit.inc' => 0, - 'test-sample-custom-unit.inc' => 0, + 'test-sample-custom-unit.1.inc' => 0, + 'test-sample-custom-unit.2.inc' => 0, + 'test-sample-custom-unit.3.inc' => 0, + 'test-sample-custom-unit.4.inc' => 0, + 'test-sample-custom-unit.5.inc' => 1, // Namespaced vs non-namespaced. 'test-sample-namespaced-declaration.1.inc' => 0, 'test-sample-namespaced-declaration.2.inc' => 1, // Namespaced vs non-namespaced. 'test-sample-namespaced-declaration.3.inc' => 1, // Wrong namespace. diff --git a/WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.inc b/WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.1.inc similarity index 100% rename from WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.inc rename to WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.1.inc diff --git a/WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.2.inc b/WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.2.inc new file mode 100644 index 0000000000..41170aa93d --- /dev/null +++ b/WordPress/Tests/Files/FileNameUnitTests/TestFiles/test-sample-custom-unit.2.inc @@ -0,0 +1,6 @@ + +phpcs:set WordPress.Files.FileName custom_test_classes[] \My_TestClass + +phpcs:set WordPress.Files.FileName custom_test_classes[] \Plugin\Tests\My_TestClass + +phpcs:set WordPress.Files.FileName custom_test_classes[] \My_TestClass + +phpcs:set WordPress.Files.FileName custom_test_classes[] \My_TestClass +