Replace ember-cli-test-loader Bower with NPM dependency

[alexlafroscia]

I tried this with a demo app, by

• generating a new Ember app
• Installing ember-cli-mocha (this branch)
• Removing the ember-cli-test-loader from the package.json and reinstalling all the dependencies, so that the test loader must have come from the shouldIncludeChildAddon hook
• Ran ember test in the demo addon

Everything works as you'd expect.

I did run into the problem that the tests for this repo are not passing after this change. For some reason, requiredModules doesn't include valid.jshint. Any idea why that might be?

Closes #122

[Turbo87]

I tend to like this approach better than ember-cli/ember-cli-qunit@115d56b ...

@trentmwillis any comments?

we should probably remove ember-cli-test-loader from the app package.json though if we have it as a dependency ourselves.

[alexlafroscia]

we should probably remove ember-cli-test-loader from the app package.json though if we have it as a dependency ourselves.

Yeah, that's true. We could check for/remove it after install. Since everyone in the Ember CLI space is using this or ember-cli-qunit it doesn't really make sense to me that the app has a dependency on the test loader at all if the framework addons can provide it themselves.

[Turbo87]

@alexlafroscia can you explain why this hook is needed? it seems like the default implementation returns true in all cases and this hook is mainly meant to filter addons out instead of explicitly including them.

[alexlafroscia]

Maybe we don't need it? I'm not sure actually. Happy to remove it -- just wasn't sure what the right approach was and had seen it done like this in another add-on.

[Turbo87]

if it works fine without it then we should probably remove it

[alexlafroscia]

Updated my PR to remove this hook.

[Turbo87]

I don't think the typings file was supposed to end up here...

[alexlafroscia]

Ahh, yup. Accidentally committed those VS Code files, removed them but missed that one. Fixed it.

[trentmwillis]

Finally got around to looking at this. The change makes sense to me, not even sure what I was thinking with the one from ember-cli-qunit now that I look back at it. Moving it to be a straight dependency would allow us to ditch the blueprint in ember-cli-qunit altogether.

[alexlafroscia]

That's the idea! To be fair to you, I think that ember-cli-test-loader was only just recently added to NPM so using Bower like this was the right approach. But, we can do better now!

Once this lands, I'm going to figure out how to pull in ember-mocha-adapter through NPM. I feel like it's a bad experience for a user to install an addon and then have their package.json or bower.json polluted with additional dependencies. It makes it hard to remove and update things, too, since they never explicitly added them in the first place.

A bonus perk of doing this is that, with the community migration away from Bower, this addon no longer requires it. I think it would be smart to eliminate it as a need from the high-profile addons like this one as much as possible.

[Turbo87]

thanks everyone! :)

[alexlafroscia]

Can we remove ember-cli-test-loader from bower.json as well?

[Turbo87]

@alexlafroscia yeah, that would probably make sense.