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

Always create addon trees when developing an addon #3915

Merged
merged 3 commits into from
Apr 22, 2015

Conversation

marcioj
Copy link
Contributor

@marcioj marcioj commented Apr 16, 2015

If we create a new addon and start the server includedModules() will return an empty object, since there isn't any module, so the addon tree is skipped.
But if later we add a new file, for instance using ember g initializer foo, these files aren't included in the pipeline because no tree was created initially and tests will fail with Could not find module ember-simple-validate/initializers/foo imported from dummy/initializers/foo

Fixes #3906

@marcioj marcioj force-pushed the issue-3906 branch 3 times, most recently from 47f6b80 to f0dad58 Compare April 17, 2015 17:33
@marcioj
Copy link
Contributor Author

marcioj commented Apr 17, 2015

Travis isn't helping today: sh: 1: istanbul: not found :(
Not sure if I'm doing something wrong here

@kellyselden
Copy link
Member

I added the istanbul coverage, but I don't see anything that would cause this to stop working.

@marcioj marcioj force-pushed the issue-3906 branch 4 times, most recently from 7cc7b85 to 6b58600 Compare April 18, 2015 05:40
@marcioj
Copy link
Contributor Author

marcioj commented Apr 18, 2015

Thanks for the reply @kellyselden.
I believe this is related with the travis ci cache.
Could you please cleanup the cache for this branch and restart the build?

@stefanpenner
Copy link
Contributor

@marcioj i purged the cache, it seemed to solve this for other PR :)

@marcioj
Copy link
Contributor Author

marcioj commented Apr 18, 2015

Thanks @stefanpenner!
Unfortunately my tests aren't passing in travis, I'll check it out tonight.

@stefanpenner
Copy link
Contributor

@marcioj thanks :)

@marcioj marcioj force-pushed the issue-3906 branch 2 times, most recently from 4b65bb6 to f17d501 Compare April 19, 2015 20:17
@marcioj
Copy link
Contributor Author

marcioj commented Apr 19, 2015

After some debugging I found the issue. Looks like the process.env.EMBER_ADDON_ENV wasn't being properly cleaned in addon-test.js.
process.env.EMBER_ADDON_ENV = undefined will coerce the undefined value to a string, which will return the string 'undefined' when called. This affect the EMBER_ADDON_ENV normalization done in ember-addon.js, which will not setup the variable to 'development' because the 'undefined' string is present. And in the end will affect the Addon#isDevelopingAddon making it always return false and which make my tests fail since it's just intended to be used when the addon is being developed.
I changed addon-test.js to handle this.

@stefanpenner
Copy link
Contributor

@marcioj nice find!

`process.env.EMBER_ADDON_ENV = undefined` will store an `'undefined'` string instead
of `undefined` value. So we delete the process.env.EMBER_ADDON_ENV if `originalEnvValue`
was initially `undefined`
@marcioj
Copy link
Contributor Author

marcioj commented Apr 21, 2015

I changed the way I was closing the test server to just send a q, like we do in the terminal.
Previously using killCliProcess was causing a problem of don't killing phantomjs processes, so other tests running phantomjs will hang forever.
Unfortunately sending the q in this way will throw other error which I sent a fix to testem in testem/testem#544.
With this fix I believe all will work. So we need to wait for this patch be merged and a new testem version be released.

@johanneswuerbach
Copy link
Contributor

The fix is included in testem v0.8.2, thanks for the PR.

@marcioj
Copy link
Contributor Author

marcioj commented Apr 21, 2015

Thanks for the release @johanneswuerbach !

@stefanpenner
Copy link
Contributor

@johanneswuerbach <3

@marcioj
Copy link
Contributor Author

marcioj commented Apr 22, 2015

@stefanpenner 👍 / 👎 ?

stefanpenner added a commit that referenced this pull request Apr 22, 2015
Always create addon trees when developing an addon
@stefanpenner stefanpenner merged commit 5d3fd96 into ember-cli:master Apr 22, 2015
@marcioj marcioj deleted the issue-3906 branch April 22, 2015 18:58
rodyhaddad pushed a commit to rodyhaddad/ember-cli that referenced this pull request May 11, 2015
Always create addon trees when developing an addon
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.

[0.2.3] Unit test blueprint mistaken as actual test
4 participants