fix: ensure output buffer is closed after use of command() (#10099)

Author: paulbalandan

system/Common.php

@@ -185,10 +185,14 @@ function command(string $command)
             $params[$arg] = $value;
         }
 
-        ob_start();
-        service('commands')->run($command, $params);
+        try {
+            ob_start();
+            service('commands')->run($command, $params);
 
-        return ob_get_clean();
+            return ob_get_contents();
+        } finally {
+            ob_end_clean();
+        }
     }
 }
 

tests/_support/Commands/DestructiveCommand.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * This file is part of CodeIgniter 4 framework.
+ *
+ * (c) CodeIgniter Foundation <[email protected]>
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Tests\Support\Commands;
+
+use RuntimeException;
+
+/**
+ * @internal
+ */
+final class DestructiveCommand extends AbstractInfo
+{
+    protected $group       = 'demo';
+    protected $name        = 'app:destructive';
+    protected $description = 'This command is destructive.';
+
+    public function run(array $params): never
+    {
+        throw new RuntimeException('This command is destructive and should not be run.');
+    }
+}

tests/system/Commands/CommandTest.php

@@ -19,6 +19,7 @@
 use CodeIgniter\Test\StreamFilterTrait;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Group;
+use RuntimeException;
 use Tests\Support\Commands\AppInfo;
 use Tests\Support\Commands\ParamsReveal;
 
@@ -136,6 +137,13 @@ public function testInexistentCommandsButWithManyAlternatives(): void
         $this->assertStringContainsString(':clear', $this->getBuffer());
     }
 
+    public function testDestructiveCommandIsNotRisky(): void
+    {
+        $this->expectException(RuntimeException::class);
+
+        command('app:destructive');
+    }
+
     /**
      * @param list<string> $expected
      */

tests/system/Commands/Translation/LocalizationSyncTest.php

@@ -171,7 +171,6 @@ public function testSyncWithNullableOriginalLangValue(): void
             TEXT_WRAP;
 
         file_put_contents(self::$languageTestPath . self::$locale . '/SyncInvalid.php', $langWithNullValue);
-        ob_get_flush();
 
         $this->expectException(LogicException::class);
         $this->expectExceptionMessageMatches('/Only "array" or "string" is allowed/');
@@ -192,7 +191,6 @@ public function testSyncWithIntegerOriginalLangValue(): void
             TEXT_WRAP;
 
         file_put_contents(self::$languageTestPath . self::$locale . '/SyncInvalid.php', $langWithIntegerValue);
-        ob_get_flush();
 
         $this->expectException(LogicException::class);
         $this->expectExceptionMessageMatches('/Only "array" or "string" is allowed/');

user_guide_src/source/changelogs/v4.7.3.rst

@@ -33,6 +33,7 @@ Bugs Fixed
 **********
 
 - **Autoloader:** Fixed a bug where ``Autoloader::unregister()`` (used during tests) silently failed to remove handlers from the SPL autoload stack, causing closures to accumulate permanently.
+- **Common:** Fixed a bug where the ``command()`` helper function did not properly clean up output buffers, which could lead to risky tests when exceptions were thrown.
 
 See the repo's
 `CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_