Testing: fix failure of testStreamFilter function.

[ping-yee]

Description
Fixed #6203

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

There was 1 failure:

1) CodeIgniter\Test\TestCaseTest::testStreamFilter
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
-first.
+'first.
 '

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Test/TestCaseTest.php:60
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:97

https://github.com/codeigniter4/CodeIgniter4/runs/8185500592?check_suite_focus=true

[ping-yee]

@kenjis I don't have this failure in my side.

./vendor/bin/phpunit tests/system/Test/TestCaseTest.php

Do I order the wrong command to run unit test or fix the wrong file?

I comment out the other function in this testing file, and this is the result I get.

[iRedds]

The first CLI::write() call adds a newline before the line.

CodeIgniter4/system/CLI/CLI.php

Lines 359 to 364 in ddee0c6

	if (static::$lastWrite !== 'write') {
	$text = PHP_EOL . $text;
	static::$lastWrite = 'write';
	}
	static::fwrite(STDOUT, $text . PHP_EOL);

It's a strange decision.

[ping-yee]

@iRedds I also notice that design so that I make that change of testing.

Probably the design can make the first line of command print look more comfortable?!

And I don't think this is a good idea to change the CLI::write design cause there are too many other classes using this class.

[kenjis]

Do I order the wrong command to run unit test or fix the wrong file?

No.

But both of the following commands should result in OK.

phpunit tests/system/Test/TestCaseTest.php
phpunit tests/system/Test/

This shows static variables are very difficult to handle.
In short, using static variables is not good design.

Speaking of the test cases, these are depending on the global state and we do not reset the global state after one test,
so the test result changes.

And I don't think this is a good idea to change the CLI::write design cause there are too many other classes using this class.

Yes, it is difficult to change the behavior because it is a breaking change.

[MGatner]

How about @runInSeparateProcess for that method?

[kenjis]

CLI::print() resets static::$lastWrite.
I think it is better to use it and reset the value.

@runInSeparateProcess makes test slower and step debugging impossible (I don't know how to do step debugging).

[ping-yee]

Whether CLI::print() is more suitable for this testing function?
It may not be effected by static::$lastWrite.

[kenjis]

How about this?

--- a/tests/system/Test/TestCaseTest.php
+++ b/tests/system/Test/TestCaseTest.php
@@ -27,6 +27,14 @@ final class TestCaseTest extends CIUnitTestCase
      */
     private $stream_filter;
 
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        // Reset CLI::$lastWrite
+        CLI::print();
+    }
+
     public function testGetPrivatePropertyWithObject()
     {
         $obj    = new __TestForReflectionHelper();

[MGatner]

How about this?

Love it! Hacky but efficient 🤗

[ping-yee]

@kenjis I have been add your suggestion code and pushed, review pls.

[kenjis]

@ping-yee See GitHub Actions results. PHPUnit failed.
It seems the code is not changed.
Did you fail the push?

[kenjis]

Please run composer cs-fix.
See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#php-style