From 3d54d4c6006f9834337059aab9fcf653b65470cf Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 1 May 2025 16:40:07 +0200 Subject: [PATCH] Config: remove more test specific conditions The `--cache` option could not be tested as the `Config` class did not allow for it to be set in a test situation. This "conditional ignore" was introduced in commit 24c7a7f7a30b38fffcaf0aecedc6882e97f5ad4e around the same time the caching feature was introduced. The commit message doesn't give a reason for introducing the conditional ignore. I can only guess why and my guess would be that it was to prevent test runs being influence by dev-user specific system-wide defaults from a `CodeSniffer.conf` file. In my opinion, that is not a good reason for keeping the "conditional ignore". * First of all, the default setting is for the cache to be _off_, so by default, tests wouldn't use the cache anyhow. * Second of all, the problem of interference from dev-user specific system-wide defaults was fixed by the introduction of the `ConfigDouble` and the `AbstractRealConfigTestCase`. All in all, I think these conditions can be safely removed. Do keep in mind though that subsequent test runs for tests involving caching may re-use the cache from an earlier run test. To prevent that, set an explicit cacheFile for the test using `--cache=` and remove this cache file after running the test(s) via the `tearDown[AfterClass]()` method. Note: one of the removed conditions also affected the `--parallel` option, but only for system-wide settings, not for CLI arguments. Related to 966 --- src/Config.php | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/Config.php b/src/Config.php index 68180859f2..1b8e7f3bcb 100644 --- a/src/Config.php +++ b/src/Config.php @@ -650,16 +650,14 @@ public function restoreDefaults() $this->colors = (bool) $colors; } - if (defined('PHP_CODESNIFFER_IN_TESTS') === false) { - $cache = self::getConfigData('cache'); - if ($cache !== null) { - $this->cache = (bool) $cache; - } + $cache = self::getConfigData('cache'); + if ($cache !== null) { + $this->cache = (bool) $cache; + } - $parallel = self::getConfigData('parallel'); - if ($parallel !== null) { - $this->parallel = max((int) $parallel, 1); - } + $parallel = self::getConfigData('parallel'); + if ($parallel !== null) { + $this->parallel = max((int) $parallel, 1); } }//end restoreDefaults() @@ -817,10 +815,8 @@ public function processLongArgument($arg, $pos) break; } - if (defined('PHP_CODESNIFFER_IN_TESTS') === false) { - $this->cache = true; - $this->overriddenDefaults['cache'] = true; - } + $this->cache = true; + $this->overriddenDefaults['cache'] = true; break; case 'no-cache': if (isset($this->overriddenDefaults['cache']) === true) { @@ -928,9 +924,7 @@ public function processLongArgument($arg, $pos) $this->exclude = $this->parseSniffCodes(substr($arg, 8), 'exclude'); $this->overriddenDefaults['exclude'] = true; - } else if (defined('PHP_CODESNIFFER_IN_TESTS') === false - && substr($arg, 0, 6) === 'cache=' - ) { + } else if (substr($arg, 0, 6) === 'cache=') { if ((isset($this->overriddenDefaults['cache']) === true && $this->cache === false) || isset($this->overriddenDefaults['cacheFile']) === true