From 6b7cc9d4df6dfdcd65fbcf5e685f18a45670d5f7 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Sat, 27 Jul 2024 10:46:05 +0200
Subject: [PATCH] Tests/ConfigDouble: bug fix - always reset Config statics
 after use

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.
---
 tests/ConfigDouble.php                        | 16 +++++++++++++
 tests/Core/AbstractMethodUnitTest.php         |  8 +++++++
 tests/Core/Filters/AbstractFilterTestCase.php | 23 +++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/tests/ConfigDouble.php b/tests/ConfigDouble.php
index 190f75ecef..62caaba8da 100644
--- a/tests/ConfigDouble.php
+++ b/tests/ConfigDouble.php
@@ -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.
      *
diff --git a/tests/Core/AbstractMethodUnitTest.php b/tests/Core/AbstractMethodUnitTest.php
index f98392d71b..3784fb072d 100644
--- a/tests/Core/AbstractMethodUnitTest.php
+++ b/tests/Core/AbstractMethodUnitTest.php
@@ -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;
diff --git a/tests/Core/Filters/AbstractFilterTestCase.php b/tests/Core/Filters/AbstractFilterTestCase.php
index 15dd5bcbc3..4277f8c6ed 100644
--- a/tests/Core/Filters/AbstractFilterTestCase.php
+++ b/tests/Core/Filters/AbstractFilterTestCase.php
@@ -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.
      *