From 8f7b3fb43db41ac04277e82496f1506e5e2435c0 Mon Sep 17 00:00:00 2001 From: "John Paul E. Balandan, CPA" Date: Sun, 12 Apr 2026 23:00:12 +0800 Subject: [PATCH] fix: refactor inconsistent behavior on `CLI::write()` and `CLI::error()` --- system/CLI/CLI.php | 13 +++++-- tests/system/CLI/CLITest.php | 26 +++++++++++--- tests/system/CLI/CommandsTest.php | 2 ++ .../system/Commands/Cache/ClearCacheTest.php | 7 ++-- .../Database/ShowTableInfoMockIOTest.php | 36 ++++++++++++------- .../Commands/Generators/TestGeneratorTest.php | 16 ++++----- .../Housekeeping/ClearDebugbarTest.php | 9 +++-- .../Commands/Housekeeping/ClearLogsTest.php | 28 +++++++++++---- .../Commands/Utilities/ConfigCheckTest.php | 33 ++++++++--------- 9 files changed, 118 insertions(+), 52 deletions(-) diff --git a/system/CLI/CLI.php b/system/CLI/CLI.php index d5980bb9bc19..f01124ec9075 100644 --- a/system/CLI/CLI.php +++ b/system/CLI/CLI.php @@ -255,6 +255,7 @@ public static function prompt(string $field, $options = null, $validation = null } static::fwrite(STDOUT, $field . (trim($field) !== '' ? ' ' : '') . $extraOutput . ': '); + static::$lastWrite = 'write'; // Read the input from keyboard. $input = trim(static::$io->input()); @@ -458,7 +459,8 @@ public static function write(string $text = '', ?string $foreground = null, ?str } if (static::$lastWrite !== 'write') { - $text = PHP_EOL . $text; + $text = PHP_EOL . $text; + static::$lastWrite = 'write'; } @@ -473,13 +475,20 @@ public static function write(string $text = '', ?string $foreground = null, ?str public static function error(string $text, string $foreground = 'light_red', ?string $background = null) { // Check color support for STDERR - $stdout = static::$isColored; + $stdout = static::$isColored; + static::$isColored = static::hasColorSupport(STDERR); if ($foreground !== '' || (string) $background !== '') { $text = static::color($text, $foreground, $background); } + if (static::$lastWrite !== 'write') { + $text = PHP_EOL . $text; + + static::$lastWrite = 'write'; + } + static::fwrite(STDERR, $text . PHP_EOL); // return STDOUT color support diff --git a/tests/system/CLI/CLITest.php b/tests/system/CLI/CLITest.php index 3dc3a20c43fe..6a69cf7e69ab 100644 --- a/tests/system/CLI/CLITest.php +++ b/tests/system/CLI/CLITest.php @@ -37,6 +37,7 @@ protected function setUp(): void Services::injectMock('superglobals', new Superglobals()); + CLI::reset(); CLI::init(); } @@ -393,16 +394,33 @@ public function testError(): void { CLI::error('test'); - // red expected cuz stderr - $expected = "\033[1;31mtest\033[0m" . PHP_EOL; + $expected = PHP_EOL . "\033[1;31mtest\033[0m" . PHP_EOL; $this->assertSame($expected, $this->getStreamFilterBuffer()); } + public function testMixedWriteError(): void + { + CLI::write('test 1'); + CLI::error('test 2'); + CLI::write('test 3'); + + $this->assertSame( + <<<'EOT' + + test 1 + test 2 + test 3 + + EOT, + preg_replace('/\e\[[^m]+m/u', '', $this->getStreamFilterBuffer()), + ); + } + public function testErrorForeground(): void { CLI::error('test', 'purple'); - $expected = "\033[0;35mtest\033[0m" . PHP_EOL; + $expected = PHP_EOL . "\033[0;35mtest\033[0m" . PHP_EOL; $this->assertSame($expected, $this->getStreamFilterBuffer()); } @@ -410,7 +428,7 @@ public function testErrorBackground(): void { CLI::error('test', 'purple', 'green'); - $expected = "\033[0;35m\033[42mtest\033[0m" . PHP_EOL; + $expected = PHP_EOL . "\033[0;35m\033[42mtest\033[0m" . PHP_EOL; $this->assertSame($expected, $this->getStreamFilterBuffer()); } diff --git a/tests/system/CLI/CommandsTest.php b/tests/system/CLI/CommandsTest.php index f6a9dc1c0ff5..34bb0601da65 100644 --- a/tests/system/CLI/CommandsTest.php +++ b/tests/system/CLI/CommandsTest.php @@ -74,6 +74,7 @@ public function testRunOnUnknownCommandButWithOneAlternative(): void $this->assertSame(EXIT_ERROR, $commands->run('app:inf', [])); $this->assertSame( <<<'EOT' + Command "app:inf" not found. Did you mean this? @@ -91,6 +92,7 @@ public function testRunOnUnknownCommandButWithMultipleAlternatives(): void $this->assertSame(EXIT_ERROR, $commands->run('app:', [])); $this->assertSame( <<<'EOT' + Command "app:" not found. Did you mean one of these? diff --git a/tests/system/Commands/Cache/ClearCacheTest.php b/tests/system/Commands/Cache/ClearCacheTest.php index 5b027e04f05a..a845eccda139 100644 --- a/tests/system/Commands/Cache/ClearCacheTest.php +++ b/tests/system/Commands/Cache/ClearCacheTest.php @@ -15,6 +15,7 @@ use CodeIgniter\Cache\CacheFactory; use CodeIgniter\Cache\Handlers\FileHandler; +use CodeIgniter\CLI\CLI; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\StreamFilterTrait; use Config\Services; @@ -32,6 +33,7 @@ protected function setUp(): void { parent::setUp(); + CLI::reset(); $this->resetServices(); // Make sure we are testing with the correct handler (override injections) @@ -42,6 +44,7 @@ protected function tearDown(): void { parent::tearDown(); + CLI::reset(); $this->resetServices(); } @@ -50,7 +53,7 @@ public function testClearCacheInvalidHandler(): void command('cache:clear junk'); $this->assertSame( - "Cache driver \"junk\" is not a valid cache handler.\n", + "\nCache driver \"junk\" is not a valid cache handler.\n", preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()), ); } @@ -79,7 +82,7 @@ public function testClearCacheFails(): void command('cache:clear'); $this->assertSame( - "Error while clearing the cache.\n", + "\nError while clearing the cache.\n", preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()), ); } diff --git a/tests/system/Commands/Database/ShowTableInfoMockIOTest.php b/tests/system/Commands/Database/ShowTableInfoMockIOTest.php index 0842c28ae12a..0ad012816eef 100644 --- a/tests/system/Commands/Database/ShowTableInfoMockIOTest.php +++ b/tests/system/Commands/Database/ShowTableInfoMockIOTest.php @@ -33,6 +33,8 @@ protected function setUp(): void { parent::setUp(); + CLI::reset(); + putenv('NO_COLOR=1'); CLI::init(); } @@ -41,6 +43,8 @@ protected function tearDown(): void { parent::tearDown(); + CLI::reset(); + putenv('NO_COLOR'); CLI::init(); } @@ -58,17 +62,25 @@ public function testDbTableWithInputs(): void $result = $io->getOutput(); - $expectedPattern = '/Which table do you want to see\? \[0, 1, 2, 3, 4, 5, 6, 7, 8, 9.*?\]: a -The "Which table do you want to see\?" field must be one of: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9.*?./'; - $this->assertMatchesRegularExpression($expectedPattern, $result); - - $expected = 'Data of Table "db_migrations":'; - $this->assertStringContainsString($expected, $result); - - $expectedPattern = '/\| id[[:blank:]]+\| version[[:blank:]]+\| class[[:blank:]]+\| group[[:blank:]]+\| namespace[[:blank:]]+\| time[[:blank:]]+\| batch \|/'; - $this->assertMatchesRegularExpression($expectedPattern, $result); - - // Remove MockInputOutput. - CLI::resetInputOutput(); + $this->assertMatchesRegularExpression( + '/Which table do you want to see\? \[[\d,\s]+\]\: a/', + $result, + ); + $this->assertMatchesRegularExpression( + '/The "Which table do you want to see\?" field must be one of: [\d,\s]+./', + $result, + ); + $this->assertMatchesRegularExpression( + '/Which table do you want to see\? \[[\d,\s]+\]\: 0/', + $result, + ); + $this->assertMatchesRegularExpression( + '/Data of Table "db_migrations"\:/', + $result, + ); + $this->assertMatchesRegularExpression( + '/\| id[[:blank:]]+\| version[[:blank:]]+\| class[[:blank:]]+\| group[[:blank:]]+\| namespace[[:blank:]]+\| time[[:blank:]]+\| batch \|/', + $result, + ); } } diff --git a/tests/system/Commands/Generators/TestGeneratorTest.php b/tests/system/Commands/Generators/TestGeneratorTest.php index 71225847cfce..3cad6b214307 100644 --- a/tests/system/Commands/Generators/TestGeneratorTest.php +++ b/tests/system/Commands/Generators/TestGeneratorTest.php @@ -33,9 +33,6 @@ protected function setUp(): void parent::setUp(); $this->resetStreamFilterBuffer(); - - putenv('NO_COLOR=1'); - CLI::init(); } protected function tearDown(): void @@ -44,14 +41,16 @@ protected function tearDown(): void $this->clearTestFiles(); $this->resetStreamFilterBuffer(); + } - putenv('NO_COLOR'); - CLI::init(); + private function getUndecoratedBuffer(): string + { + return preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()); } private function clearTestFiles(): void { - preg_match('/File created: (.*)/', $this->getStreamFilterBuffer(), $result); + preg_match('/File created: (.*)/', $this->getUndecoratedBuffer(), $result); $file = str_replace('ROOTPATH' . DIRECTORY_SEPARATOR, ROOTPATH, $result[1] ?? ''); if (is_file($file)) { @@ -71,7 +70,7 @@ public function testGenerateTestFiles(string $name, string $expectedClass): void $expectedTestFile = str_replace('/', DIRECTORY_SEPARATOR, sprintf('%stests/%s.php', ROOTPATH, $expectedClass)); $expectedMessage = sprintf('File created: %s', str_replace(ROOTPATH, 'ROOTPATH' . DIRECTORY_SEPARATOR, $expectedTestFile)); - $this->assertStringContainsString($expectedMessage, $this->getStreamFilterBuffer()); + $this->assertStringContainsString($expectedMessage, $this->getUndecoratedBuffer()); $this->assertFileExists($expectedTestFile); } @@ -93,6 +92,7 @@ public static function provideGenerateTestFiles(): iterable public function testGenerateTestWithEmptyClassName(): void { $expectedFile = ROOTPATH . 'tests/FooTest.php'; + CLI::reset(); try { $io = new MockInputOutput(); @@ -106,7 +106,7 @@ public function testGenerateTestWithEmptyClassName(): void $expectedOutput .= 'The "Test class name" field is required.' . PHP_EOL; $expectedOutput .= 'Test class name : Foo' . PHP_EOL . PHP_EOL; $expectedOutput .= 'File created: ROOTPATH/tests/FooTest.php' . PHP_EOL . PHP_EOL; - $this->assertSame($expectedOutput, $io->getOutput()); + $this->assertSame($expectedOutput, preg_replace('/\e\[[^m]+m/', '', $io->getOutput())); $this->assertFileExists($expectedFile); } finally { if (is_file($expectedFile)) { diff --git a/tests/system/Commands/Housekeeping/ClearDebugbarTest.php b/tests/system/Commands/Housekeeping/ClearDebugbarTest.php index bb14906bcff9..3925eae6a033 100644 --- a/tests/system/Commands/Housekeeping/ClearDebugbarTest.php +++ b/tests/system/Commands/Housekeeping/ClearDebugbarTest.php @@ -13,6 +13,7 @@ namespace CodeIgniter\Commands\Housekeeping; +use CodeIgniter\CLI\CLI; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\StreamFilterTrait; use PHPUnit\Framework\Attributes\Group; @@ -35,6 +36,8 @@ protected function setUp(): void command('debugbar:clear'); $this->resetStreamFilterBuffer(); + CLI::reset(); + $this->time = time(); $this->createDummyDebugbarJson(); } @@ -44,6 +47,8 @@ protected function tearDown(): void command('debugbar:clear'); $this->resetStreamFilterBuffer(); + CLI::reset(); + parent::tearDown(); } @@ -70,7 +75,7 @@ public function testClearDebugbarWorks(): void $this->assertFileDoesNotExist(WRITEPATH . 'debugbar' . DIRECTORY_SEPARATOR . "debugbar_{$this->time}.json"); $this->assertFileExists(WRITEPATH . 'debugbar' . DIRECTORY_SEPARATOR . 'index.html'); $this->assertSame( - "Debugbar cleared.\n", + "\nDebugbar cleared.\n", preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()), ); } @@ -111,7 +116,7 @@ public function testClearDebugbarWithError(): void $this->assertFileExists($path); $this->assertSame( - "Error deleting the debugbar JSON files.\n", + "\nError deleting the debugbar JSON files.\n", preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()), ); } diff --git a/tests/system/Commands/Housekeeping/ClearLogsTest.php b/tests/system/Commands/Housekeeping/ClearLogsTest.php index 70406956d66e..38c247ec8d90 100644 --- a/tests/system/Commands/Housekeeping/ClearLogsTest.php +++ b/tests/system/Commands/Housekeeping/ClearLogsTest.php @@ -41,6 +41,8 @@ protected function setUp(): void command('logs:clear --force'); $this->resetStreamFilterBuffer(); + CLI::reset(); + $this->createDummyLogFiles(); } @@ -78,7 +80,7 @@ public function testClearLogsUsingForce(): void $this->assertFileDoesNotExist(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log"); $this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . 'index.html'); - $this->assertSame("Logs cleared.\n", preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer())); + $this->assertSame("\nLogs cleared.\n", preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer())); } public function testClearLogsAbortsClearWithoutForce(): void @@ -94,11 +96,12 @@ public function testClearLogsAbortsClearWithoutForce(): void $this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log"); $this->assertSame( <<<'EOT' + Are you sure you want to delete the logs? [n, y]: n Deleting logs aborted. If you want, use the "--force" option to force delete all log files. EOT, - preg_replace('/\e\[[^m]+m/', '', $io->getOutput(2) . $io->getOutput(3)), + preg_replace('/\e\[[^m]+m/', '', $io->getOutput()), ); } @@ -110,16 +113,19 @@ public function testClearLogsAbortsClearWithoutForceWithDefaultAnswer(): void $io->setInputs(['']); CLI::setInputOutput($io); + $space = ' '; + command('logs:clear'); $this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log"); $this->assertSame( - <<<'EOT' + <<getOutput(2) . $io->getOutput(3)), + preg_replace('/\e\[[^m]+m/', '', $io->getOutput()), ); } @@ -134,7 +140,14 @@ public function testClearLogsWithoutForceButWithConfirmation(): void command('logs:clear'); $this->assertFileDoesNotExist(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log"); - $this->assertSame("Logs cleared.\n", preg_replace('/\e\[[^m]+m/', '', $io->getOutput(2))); + $this->assertSame( + <<<'EOT' + Are you sure you want to delete the logs? [n, y]: y + Logs cleared. + + EOT, + preg_replace('/\e\[[^m]+m/', '', $io->getOutput()), + ); } #[RequiresOperatingSystem('Darwin|Linux')] @@ -173,6 +186,9 @@ public function testClearLogsFailsOnChmodFailure(): void } $this->assertFileExists($path); - $this->assertSame("Error in deleting the logs files.\n", preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer())); + $this->assertSame( + "\nError in deleting the logs files.\n", + preg_replace('/\e\[[^m]+m/', '', $this->getStreamFilterBuffer()), + ); } } diff --git a/tests/system/Commands/Utilities/ConfigCheckTest.php b/tests/system/Commands/Utilities/ConfigCheckTest.php index 9ac1c84fa51f..d6aeba15e664 100644 --- a/tests/system/Commands/Utilities/ConfigCheckTest.php +++ b/tests/system/Commands/Utilities/ConfigCheckTest.php @@ -14,6 +14,7 @@ namespace CodeIgniter\Commands\Utilities; use Closure; +use CodeIgniter\CLI\CLI; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\StreamFilterTrait; use Config\App; @@ -31,34 +32,38 @@ final class ConfigCheckTest extends CIUnitTestCase public static function setUpBeforeClass(): void { + parent::setUpBeforeClass(); + App::$override = false; putenv('NO_COLOR=1'); CliRenderer::$cli_colors = false; - - parent::setUpBeforeClass(); } public static function tearDownAfterClass(): void { + parent::tearDownAfterClass(); + App::$override = true; putenv('NO_COLOR'); CliRenderer::$cli_colors = true; - - parent::tearDownAfterClass(); } protected function setUp(): void { - $this->resetServices(); parent::setUp(); + + $this->resetServices(); + CLI::reset(); } protected function tearDown(): void { - $this->resetServices(); parent::tearDown(); + + $this->resetServices(); + CLI::reset(); } public function testCommandConfigCheckWithNoArgumentPassed(): void @@ -67,13 +72,14 @@ public function testCommandConfigCheckWithNoArgumentPassed(): void $this->assertSame( <<<'EOF' + You must specify a Config classname. Usage: config:check Example: config:check App config:check 'CodeIgniter\Shield\Config\Auth' EOF, - str_replace("\n\n", "\n", $this->getStreamFilterBuffer()), + $this->getStreamFilterBuffer(), ); } @@ -82,7 +88,7 @@ public function testCommandConfigCheckNonexistentClass(): void command('config:check Nonexistent'); $this->assertSame( - "No such Config class: Nonexistent\n", + "\nNo such Config class: Nonexistent\n", $this->getStreamFilterBuffer(), ); } @@ -98,7 +104,7 @@ public function testConfigCheckWithKintEnabledUsesKintD(): void command('config:check App'); $this->assertSame( - $command(config('App')) . "\n", + "\n" . $command(config('App')) . "\n", preg_replace('/\s+Config Caching: \S+/', '', $this->getStreamFilterBuffer()), ); } @@ -110,19 +116,14 @@ public function testConfigCheckWithKintDisabledUsesVarDump(): void new ConfigCheck(service('logger'), service('commands')), 'getVarDump', ); - $clean = static fn (string $input): string => trim(preg_replace( - '/(\033\[[0-9;]+m)|(\035\[[0-9;]+m)/u', - '', - $input, - )); try { Kint::$enabled_mode = false; command('config:check App'); $this->assertSame( - $clean($command(config('App'))), - $clean(preg_replace('/\s+Config Caching: \S+/', '', $this->getStreamFilterBuffer())), + "\n" . $command(config('App')), + preg_replace('/\s+Config Caching: \S+/', '', $this->getStreamFilterBuffer()), ); } finally { Kint::$enabled_mode = true;