fix: refactor inconsistent behavior on CLI::write() and CLI::error()

Author: paulbalandan

system/CLI/CLI.php

@@ -458,7 +458,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 +474,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

tests/system/CLI/CLITest.php

@@ -37,6 +37,7 @@ protected function setUp(): void
 
         Services::injectMock('superglobals', new Superglobals());
 
+        CLI::reset();
         CLI::init();
     }
 
@@ -393,24 +394,41 @@ 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());
     }
 
     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());
     }
 

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?

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()),
         );
     }

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,
+        );
     }
 }

tests/system/Commands/Generators/TestGeneratorTest.php

@@ -33,6 +33,7 @@ protected function setUp(): void
         parent::setUp();
 
         $this->resetStreamFilterBuffer();
+        CLI::reset();
 
         putenv('NO_COLOR=1');
         CLI::init();
@@ -44,6 +45,7 @@ protected function tearDown(): void
 
         $this->clearTestFiles();
         $this->resetStreamFilterBuffer();
+        CLI::reset();
 
         putenv('NO_COLOR');
         CLI::init();
@@ -102,7 +104,7 @@ public function testGenerateTestWithEmptyClassName(): void
             $io->setInputs(['', 'Foo']);
             command('make:test');
 
-            $expectedOutput = 'Test class name : ' . PHP_EOL;
+            $expectedOutput = 'Test class name : ' . PHP_EOL . PHP_EOL;
             $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;

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()),
         );
     }

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,6 +96,7 @@ public function testClearLogsAbortsClearWithoutForce(): void
         $this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
         $this->assertSame(
             <<<'EOT'
+
                 Deleting logs aborted.
                 If you want, use the "--force" option to force delete all log files.
 
@@ -115,6 +118,7 @@ public function testClearLogsAbortsClearWithoutForceWithDefaultAnswer(): void
         $this->assertFileExists(WRITEPATH . 'logs' . DIRECTORY_SEPARATOR . "log-{$this->date}.log");
         $this->assertSame(
             <<<'EOT'
+
                 Deleting logs aborted.
                 If you want, use the "--force" option to force delete all log files.
 
@@ -134,7 +138,7 @@ 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("\nLogs cleared.\n", preg_replace('/\e\[[^m]+m/', '', $io->getOutput(2)));
     }
 
     #[RequiresOperatingSystem('Darwin|Linux')]
@@ -173,6 +177,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()),
+        );
     }
 }