Bug: using `resetServices()` in tests cause named routes to fail

[michalsn]

PHP Version

8.2

CodeIgniter4 Version

4.4.1 and develop

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

macOS

Which server did you use?

cli

Database

irrelevant

What happened?

Using resetServices() in tests causes named routes to fail.

Steps to Reproduce

Routes.php

$routes->get('sample-url-1', 'HomeController::index', ['as' => 'sample1']);

Sample.php - sample library

namespace App\Libraries;

class Sample
{
    public function urlTo($route)
    {
        return url_to($route);
    }

    public function routeTo($route)
    {
        return route_to($route);
    }
}

SampleTest.php - sample test

use App\Libraries\Sample;
use CodeIgniter\Test\CIUnitTestCase;

/**
 * @internal
 */
final class SampleTest extends CIUnitTestCase
{
    protected function setUp(): void
    {
        $this->resetServices();

        parent::setUp();
    }

    public function testSampleUrlTo()
    {
        $sample = new Sample();
        $this->assertInstanceOf(Sample::class, $sample);

        $this->assertSame(
            'http://example.com/index.php/sample-url-1',
            $sample->urlTo('sample1')
        );
    }

    public function testSampleRouteTo()
    {
        $sample = new Sample();
        $this->assertInstanceOf(Sample::class, $sample);

        $this->assertSame('/sample-url-1', $sample->routeTo('sample1'));
    }
}

Expected Output

Passing tests.

Anything else?

If we comment out the line with $this->resetServices();, then all tests will pass.

[kenjis]

In the constructor of RouteCollection, the loadRoutes() method should be called?

https://github.com/codeigniter4/CodeIgniter4/blob/83b64b4f9949ecdf1f351832b0e29391205e6e2c/system/Router/RouteCollection.php#L286

https://github.com/codeigniter4/CodeIgniter4/blob/83b64b4f9949ecdf1f351832b0e29391205e6e2c/system/Router/RouteCollection.php#L317

[michalsn]

That would solve the problem, but I've never done anything related to routes, so I don't know how such a change would affect the rest of the code.

[kenjis]

In normal use cases, there is no change because loadRoutes() should be called (and is called now) right after the instantiation of RouteCollection.

But the existing tests for the framework would fail, because some tests expect the RouteCollection does not have any route right after the instantiation.

[kenjis]

I found a bug #8024 in a test and the framework code. It may be better to change to call loadRoutes() in the constructor. But many existing tests for the framework fail, so it seems better to change it in 4.5 just to be on the safe side

[michalsn]

How about adding loadRoutes() to the resetServices()? Would that help with failing framework tests?

[kenjis]

Oh, that might be better. As for RouteCollection, I think it would be preferable to have no route at instantiation.

[kenjis]

Oh, it doesn't work. Services::routes()->loadRoutes() creates the Request instance because RouteCollection depends on Request. And hack like $_SERVER['HTTP_HOST'] = 'adm.example.com'; for testing does not work in the test method, because the Request is already created in setUp().

[kenjis]

As you probably already know, the workaround is to call Services::routes()->loadRoutes() after resetServices() in the test code.

[michalsn]

Yeah... maybe we should just promote this as a permanent solution. Not sure what others think about it.

[kenjis]

This is not a bug, and you need to call loadRoutes() if you need. I sent a PR like that: #8047

[michalsn]

I can live with that, thanks!