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

Generated tests for mixin use the wrong path when the recipe is used in addons #4803

Closed
MiguelMadero opened this issue Sep 6, 2015 · 6 comments

Comments

@MiguelMadero
Copy link
Contributor

Steps to reproduce

  1. Create a new addon (ember addon ember-addon)
  2. cd ember-addon
  3. Generate a mixin ember g mixin sample
  4. Run the tests ember test

Expected:

  • The default test should succeed (or be incomplete)

Actual:

  • The test fails with the message "Error: Could not find module dummy/mixins/sample imported from `dummy/tests/unit/mixins/sample-test".

The import statement used is import SampleMixin from '../../../mixins/sample'; When replaced with import SampleMixin from 'ember-addon/mixins/sample'; it works.

I could fix it by simply using modulePrefix on the import path, but I want to make sure I'm considering everything.

@kellyselden
Copy link
Member

Just came across this, it's because the mixin isn't re-exported in the app folder like other blueprints are.

@MiguelMadero
Copy link
Contributor Author

Well, I don't think the addon needs to re-export the mixin. That makes sense for other types of objects, but IMO, it's better it the consumer import's it via ES6 from the add-ons namespace. For other types of objects, re-exporting is important so they can be found by the resolver.

@rwjblue
Copy link
Member

rwjblue commented Jan 22, 2016

Confirm, it should be imported from addon-name/mixins/whatever-lol

@MiguelMadero
Copy link
Contributor Author

Thanks. PR coming

@kellyselden
Copy link
Member

Well, I don't think the addon needs to re-export the mixin.

Agreed. I didn't mean to imply we should, just that that was probably a copy paste issue of this test blueprint.

@MiguelMadero
Copy link
Contributor Author

Makes sense. Thanks for clarifying. 

homu added a commit that referenced this issue Jan 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants