-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
Support for npm dependencies + improved output #45
Conversation
preparation for handling npm dependencies
- Extract 'runCommand' common pattern - Various cleanup and changes that were needed for testability [Katie & Michelle]
test - Remove tasks/test and just use tasks/try-each, for a single scenario - Inject dependencyManagerAdapter & speed up some tests - Appease JSCS - Add note about some tests using a dependency manager for real - Use ember try:testall for "client" test for a smoke test - Remove dead code
- Fix test for findBowerPath
- Support multiple package managers - Tests for npm adapter that I'm *sure* won't be flaky
This looks awesome! Having This will greatly help in https://github.com/switchfly/ember-test-helpers, where the I created emberjs/ember-test-helpers#145 to use the latest bower resolved git://github.com/components/ember.git#b27df6ad05
bower resolution Unsuitable resolution declared for ember: components/ember#release
bower ECONFLICT Unable to find suitable version for ember
Error!
1
undefined I just wanted to let you know of this and give you feedback on this. I don't know how finished this PR is, so please ignore if the current state shouldn't be tried out yet... Anyway, thanks again for this! |
@pangratz Thanks for trying it out. Looks like I messed something up where the resolutions for bower are no longer set to what they are in the config, so that's a good catch. I'll try to update this tonight. |
Awesome, thank you kindly! |
- Add test at that level - Change integration tests to use config including all types of options
@pangratz Should be all set. Note that to use npm deps, you need to use the new config syntax, see the upgraded README |
Thanks for the heads up. I have good news: emberjs/ember-test-helpers#145 is all green now! Everything works as expected. 🎉 🎈 👍 |
Great! |
Support for npm dependencies + improved output
❤️ |
npm
andbower
key under each scenario, then thedependencies
anddevDependencies
(andresolutions
, for bower) under that. The old configuration format is still supported, but only forbower
dependencies.npm
dependencies, at least some way: Backs upnode_modules
andpackage.json
before any commands are run, modifiespackage.json
based on the scenario, runsnpm install
andnpm prune
, runs the command, and to cleanup, restores thepackage.json
andnode_modules
folder and runsnpm install
andnpm prune
.ember try
andember try:testall
ember-cli
Limitation: Currently packages can be removed between scenarios (thanks to
npm prune
) but packages inpackage.json
cannot be removed. This needs further thought and perhaps our own configuration syntax. This is the same as the current support forbower
.Further things to do:
ember-try
between scenarios.ember-cli
between scenarios.Important Note: When specifying packages for
npm
in scenarios, remember to put them in the same "bucket" as inpackage.json
--devDependencies
go underdevDependencies
, and so forth.