feat: Fail on launcher-, reporter-, or preprocessor-load errors.

[srawlins]

Fail out of karma if a launcher, reporter, or preprocessor fails to load, after attempting to load each of them, and reporting errors for each load error.

Update

This PR is now ready, barring the issue with changing the signature of createPreprocessor that I highlight below. I did not add any tests, but would be happy to if someone could point me the way there.

Example

I put the following into a config file:

preprocessors: {
  "**/*.js": ["tea"]
},
reporters: ["progress", "regress"],
browsers: ["Chrome", "Crime"],

and started karma:

$ ../bin/karma start my.conf.js 
16 07 2015 13:14:36.172:ERROR [reporter]: Cannot load reporter "regress": it is not registered! Perhaps you are missing some plugin?
16 07 2015 13:14:36.182:ERROR [preprocess]: Cannot load preprocessor "tea": it is not registered! Perhaps you are missing some plugin?
16 07 2015 13:14:36.189:INFO [karma]: Karma v0.13.0 server started at http://localhost:9876/
16 07 2015 13:14:36.189:ERROR [launcher]: Cannot load browser "Chrome": it is not registered! Perhaps you are missing some plugin?
16 07 2015 13:14:36.189:ERROR [launcher]: Cannot load browser "Crime": it is not registered! Perhaps you are missing some plugin?

then karma promptly exits.

Closes #855

[srawlins]

As each error is caught, they are collected here. After this, however, I can't figure out where to perform a final check of whether loadErrors is empty or not...

[vojtajina]

Karma should fail if any plugin can't be instantiated. That is: launcher, preprocessor, reporter.

Btw, I like the idea of collecting all errors and then failing, so that user sees all the errors and not only the first one.

Here are all the places where Karma instantiate plugins and thus a fatal error can happen (in order in which it happens):

1. Loading plugins (loading the DI modules):
   

   karma/lib/server.js

   Line 310 in 6d53309

   modules = modules.concat(plugin.resolve(config.plugins));

2. Instantiate reporters
   

   karma/lib/reporter.js

   Line 85 in 51f1b88

   reporters.push(injector.createChild([locals], ['reporter:' + name]).get('reporter:' + name));

3. Instantiating frameworks (inside start function):
   

   karma/lib/server.js

   Line 30 in 6d53309

   config.frameworks.forEach(function(framework) {

4. Start browsers (after the webserver is ready listening):
   

   karma/lib/server.js

   Line 69 in 6d53309

   injector.invoke(launcher.launch, launcher).forEach(function(browserLauncher) {

5. Instantiate preprocessors (this happens lazily, once the preprocessor is needed; we need to change that and instantiate all needed preprocessors ahead, inside createPreprocessor factory)
   

   karma/lib/preprocessor.js

   Line 88 in 6d53309

   config[patterns[i]].forEach(instantiatePreprocessor);

Thus, I would add the check into the webserver listener, after launching browsers (), and fail Karma if any error occurred. At that point, all of the plugins were instantiated, except preprocessors. We need to change preprocessors to instantiate all needed preprocessors ahead (in the createPreprocessor factory), instead of lazily.

I'm sorry this is confusing. I think it would be good to refactor Karma to instantiate everything first. Right now, "instantiating things" is kind of tangled together with bootstrapping (this is probably more efficient, but harder to understand). I wish I did it differently;-)

@srawlins Let me know if I can help you more. This is very valuable work you are doing!

[dignifiedquire]

@srawlins any chance for you updating this PR with the suggestions from @vojtajina. I would really appreciate it and I promise a shorter feedback cycle!

[srawlins]

Hi Friedel, i'm actually on vacation until June 20. Please feel free to
submit a different PR with my commits if you want this in soon. Otherwise I
can try to finish at the end of June.
On May 17, 2015 7:27 PM, "Friedel Ziegelmayer" [email protected]
wrote:

@srawlins https://github.com/srawlins any chance for you updating this
PR with the suggestions from @vojtajina https://github.com/vojtajina. I
would really appreciate it and I promise a shorter feedback cycle!

—
Reply to this email directly or view it on GitHub
#1239 (comment).

[dignifiedquire]

Thanks @srawlins for the quick answer, I'll see where we're at until June the 20th and update this pr if I get to doing it myself before.

[dignifiedquire]

hey @srawlins any news on you getting back?

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[srawlins]

Hi @dignifiedquire , I've updated this with @vojtajina's suggestions, and it's working great! There's one problem though: I've changed the signature of createPreprocessor, adding emitter:

var createPreprocessor = function(config, basePath, injector, emitter) {

Travis revealed this as a problem, I think specifically with the karma-browserify which calls createPreprocessor. I don't know karma's ecosystem well enough to know how big a problem this is. But I don't see another way around my implementation.

[dignifiedquire]

Please use {} instead

[dignifiedquire]

As far as I understand this di business, there should be no issue changing the signature, as it's only used as a factory function, which receives it's arguments through the injector

[srawlins]

Yeah, I think you're right. Unfortunately, karma-browserify calls createPreprocessor directly.

[dignifiedquire]

@srawlins any updates?

[srawlins]

Closing to try to work this another way with #1931