Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing bug with legacy autoRoute when testing #7060

Merged
merged 1 commit into from
Jan 7, 2023

Conversation

baycik
Copy link
Contributor

@baycik baycik commented Jan 7, 2023

When legacy autoRoute is used in case of using controller that sits in sub directory, that directory is determined and stored in $this->directory and new AutoRouter class. But if we subsequently try to load controller without that is not in a directory, then codeigniter can't find it because the $this->directory var in AutoRoute instance is still holding reference to previous directory. This only happens when we load several controllers from console. In my case after upgrading to 4.2.11 my tests fail because of it.

I suggest to remove this return; statement to reset directory in AutoRoute instance also.

Description
Explain what you have changed, and why.

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

When legacy autoRoute is used in case of using controller that sits in sub directory, that directory is determined and stored in $this->directory and new AutoRouter class. But if we subsequently try to load controller without that is not in a directory, then codeigniter can't find it because the $this->directory var in AutoRoute instance is still holding reference to previous directory. This only happens when we load several controllers from console. In my case after upgrading to 4.2.11 my tests fail because of it.

I suggest to remove this return; statement to reset directory in AutoRoute instance also.
@kenjis
Copy link
Member

kenjis commented Jan 7, 2023

I don't understand your use case.
Can you show us minimum sample code?

And why after upgrading to 4.2.11 your tests fail?
It seems there is no change in Router in v4.2.11.

@baycik
Copy link
Contributor Author

baycik commented Jan 7, 2023

Ok I have upgraded my app from CI 4.0.x to 4.2.11. I'm using codeigniters phpunit test system. In test I mimic subsequental requests to app using FeatureTestTrait. After upgrade when I make

  1. $this->withSession($_SESSION)->post('/Admin/Tariff/storeTariffAdd',$params);
    then
  2. $this->withSession($_SESSION)->post('/User/signIn',$params);
    I get this error
  3. CodeIgniter\Exceptions\PageNotFoundException: Controller or its method is not found: \App\Controllers\Admin\User::signIn
    I have spent a lot time to trace down this bug and as I think problem is in the return statement. When I delete it test passes normally

@kenjis
Copy link
Member

kenjis commented Jan 7, 2023

Thank you for the explanation. I got the situation.
I agree with removing the return.

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Jan 7, 2023
@kenjis kenjis changed the title Fixing bug with legacy autoRoute Fixing bug with legacy autoRoute when testing Jan 7, 2023
@samsonasik samsonasik merged commit d8c925a into codeigniter4:develop Jan 7, 2023
@samsonasik
Copy link
Member

Thank you @baycik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants