[BUGFIX beta] Fix link-to throwing in integration tests

[simonihmig]

Fixes #15831

Fixes a regression introduced by #15788 by returning early again from routing.generateURL() when router._routerMicrolib is not present. This is the line that got lost during the refactoring.

Was not able to write a regression test for this, as e.g. in this test _routerMicrolib is correctly setup, so was not able to write a failing test. If there is a way to setup the router in the same state as in integration tests (i.e. without _routerMicrolib), then I would be happy for any hint! 😉

Would be nice to get this into a minor beta release, as this is causing failing tests in ember-try scenarios for ember-beta now (was already failing for canary, but these were "allowed" failures. Now that canary has been promoted to beta, this is causing failed builds).

cc @rwjblue @bekzod

[rwjblue]

To add a test I think we would need to have a test module that extends from rendering test (but not application test).

Something like what you linked, but slightly different:

import { moduleFor, RenderingTest } from '../../utils/test-case';

moduleFor('Link-to component', class extends RenderingTest {
  ['@test should be able to be inserted in DOM when the router is not present - block']() {
    this.addTemplate('application', `{{#link-to 'index'}}Go to Index{{/link-to}}`);

    this.render(`{{#link-to 'index'}}Go to Index{{/link-to}}`);
    this.assertText('Go to Index');
  }

  ['@test should be able to be inserted in DOM when the router is not present - inline']() {
    this.addTemplate('application', `{{#link-to 'index'}}Go to Index{{/link-to}}`);

    this.render(`{{link-to 'Go to Index' 'index'}}`);
    this.assertText('Go to Index');
  }
});

You can add ^ to the same file, just below the current moduleFor...

[rwjblue]

Can you add a comment above this to explain why its needed?

[simonihmig]

done

[simonihmig]

You can add ^ to the same file, just below the current moduleFor...

Thanks, that was very helpful! The this.addTemplate() calls were not needed and failing, but other than that this worked fine and indeed captured the error throwing the same exception as described in #15831! 🚀

[rwjblue]

Merging here should auto publish the fix for ember-canary, but I’ll have to cherry-pick into beta branch to get a fixed ember-beta build out. Will try to do that as soon as I’m back to a computer (out at a play with the family for a bit).

[simonihmig]

Merging here should auto publish the fix for ember-canary, but I’ll have to cherry-pick into beta branch to get a fixed ember-beta build out.

FWIW this PR was targeting beta, but I see it has been cherry-picked to master already...

[rwjblue]

Ya, I missed that while reviewing + merging on mobile (apparently GH's mobile view doesn't show the target branch?!!?!?). @kategengler fixed it for us though.

[simonihmig]

@rwjblue just for clarification/learning: isn't a [BUGFIX beta] PR supposed to target the beta branch? Or should I have targeted master instead?