Skip to content

Commit

Permalink
Tests/ConfigDouble: bug fix - always reset Config statics after use
Browse files Browse the repository at this point in the history
The `Config` class uses a number of static properties, which may be updated during tests. These were previously - prior to 275 - reset to their default values in the `AbstractMethodUnitTest::resetTestFile()` method, but this reset was inadvertently removed with the reasoning that, if all tests use the `ConfigDouble`, the reset would no longer be needed as the `ConfigDouble` resets on being initialized.

The flaw in this logic is that not all tests are guaranteed to use the `ConfigDouble`, which means that without the reset, the `Config` class may be left "dirty" after tests using the `ConfigDouble`, which could break tests.

This commit fixes this issue by:
* Adding a `__destruct()` method to the `ConfigDouble` class which will reset the static properties on the PHPCS native `Config` class whenever an object created from this class is destroyed.
* Explicitly calling the `__destruct()` method from the `AbstractMethodUnitTest::reset()` method to ensure it is always run after a test has finished ("after class"), even if there would still be a lingering reference to the object.

This is only a stability tweak, there are no existing tests affected by this issue at this time.
  • Loading branch information
jrfnl committed Jul 27, 2024
1 parent c4c389c commit 6b7cc9d
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 0 deletions.
16 changes: 16 additions & 0 deletions tests/ConfigDouble.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ public function __construct(array $cliArgs=[], $skipSettingStandard=false, $skip
}//end __construct()


/**
* Ensures the static properties in the Config class are reset to their default values
* when the ConfigDouble is no longer used.
*
* @return void
*/
public function __destruct()
{
$this->setStaticConfigProperty('overriddenDefaults', []);
$this->setStaticConfigProperty('executablePaths', []);
$this->setStaticConfigProperty('configData', null);
$this->setStaticConfigProperty('configDataFile', null);

}//end __destruct()


/**
* Sets the command line values and optionally prevents a file system search for a custom ruleset.
*
Expand Down
8 changes: 8 additions & 0 deletions tests/Core/AbstractMethodUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ public static function initializeFile()
*/
public static function reset()
{
// Explicitly trigger __destruct() on the ConfigDouble to reset the Config statics.
// The explicit method call prevents potential stray test-local references to the $config object
// preventing the destructor from running the clean up (which without stray references would be
// automagically triggered when `self::$phpcsFile` is reset, but we can't definitively rely on that).
if (isset(self::$phpcsFile) === true) {
self::$phpcsFile->config->__destruct();
}

self::$fileExtension = 'inc';
self::$tabWidth = 4;
self::$phpcsFile = null;
Expand Down
23 changes: 23 additions & 0 deletions tests/Core/Filters/AbstractFilterTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,29 @@ public static function initializeConfigAndRuleset()
}//end initializeConfigAndRuleset()


/**
* Clean up after finished test by resetting all static properties on the Config class to their default values.
*
* Note: This is a PHPUnit cross-version compatible {@see \PHPUnit\Framework\TestCase::tearDownAfterClass()}
* method.
*
* @afterClass
*
* @return void
*/
public static function reset()
{
// Explicitly trigger __destruct() on the ConfigDouble to reset the Config statics.
// The explicit method call prevents potential stray test-local references to the $config object
// preventing the destructor from running the clean up (which without stray references would be
// automagically triggered when `self::$phpcsFile` is reset, but we can't definitively rely on that).
if (isset(self::$config) === true) {
self::$config->__destruct();
}

}//end reset()


/**
* Helper method to retrieve a mock object for a Filter class.
*
Expand Down

0 comments on commit 6b7cc9d

Please sign in to comment.