Bug: [4.4] Failed tests on Windows.

[iRedds]

PHP Version

7.4

CodeIgniter4 Version

4.4

CodeIgniter4 Installation Method

Git

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

No response

What happened?

After PR #7380 in Windows CodeIgniter\CodeIgniterTest::testRun404Override() and similar tests fail.

Instead of the expected response from the 404 error handler, a welcome_page is returned (from App\Controller\Home::index)

$ d:\1\CodeIgniter4\vendor\bin\phpunit.bat tests\system\CodeIgniterTest.php
PHPUnit 9.6.7 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.1
Configuration: D:\1\CodeIgniter4\phpunit.xml.dist
Warning:       No code coverage driver available

...FFFF............................                                                      35 / 35 (100%)

Time: 00:04.175, Memory: 28.00 MB

There were 4 failures:

1) CodeIgniter\CodeIgniterTest::testRun404Override
Failed asserting that '<!-- DEBUG-VIEW START 1 APPPATH\Views\welcome_message.php -->\r\n
[cut]
D:\1\CodeIgniter4\tests\system\CodeIgniterTest.php:124

2) CodeIgniter\CodeIgniterTest::testRun404OverrideControllerReturnsResponse
Failed asserting that '<!-- DEBUG-VIEW START 1 APPPATH\Views\welcome_message.php -->\r\n
[cut]
...etc

This is happening in this code snippet due to the difference in paths (directory separators) between Windows and UNIX.

CodeIgniter4/system/Router/RouteCollection.php

Lines 323 to 332 in d9fd9d5

	$files = $this->fileLocator->search('Config/Routes.php');
	foreach ($files as $file) {
	// Don't include our main file again...
	if (in_array($file, $this->routeFiles, true)) {
	continue;
	}
	include $file;
	}

$files
array(2) {
  [0]=>
  string(59) "D:\1\CodeIgniter4\app\Config\Routes.php"
  [1]=>
  string(70) "D:\1\CodeIgniter4\tests\_support\Config\Routes.php"
}

$this->routeFiles
array(1) {
  [0]=>
  string(59) "D:\1\CodeIgniter4\app\Config/Routes.php"  <--- DS
}

Due to the difference in paths, the comparison fails and the file with routes from the application is loaded, thereby adding the route that is accessed in the tests.

Expected Output

Since the framework can be used not only on UNIX, the tests must also pass on Windows. This issue is just a special case and fixing by changing the requested resource in the tests will not solve the problem. That is, the behavior should be like in UNIX, that is, the route from the application should not be loaded.

Anything else?

In other tests related to file/directory paths, there are also errors on Windows due to the directory separator.

[kenjis]

This is a bug and should be fixed.

However, I don't think this directory separator issue can be solved that easily.
To be honest, I have never used PHP on a Windows server, so I do not consider the directory separator to be a \. So, if I write the code, the code sometimes will not work like that.

[datamweb]

I'm curious, is there a specific reason for the CI team not to check code with multiple platforms via GitHub Actions?

https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs

[iRedds]

It seems to me that we can replace include with include_once and remove the check for included files.

[iRedds]

The use of include_once is possible in practice, but due to the reset of services in tests, this does not work.

[kenjis]

Using _once in this case makes it impossible to write test.
We need to create many RouteCollection instances with the different routes for testing.

It seems we need to normalize the directory separator.

[ddevsr]

By the way, can we make workflow for windows?

[paulbalandan]

By the way, can we make workflow for windows?

I'm on it.

[iRedds]

Using _once in this case makes it impossible to write test. We need to create many RouteCollection instances with the different routes for testing.

It seems we need to normalize the directory separator.

Is there a case in which the routes need to be reloaded while the application is running?
To be honest, I can't imagine such a case.
If I'm right, then the collection class is not made for optimal performance, but for convenient testing. I think that in this case it is easier to use a mock class for tests.

But here the problem is because of the Services that predefine the classes. That is, during the tests it is impossible to replace services.
Some of these situations could be solved through DI.

[lonnieezell]

But here the problem is because of the Services that predefine the classes. That is, during the tests it is impossible to replace services.
Some of these situations could be solved through DI.

$collection = new MockRouteCollection();
Services::injectMock('routes', $collection);

// in setUp or tearDown
Services::reset()

[iRedds]

@lonnieezell that is, we can replace include with include_once and use the service in tests with routes?

[lonnieezell]

that is, we can replace include with include_once and use the service in tests with routes?

Possibly, though I don't know how big the refactor would be to make that happen. That also doesn't solve the issue with them running on Windows.

The RouteCollection is injected in the Router class in the constructor, so just changing the RouteCollection class used wouldn't be enough, unless it was done prior to the the Router being instantiated the first time in each test.

I'm surprised we are running into these issues on Windows, though. I thought PHP solved that years ago so we didn't need to use that anymore. Guess not in all situations. Too bad.

[ping-yee]

But shouldn't this problem be solved with handling normalize the route path?

$this->routeFiles
array(1) {
  [0]=>
  string(59) "D:\1\CodeIgniter4\app\Config/Routes.php"  <--- DS
}

[lonnieezell]

But shouldn't this problem be solved with handling normalize the route path?

Seems like the appropriate fix to me.

[ping-yee]

Seems like the appropriate fix to me.

I have implemented the normalize work in #7487, review please.