Merge pull request #19733 from chriskrycho/router-setup-for-tests

[BUGFIX release] fix router test regression in urlFor and recognize
emberjs · Aug 30, 2021 · f828888 · f828888
2 parents 70e95ea + 9f61c7a
commit f828888
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 4 deletions.
diff --git a/packages/@ember/-internals/routing/lib/services/router.ts b/packages/@ember/-internals/routing/lib/services/router.ts
@@ -241,6 +241,7 @@ export default class RouterService extends Service {
      @public
    */
   urlFor(routeName: string, ...args: any[]) {
+    this._router.setupRouter();
     return this._router.generate(routeName, ...args);
   }
 
@@ -376,6 +377,7 @@ export default class RouterService extends Service {
       `You must pass a url that begins with the application's rootURL "${this.rootURL}"`,
       url.indexOf(this.rootURL) === 0
     );
+    this._router.setupRouter();
     let internalURL = cleanURL(url, this.rootURL);
     return this._router._routerMicrolib.recognize(internalURL);
   }
@@ -396,6 +398,7 @@ export default class RouterService extends Service {
       `You must pass a url that begins with the application's rootURL "${this.rootURL}"`,
       url.indexOf(this.rootURL) === 0
     );
+    this._router.setupRouter();
     let internalURL = cleanURL(url, this.rootURL);
     return this._router._routerMicrolib.recognizeAndLoad(internalURL);
   }

diff --git a/packages/ember/tests/routing/router_service_test/non_application_test_test.js b/packages/ember/tests/routing/router_service_test/non_application_test_test.js
@@ -66,14 +66,15 @@ moduleFor(
     }
 
     ['@test RouterService#urlFor returns url'](assert) {
-      let router = this.owner.lookup('router:main');
-      router.setupRouter();
       assert.equal(this.routerService.urlFor('parent.child'), '/child');
     }
 
     ['@test RouterService#transitionTo with basic route'](assert) {
       assert.expect(2);
 
+      // Callers who want to actually execute a transition in a non-application
+      // test are doing something weird and therefore should do
+      // `owner.setupRouter()` explicitly in their tests.
       let componentInstance;
       let router = this.owner.lookup('router:main');
       router.setupRouter();
@@ -107,8 +108,6 @@ moduleFor(
     }
 
     ['@test RouterService#recognize recognize returns routeInfo'](assert) {
-      let router = this.owner.lookup('router:main');
-      router.setupRouter();
       let routeInfo = this.routerService.recognize('/dynamic-with-child/123/1?a=b');
       assert.ok(routeInfo);
       let { name, localName, parent, child, params, queryParams, paramNames } = routeInfo;