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

[Bug] 3.26: Router is not setup in a non-application test #19494

Closed
grayt0r opened this issue Apr 11, 2021 · 6 comments
Closed

[Bug] 3.26: Router is not setup in a non-application test #19494

grayt0r opened this issue Apr 11, 2021 · 6 comments
Labels

Comments

@grayt0r
Copy link

grayt0r commented Apr 11, 2021

🐞 Describe the Bug

Ember 3.24 included a change that meant the router service automatically worked in non-application tests (i.e. no need for this.owner.setupRouter()) - this worked great in 3.25, but seems to have regressed in 3.26.

🔬 Minimal Reproduction

Run the tests in this repo: https://github.com/grayt0r/ember-router-setup-in-tests.

Relevant code changes are here.

😕 Actual Behavior

The router is not setup and the test fails with an error (the exact error depends on which router method the component calls):

Cannot read property 'generate' of undefined

🤔 Expected Behavior

The router is setup, the link is correctly rendered and the test passes.

🌍 Environment

  • Ember: - v3.26.1

➕ Additional Context

The test passes if I downgrade to 3.25.4 or if I add this.owner.setupRouter() to the test setup.

I think it might be related to this change: #19405.

/cc @Ravenstine @rwjblue

@rwjblue
Copy link
Member

rwjblue commented Apr 12, 2021

Chatted with @Ravenstine about this today, they are going to work on a failing test reproduction first so we can figure out the right way to fix...

@rwjblue rwjblue added the Bug label Apr 12, 2021
@chriskrycho
Copy link
Contributor

Hit this as a blocker for upgrading to 3.28. @Ravenstine did you ID the root cause and/or a path forward? Happy to collaborate on landing that!

@chriskrycho
Copy link
Contributor

@grayt0r I've opened #19733 to address this – do you mind trying it and seeing if it solves all of the issues you hit? I know it solves some of them (explicitly updated the tests to cover the only path I see which invokes generate), but would like to make sure it doesn't hit others.

Note that in the case of transitionTo, I have intentionally left it requiring the use of this.owner.setupRouter() because the idea that you'd invoke a transition in a rendering test (not an an application test) is… odd.

@grayt0r
Copy link
Author

grayt0r commented Sep 6, 2021

@chriskrycho sorry for the delay, but that fix appears to solve our issues, thank you very much! 🙌

@chriskrycho
Copy link
Contributor

Excellent!

@chriskrycho
Copy link
Contributor

Noticed this was still open despite the issue being resolved, just closed it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants