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

Addons can pull in test trees #1453

Merged
merged 1 commit into from
Jul 30, 2014
Merged

Addons can pull in test trees #1453

merged 1 commit into from
Jul 30, 2014

Conversation

bcardarella
Copy link
Contributor

The use case for this is with ember-cli-proxy-fixtures. I need to add additional QUnit functionality to the test suite to serve up proxy fixtures. Anything in test-support/ will get pulled into the test free

@rwjblue
Copy link
Member

rwjblue commented Jul 23, 2014

I am 👍 on this (it should be possible to have a test tree from an addon).

I'm not really sure that an addon should be adding new tests of its own, but it should definitely be allowed to add test helpers and such (which this makes easier).

@bcardarella
Copy link
Contributor Author

@rwjblue correct, I don't advocate for adding tests either. Although it would now be possible, it should not be encouraged

@jdjkelly
Copy link
Contributor

Hmm, so I've been building out addons with their own unit tests. Are you suggesting those should instead be written on the app in which the addon is being included?

@bcardarella
Copy link
Contributor Author

@jdjkelly no, this is for pulling helpers from an addon into your test suite. For example, my ember-cli-proxy-fixtures addon will need to concat onto tests.js with the proxy fixture data and loader.

@jdjkelly
Copy link
Contributor

Wait, I think I misunderstood the PR. Addons can continue to have its own test - but those would not be injected into application in which the addon is included.

EDIT: Got it. Thanks for the clarification.

@bcardarella
Copy link
Contributor Author

@jdjkelly correct, the structure of addons needs some thought for how the treeFor function will be written for general use. IMO your addon's tests should go into tests/ and we clearly do not want to pull this tree into your app's testsTree.

@bcardarella
Copy link
Contributor Author

@rwjblue @stefanpenner anything holding this up?

@rwjblue
Copy link
Member

rwjblue commented Jul 30, 2014

@bcardarella - Not sure why, but this is failing the test I added in #1522. It could just be that you need to rebase and force push (as it doesn't appear that this is changing that behavior at all).

@bcardarella
Copy link
Contributor Author

@rwjblue I just rebased again but I actually rebased against master last night. Not sure why its failing, Npm reports:

npm ERR! weird error 1

Never seen that before

@stefanpenner
Copy link
Contributor

@bcardarella thats a trolling error if i ever saw one.

@bcardarella
Copy link
Contributor Author

@stefanpenner I think I found the answer, seems the failing test was in the smoke test and not regular npm test. My latest push should fix things

stefanpenner added a commit that referenced this pull request Jul 30, 2014
@stefanpenner stefanpenner merged commit 5353747 into ember-cli:master Jul 30, 2014
@bcardarella bcardarella deleted the bc-test-addon-trees branch July 30, 2014 17:43
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 this pull request may close these issues.

4 participants