Fix options are not passed to Command $params

[kenjis]

Description
Fixes #3400

When you run the following command:

$ php spark test param1 param2 -opt1 opt1val

$params of the Test command would be:

Array
(
    [0] => param1
    [1] => param2
    [opt1] => opt1val
)

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]

I can't fix the test: https://github.com/codeigniter4/CodeIgniter4/pull/5206/files#diff-547eef440769bc0c1aac5c4f0a08ee7a180baa78a52bf5eb6d624fb0a77c04aaL81
It causes the error: Serialization of 'Closure' is not allowed

[kenjis]

Another test error:

There was 1 error:

1) CodeIgniter\CodeIgniterTest::testControllersRunFilterByClassName
ErrorException: Undefined property: CodeIgniter\HTTP\IncomingRequest::$url

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/CodeIgniterTest.php:186
/home/runner/work/CodeIgniter4/CodeIgniter4/system/CodeIgniter.php:780
/home/runner/work/CodeIgniter4/CodeIgniter4/system/CodeIgniter.php:397
/home/runner/work/CodeIgniter4/CodeIgniter4/system/CodeIgniter.php:318
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/CodeIgniterTest.php:193

It is true, IncomingRequest::$url is not defined.

[paulbalandan]

Use runTestInSeparateProcess or runTestsInSeparateProcess (for class level) to fix Serialization error

[kenjis]

@paulbalandan

Use runTestInSeparateProcess or runTestsInSeparateProcess (for class level) to fix Serialization error

No, this is the opposite case. Adding @runInSeparateProcess caused the Serialization error.

I separated the test method to another test case, and fixed it.

[paulbalandan]

I meant @runTestInSeparateProcess.

diff --git a/tests/system/CLI/ConsoleTest.php b/tests/system/CLI/ConsoleTest.php
index a5b3dd1e9..389bbe129 100644
--- a/tests/system/CLI/ConsoleTest.php
+++ b/tests/system/CLI/ConsoleTest.php
@@ -78,6 +78,9 @@ final class ConsoleTest extends CIUnitTestCase
         $this->assertSame('', $result);
     }

+    /**
+     * @runTestInSeparateProcess
+     */
     public function testRun()
     {
         $request = new CLIRequest(config('App'));

PS C:\Users\P\Desktop\Web Dev\CodeIgniter4> vendor/bin/phpunit --filter ConsoleTest
PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.0.11 with Xdebug 3.0.4
Configuration: C:\Users\P\Desktop\Web Dev\CodeIgniter4\phpunit.xml.dist

....                                                                4 / 4 (100%)

Time: 00:05.281, Memory: 74.00 MB

OK (4 tests, 5 assertions)

Generating code coverage report in Clover XML format ... done [00:06.127]

[kenjis]

In my understanding @runTestsInSeparateProcesses is class level.
https://phpunit.readthedocs.io/en/9.5/annotations.html#runtestsinseparateprocesses
So I put it in the class phpdoc.

As you say, @runTestsInSeparateProcesses can solve the Serialization error.

[paulbalandan]

Yes. runTests is class level. but runTest (singular) is per test level.

[kenjis]

No, it wasn't.

There are only @runTestsInSeparateProcesses and @runInSeparateProcess.
And @runTestsInSeparateProcesses is valid only in class doc comment.
@runInSeparateProcess is valid only in method doc comment.

They are just comment, so if we have one typo, it is just ignored.

Without them, all tests in ConsoleTest pass, but the constant SPARKED is defined.
To remove the constant, we have to separate the process.

[kenjis]

When I put only @runTestsInSeparateProcesses in ConsoleTest, Serialization of 'Closure' is not allowed.
Because global $routes contains Closure.

When I put @runTestsInSeparateProcesses and @preserveGlobalState disabled, stty: stdin isn't a terminal.
On GitHub Action:

There was 1 error:

1) CodeIgniter\CLI\ConsoleTest::testRun
PHPUnit\Framework\Exception: stty: 'standard input': Inappropriate ioctl for device

I can't write test for this PR.

[MGatner]

Paul has much more recent experience with this but I can hop in and take a look if needed.

[kenjis]

The problem is running ConsoleTest::testRun() in separate process causes stty: 'standard input': Inappropriate ioctl for device or stty: stdin isn't a terminal error.

I have no idea to solve it.

[paulbalandan]

As far as I remember, calling script -e -c in GHA already solved that ioctl problem. It seems either adding the SPARKED constant or mocking the request caused this. Have you tried enabling only one at a time to see which is the problem?

[kenjis]

@paulbalandan

It seems either adding the SPARKED constant or mocking the request caused this.

No. The request is not a mock. I just changed $this->app->setRequest() to Services::injectMock().
Adding @runTestsInSeparateProcesses caused this.

Adding @runTestsInSeparateProcesses and @preserveGlobalState disabled to
ConsoleTest on develop causes this error.

[kenjis]

Updated completely.

[MGatner]

Looks like maybe this needs to be rebased? Showing some of the work from the Context PR, unless I'm mistaken.

[kenjis]

No, this is up to date.

[MGatner]

Gotcha. The context comment on CodeIgniter threw me off.