Changing the config syntax again ;-)

[vojtajina]

Guys, I know you gonna hate me, but I was fiddling with the config file during migrating whole AngularJS test suite to latest karma@canary and realized I'm not happy with the config syntax.

So I'm proposing another little change, here's an example:

// before:
module.exports = function(karma) {
  karma.configure({port: 123});
  karma.defineLauncher('x', 'Chrome', {
    flags: ['--disable-web-security']
  });
  karma.definePreprocessor('y', 'coffee', {
    bare: false
  });
  karma.defineReporter('z', 'coverage', {
    type: 'html'
  });
};

// after:
module.exports = function(config) {
  config.set({
    port: 123,
    definedLaunchers: {
      'x': {
        base: 'Chrome',
        flags: ['--disable-web-security']
      }
    },
    definedPreprocessors: {
      'y': {
        base: 'coffee',
        bare: false
      }
    },
    definedReporters: {
      'z': {
        base: 'coverage',
        type: 'html'
      }
    }    
  });

  // but now, you can also change the config directly
  // so you can load shared config and change only some bits
  require('./shared.conf')(config);
  config.a.b = '...';

  // using only config.set (or karma.configure),
  // you could only override config.a (the entire object)
};

The reasons for this change:
1/ easier to override config properties
The config object was not exposed and karma.configure() only does shallow merge.
Now, this is possible, eg. config.sauceLabs.testName = 'overridden';
This is helpful especially when using multiple config files (eg. a shared config between multiple projects or having a per developer config file that overrides project defaults).

2/ declarative definition of custom launchers/preprocessors/reporters makes more consistent and therefore easier to explain.

3/ declarative definition of custom launchers/preprocessors/reporters also makes it possible to use these with grunt-karma or from JSON config file.

The change is done in a backwards compatible manner, if one use old APIs such as karma.configure() or karma.defineLauncher(), it works. Only warning is displayed. These extra APIs will be removed in v0.11.0.

Please, let me know, what you think. I was about to push the current canary into stable, but I think we should do this before, so that people using stable version only have to migrate their configs one.

CC @dignifiedquire, @bitwiseman, @gfxmonk, @iammerrick

[iammerrick]

I haven't been screwed this hard in years.

[iammerrick]

Just kidding, I think this was a great change!

[bitwiseman]

definedLaunchers, definedPreprocessors, definedReporters: why not just launchers, preprocessors, and reporters?

As, long as you're going to remove the old format, why no do it in 10.0? If you know where we're going to go, let's go there for the next major release.

Looks otherwise good to me, too.

[kimjoar]

I think this is a great change, but as @bitwiseman says, I think it would be better with launchers, ...

And, as long as karma.definePreprocessor and friends have never been in stable, I see no point in pushing them into stable, already deprecated.

[vojtajina]

@bitwiseman I'm open to better names, definedLaunchers is pretty verbose, however reporters and preprocessors is already taken ;-)

Agree with removing the old format in 0.10, will do it.

[bitwiseman]

I'll just use launchers but this applies to all three.
It's not the verboseness that really bothers me. The problem is that all objects are "defined".
What we're talking about in each case is a named alias for a base launcher with some custom configuration, right?
So how about customLaunchers or launcherConfigs? Or even namedLauncherConfigurations.
Just about anything but "defined". 😄

[vojtajina]

I don't like "defined" either. I vote for "customLaunchers" ("customPreprocessors", "customReporters").

[hppycoder]

Oh man, now this does hurt! I agree with "customLaunchers" instead of launchers, or definedLaunchers.

Since you are changing the config around anyway how about a way to define files on the fly via the CLI (or is this something you would like me to handle?)

karma start e2e.dev.conf.js --files 'test/e2e/homepage/functionality.js'

Then it merges the "files" that are already defined in the configuration. That would be great.

Also while we are at it, could you modify your karma-saucelab-launcher to be agnostic to just saucelabs and instead open it up to the entire WebDriver stack? I know there was already a project for that around here, but given your recent changes it doesn't work anymore.

[vojtajina]

@hppycoder this PR is backwards compatible and the new config syntax has never been in stable, that's why I wanna make this change before cutting 0.10 (stable).

Regards generic webdriver launcher, there's one. I think it makes sense to have both, as the sauce launcher is very tight to sauce labs to make the integration as simple as possible.
The generic one is here https://github.com/hindsightsoftware/karma-webdriver-launcher

Regards --files in CLI, it's not that related to this change, we can add that later. In fact, I've refusing it for quit a bit, but I'm getting warmed on that idea, so send a PR (add --exclude as well)...

[hppycoder]

@vojtajina - Great on this PR being backwards compatible. I'm sorry I missed that in the initial read through.

I will work with the hindsightsoftware guys, as it doesn't actually send the URL. I will look at your launcher and theirs because they initially set it up for SauceLabs as well.

I will take on the --files injection, I will submit a pull request for it along with --exclude. To me it makes a lot of sense given the CI build systems out there such as Atlassian Bamboo where you can define explicit tests or wide open test/e2e/**/*.js

[vojtajina]

Btw, this change also makes it much easier for WebStorm plugin to amend the config...

[hppycoder]

That makes a lot of sense to amend the config, I think it will be a good change overall.

[vojtajina]

Thanks for comments guys. I changed the "defined" to "custom" and it's in the master...

[dignifiedquire]

@vojtajina really nice :) like it a lot 🌞