Skip to content

Commit

Permalink
Helpers\IsUnitTestTrait: bug fix - allow for custom test classes pass…
Browse files Browse the repository at this point in the history
…ed 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.
  • Loading branch information
jrfnl committed Jun 27, 2023
1 parent 7ab5947 commit 2b368bf
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 16 deletions.
74 changes: 59 additions & 15 deletions WordPress/Helpers/IsUnitTestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<string, bool>
*/
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<string, bool>
*/
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.
*
Expand Down Expand Up @@ -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 );
Expand Down
6 changes: 5 additions & 1 deletion WordPress/Tests/Files/FileNameUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<!-- Annotation has to be on the second line as errors are thrown on line 1 and errors on annotation lines are ignored. -->
phpcs:set WordPress.Files.FileName custom_test_classes[] \My_TestClass
<?php

class TestSample extends My_TestClass {}
/* phpcs:set WordPress.Files.FileName custom_test_classes[] */
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!-- Annotation has to be on the second line as errors are thrown on line 1 and errors on annotation lines are ignored. -->
phpcs:set WordPress.Files.FileName custom_test_classes[] \Plugin\Tests\My_TestClass
<?php

namespace Plugin\Tests;

class TestSample extends My_TestClass {}
/* phpcs:set WordPress.Files.FileName custom_test_classes[] */
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<!-- Annotation has to be on the second line as errors are thrown on line 1 and errors on annotation lines are ignored. -->
phpcs:set WordPress.Files.FileName custom_test_classes[] \My_TestClass
<?php

class TestSample extends \My_TestClass {}
/* phpcs:set WordPress.Files.FileName custom_test_classes[] */
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!-- Annotation has to be on the second line as errors are thrown on line 1 and errors on annotation lines are ignored. -->
phpcs:set WordPress.Files.FileName custom_test_classes[] \My_TestClass
<?php

namespace Plugin\Tests;

class TestSample extends My_TestClass {}
/* phpcs:set WordPress.Files.FileName custom_test_classes[] */

0 comments on commit 2b368bf

Please sign in to comment.