From b07428d4601f1745fa857b8fe95a382a0cd14e84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcel=20M=C3=BCller?= Date: Sun, 16 Aug 2026 16:07:07 +0200 Subject: [PATCH] test: Fix overwriting and restoring config values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Müller --- .../UserMigration/AccountMigratorTest.php | 9 +- tests/lib/TestCase.php | 37 ++++++++ tests/lib/TestCaseTest.php | 91 +++++++++++++++++++ 3 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 tests/lib/TestCaseTest.php diff --git a/apps/settings/tests/UserMigration/AccountMigratorTest.php b/apps/settings/tests/UserMigration/AccountMigratorTest.php index f1df03343f0..b8f35f65ac3 100644 --- a/apps/settings/tests/UserMigration/AccountMigratorTest.php +++ b/apps/settings/tests/UserMigration/AccountMigratorTest.php @@ -13,9 +13,7 @@ use OCA\Settings\UserMigration\AccountMigrator; use OCP\Accounts\IAccountManager; use OCP\AppFramework\App; use OCP\IAvatarManager; -use OCP\IConfig; use OCP\IUserManager; -use OCP\Server; use OCP\UserMigration\IExportDestination; use OCP\UserMigration\IImportSource; use PHPUnit\Framework\Constraint\JsonMatches; @@ -46,7 +44,7 @@ class AccountMigratorTest extends TestCase { $app = new App(Application::APP_ID); $container = $app->getContainer(); - $container->get(IConfig::class)->setSystemValue('has_internet_connection', false); + $this->overwriteSystemConfig('has_internet_connection', false); $this->userManager = $container->get(IUserManager::class); $this->avatarManager = $container->get(IAvatarManager::class); @@ -57,11 +55,6 @@ class AccountMigratorTest extends TestCase { $this->output = $this->createMock(OutputInterface::class); } - protected function tearDown(): void { - Server::get(IConfig::class)->setSystemValue('has_internet_connection', true); - parent::tearDown(); - } - public static function dataImportExportAccount(): array { return array_map( static function (string $filename): array { diff --git a/tests/lib/TestCase.php b/tests/lib/TestCase.php index 0c90b7fb917..2f5b34fea62 100644 --- a/tests/lib/TestCase.php +++ b/tests/lib/TestCase.php @@ -47,10 +47,13 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { /** @psalm-suppress ImpureStaticProperty */ private static bool $wasDatabaseAllowed = false; protected array $services = []; + /** Original values keyed by config key; null means the key was unset. */ + private array $systemConfigValues = []; #[\Override] protected function onNotSuccessfulTest(\Throwable $t): never { $this->restoreAllServices(); + $this->restoreAllSystemConfig(); // restore database connection if (!$this->IsDatabaseAccessAllowed()) { @@ -114,6 +117,39 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { } } + /** + * Sets a system config value for the duration of the test, restoring the + * previous one in tearDown. System config is persisted to config.php, so a + * leaked value would outlive the whole run. + */ + protected function overwriteSystemConfig(string $key, mixed $value): void { + $config = Server::get(IConfig::class); + + if (!array_key_exists($key, $this->systemConfigValues)) { + $this->systemConfigValues[$key] = $config->getSystemValue($key, null); + } + + $config->setSystemValue($key, $value); + } + + public function restoreAllSystemConfig(): void { + if ($this->systemConfigValues === []) { + return; + } + + $config = Server::get(IConfig::class); + foreach ($this->systemConfigValues as $key => $value) { + // null reads back as the default, so remove the key instead. + if ($value === null) { + $config->deleteSystemValue($key); + } else { + $config->setSystemValue($key, $value); + } + } + + $this->systemConfigValues = []; + } + protected function getTestTraits(): array { $traits = []; $class = $this; @@ -161,6 +197,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { #[\Override] protected function tearDown(): void { $this->restoreAllServices(); + $this->restoreAllSystemConfig(); // restore database connection if (!$this->IsDatabaseAccessAllowed()) { diff --git a/tests/lib/TestCaseTest.php b/tests/lib/TestCaseTest.php new file mode 100644 index 00000000000..81d9c68ca67 --- /dev/null +++ b/tests/lib/TestCaseTest.php @@ -0,0 +1,91 @@ +config = Server::get(IConfig::class); + $this->config->deleteSystemValue(self::KEY); + } + + #[\Override] + protected function tearDown(): void { + parent::tearDown(); + + $this->config->deleteSystemValue(self::KEY); + } + + public function testOverwriteSetsTheValue(): void { + $this->overwriteSystemConfig(self::KEY, 'overwritten'); + + $this->assertSame('overwritten', $this->config->getSystemValue(self::KEY)); + } + + public function testRestoreRemovesAPreviouslyUnsetKey(): void { + $this->overwriteSystemConfig(self::KEY, 'overwritten'); + $this->restoreAllSystemConfig(); + + $this->assertSame('fallback', $this->config->getSystemValue(self::KEY, 'fallback')); + } + + public function testRestoreReturnsThePreviousValue(): void { + $this->config->setSystemValue(self::KEY, 'original'); + + $this->overwriteSystemConfig(self::KEY, 'overwritten'); + $this->restoreAllSystemConfig(); + + $this->assertSame('original', $this->config->getSystemValue(self::KEY)); + } + + public function testRestoreReturnsThePreviousValueAfterRepeatedOverwrites(): void { + $this->config->setSystemValue(self::KEY, 'original'); + + $this->overwriteSystemConfig(self::KEY, 'first'); + $this->overwriteSystemConfig(self::KEY, 'second'); + $this->restoreAllSystemConfig(); + + $this->assertSame('original', $this->config->getSystemValue(self::KEY)); + } + + public function testRestoreIsIdempotent(): void { + $this->config->setSystemValue(self::KEY, 'original'); + + $this->overwriteSystemConfig(self::KEY, 'overwritten'); + $this->restoreAllSystemConfig(); + $this->config->setSystemValue(self::KEY, 'set afterwards'); + $this->restoreAllSystemConfig(); + + $this->assertSame('set afterwards', $this->config->getSystemValue(self::KEY)); + } + + /** false is falsy but set: an isset-based check would wrongly delete the key. */ + public function testRestoreReturnsAPreviousFalseValue(): void { + $this->config->setSystemValue(self::KEY, false); + + $this->overwriteSystemConfig(self::KEY, true); + $this->restoreAllSystemConfig(); + + $this->assertFalse($this->config->getSystemValue(self::KEY, 'fallback')); + } +}