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

Tricky to use moduleForComponent for components with sub-components #74

Closed
tomdale opened this issue Jul 14, 2014 · 8 comments · Fixed by #153
Closed

Tricky to use moduleForComponent for components with sub-components #74

tomdale opened this issue Jul 14, 2014 · 8 comments · Fixed by #153

Comments

@tomdale
Copy link
Member

tomdale commented Jul 14, 2014

If you want to test a component that uses subcomponents, you have to know to do a special needs incantation to include both the template and the component. Here's an example from Skylight:

moduleForComponent("billing-trial-status", "billing-trial-status component", {
  needs: [
    'component:calendar-date',
    'template:components/calendar-date',
    'helper:format-date'
  ]
});

I'm opposed exposing the container key like this anyway, but having to know how components are represented internally is definitely too much to ask new users, IMO. I'd like to see some sugar for specifying subcomponents that are required.

@ryanflorence
Copy link
Contributor

Yes, all my component tests are this way. Any suggestions? You have to register things on the container somewhere.

Sent from my iPhone

On Jul 14, 2014, at 5:46 PM, Tom Dale notifications@github.com wrote:

If you want to test a component that uses subcomponents, you have to know to do a special needs incantation to include both the template and the component. Here's an example from Skylight:

moduleForComponent("billing-trial-status", "billing-trial-status component", {
needs: [
'component:calendar-date',
'template:components/calendar-date',
'helper:format-date'
]
});
I'm opposed exposing the container key like this anyway, but having to know how components are represented internally is definitely too much to ask new users, IMO. I'd like to see some sugar for specifying subcomponents that are required.


Reply to this email directly or view it on GitHub.

@fivetanley
Copy link
Member

I wanted to bring this up on discourse. It seems like ember-qunit should have a mode where it uses App.__container__. The boilerplate is kind of just boilerplate and serves no real value other than explicitly stating dependencies in tests. I don't think you can, for example, provide an alternate helper implementation in the setup.

@workmanw
Copy link

I'd love to hear any thoughts on this topic. Our needs for some components are getting rather long and tedious. For example, if your component uses an {{each}} you have to do: this.container.register('controller:array', Ember.ArrayController); in your setup function. And if your each happens to use an item controller, you also have to inject that item controller.

I'm not sure what the right solution here is, but it definitely feels that manually declaring ember's internal dependencies isn't it.

@stefanpenner
Copy link
Member

i think the solution is to have a mode for these tests that are not isolated, rather pull in the entire app + non isolated container.

@workmanw
Copy link

@stefanpenner I assume that includes actually starting the app, or at the very least running the application initializers. We've often had to manually invoke initializers in some cases to get the test bootstrapped. I would agree though, this feels like the right approach.

@stefanpenner
Copy link
Member

@workmanw basically the current helpers nail unit tests, but as soon as you need collaborators you are integration testing land, and these helpers leave you wanting for much more. This is a known limitation, and the hope was to continue to flesh out that aspect of these helpers but time wasn't fair to me :P

@workmanw
Copy link

I completely understand (and know the feeling). Thanks for the response.

@ro-ka
Copy link

ro-ka commented Feb 23, 2015

Are there any updates on this?

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

Successfully merging a pull request may close this issue.

6 participants