Add an error message for missing dependencies.

[boushley]

This ensures that something is printed out for users who didn't add a mapping for one of their dependencies. Either they missed a deps.js file or they didn't preprocess all of their source files. The current behavior adds an undefined filepath which ends up causing the karma web-server to blow up when it trys to do indexOf on undefined.

I'm sure it would be better to actually get the real karma logger here, but I didn't see any examples of doing that in any of the other plugins. So for now I'm just using a console.error.

[vojtajina]

@boushley thanks! Can you please;

• make sure travis build passes (https://travis-ci.org/karma-runner/karma-closure/builds/8770283), it's just formatting
• change the commit msg to something like fix: show error for a missing dependency
• use karma logger

Ad. using karma logger, something like this:

var DependencyResolver = function(logger) {
  var log = logger.create('closure');
  // ...
  log.error('...');

[boushley]

Added a single unit test for the resolver that isn't very thorough, but it provides the logger dependency. @vojtajina let me know if you need me to get the resolver unit tests done further before landing this.

I've taken care of the other issues.

[boushley]

It looks like the Travis test is still failing, but it appears that part of the test was failing prior to my changes.

At least when I run karma start test-app/karma.conf.js --browsers Firefox --single-run locally before my change I get the same error. Which is different than the Travis error, it is something about missing karma-closure

Error: No provider for "framework:closure"! (Resolving: framework:closure)
    at error (/projects/karma-closure/node_modules/karma/node_modules/di/lib/injector.js:22:68)
    at Object.parent.get (/projects/karma-closure/node_modules/karma/node_modules/di/lib/injector.js:9:13)
    at get (/projects/karma-closure/node_modules/karma/node_modules/di/lib/injector.js:54:19)
    ...

I'll try and look into it at some point if I can find time to dig into that error.

[vojtajina]

remove this todo

[vojtajina]

The Travis build passes on the master, but it's weird, it should actually fail - I don't understand how come that it does find "karma" reference, as karma is not installed globally.

Anyway, I fixed that. Can you rebase it on the latest master, to see if it passes ?

[vojtajina]

Also, please remove the note about using karma logger (in the commit msg), as you already did it ;-)

[boushley]

Good catch on the todo comment :D And it looks like the Travis build is passing again.

[vojtajina]

Can you remove the note about using karma logger (in the commit msg), as you already did it ;-)

[boushley]

I think the message is fixed and pushed, but the github site isn't coming
up for me, so I can't verify if it's actually pushed.

Aaron

On Sat, Jul 13, 2013 at 7:37 PM, Vojta Jina notifications@github.comwrote:

Can you remove the note about using karma logger (in the commit msg), as
you already did it ;-)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-20930652
.

[vojtajina]

Github has been pretty lame today ;-)

Let's remove the test, as it does not test anything anyway and it's good to merge.

Thanks a lot @boushley !

[boushley]

Got it pushed up with the tests commented out.

Hope it wasn't more effort getting my commit squared away :)

I figured it was on my end (hotel internet today).

Aaron

On Sat, Jul 13, 2013 at 7:53 PM, Vojta Jina notifications@github.comwrote:

Github has been pretty lame today ;-)

Let's remove the test, as it does not test anything anyway and it's good
to merge.

Thanks a lot @boushley https://github.com/boushley !

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-20930790
.

[vojtajina]

Merged as ce16632

You need to reset your "master" branch, because I did change your commit a bit (because the travis was failing due to linting errors). It's easier if you create a branch per PR.

Also, you can run all the tests/linting locally, using grunt.

Thanks a lot @boushley !

[boushley]

Sounds good. I normally do create a branch power feature but with this one
I forgot since I was doing it to fix the problem I was having while running
my tests :) And sorry about not running the tests.

Aaron
On Jul 14, 2013 12:45 AM, "Vojta Jina" notifications@github.com wrote:

Merged as ce16632ce16632

You need to reset your "master" branch, because I did change your commit a
bit (because the travis was failing due to linting errors). It's easier if
you create a branch per PR.

Also, you can run all the tests/linting locally, using grunt.

Thanks a lot @boushley https://github.com/boushley !

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-20931649
.