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

fix: adds setupRouter to the documentation #1651

Closed
wants to merge 1 commit into from

Conversation

brunousml
Copy link

The integration test does not load the router by default. It needs to be set up while the test is executing. It will make the documentation consistent.

The integration test does not load the router by default. It needs to be set up while the test is executing. It will make the documentation consistent.
@ijlee2
Copy link
Member

ijlee2 commented Mar 28, 2021

Hi, @brunousml. Can you provide more context (which version of Ember were you using?) and a reproduction app on GitHub?

It is correct that earlier versions of Ember require us to write this.owner.setupRouter(); in rendering tests, if we want to have the router set up URLs to make assertions, for example.

Since Ember 3.25.3, I don't think this is the case. Here are a couple of related links:

ember-learn/super-rentals-tutorial#179
ember-learn/super-rentals-tutorial#176

@brunousml
Copy link
Author

brunousml commented Apr 6, 2021 via email

@chancancode
Copy link
Contributor

@brunousml did you upgrade your ember-source version in addition to the ember-cli version?

@ijlee2
Copy link
Member

ijlee2 commented Apr 24, 2021

Hi, @brunousml. Hope you are well.

I wanted to check in to see if you can provide us your super rentals tutorial repo. Seeing the code would help me understand what might have happened, and decide if the GitHub issue needs to be worked on.

@ijlee2
Copy link
Member

ijlee2 commented May 8, 2021

Closing this issue due to a lack of response and GitHub repo for a reproducible bug.

@ijlee2 ijlee2 closed this May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants