Fix router reload regression in integration testing. #12695
Conversation
|
looks good to me, just needs a test |
|
I tried adding a test, but I cant reproduce the issue apps or plugins are facing. I need help here to finalize this PR for the upcoming patch release. |
|
As proof, I have dereuromark/cakephp-sandbox#14 which shows this fixes the currently broken master tests ("Tests: 65, Assertions: 286, Errors: 4"). |
| @@ -43,7 +44,8 @@ public function setUp() | |||
| static::setAppNamespace(); | |||
|
|
|||
| Router::reload(); | |||
| Router::scope('/', function ($routes) { | |||
| Router::extensions(['json']); | |||
There was a problem hiding this comment.
Because routes are added in the setup method I don't think the router would be reloaded as routes would not be automatically loaded.
There was a problem hiding this comment.
Do you want to change anything on that test case?
Feel free to add a commit on top with desired changes.
There was a problem hiding this comment.
It'll be ok. I think this code won't be needed in 4.x, or at least I hope it isn't.
I wasn't entirely happy with the solution in #12695 and wanted to try an alternate approach. This set of changes adds state to fewer places and doesn't require plugins to opt-in to backwards compatible behavior. Instead it introduces new internal methods to reset router state that can cause conflicts/duplicate work. Global extensions and default route classes are not reset. Refs #12695 Refs #12577
Alternative to #12628
It provides a now "BC way" to shim it back for the use cases where we need it.
Not sure that is the best solution, but it would provide a way for integration tests to fix the regression manually where needed via
per class.