feat: support asynchronous config.set() call in karma.conf.js

[npetruzzelli]

With this change, Karma allows to return a Promise from a configuration function directly or make such function async and thus allows to perform some asynchronous initialization/fetch configuration from somewhere. When the configuration function returns a Promise, Karma will wait until the promise is resolved before it proceeds with the next steps.

Example usage:

module.exports = async (config) => {
  const karmaConfig = await getKarmaConfig("dev");
  
  config.set({
    ...karmaConfig
  });
};

For Karma's end-users the change is fully backward compatible. For Karma's programmatic users the change is backward compatible, but the current behavior of parseConfig(), Server, runner, and stopper is deprecated and will change in the next major release. New behavior (which supports asynchronous configuration function) can be enabled with a flag. Please refer to the public API documentation for more details.

Fixes #1259

[karmarunnerbot]

✅ Build karma 540 completed (commit eeaff6b835 by @npetruzzelli)

[AppVeyorBot]

✅ Build karma 2937 completed (commit eeaff6b835 by @npetruzzelli)

[npetruzzelli]

@devoto13 - I'm not sure if I will be able to see this PR through to completion (quickly), but I was able to at least start the work so that the conversation/review can start.

I can't help but to "throw my hat into the ring" as it were. I initially opened #3631 / #3635 because I was working on a personal module that would support ES Modules and Promises. It still needs a lot of work, including the addition of tests (which is why it is only v 0.1.0). It is significantly overbuilt for what Karma would want or need, but I'll link it here in case you want to have a look:
https://github.com/promise-file/promise-file/tree/v0.1.0/packages/promise-file-karma-config

[karmarunnerbot]

✅ Build karma 539 completed (commit eeaff6b835 by @npetruzzelli)

[npetruzzelli]

Would this need the ability for users to pass their own promise implementation, or will the default Promise meet the needs of all environments we expect this to serve in?

[karmarunnerbot]

✅ Build karma 541 completed (commit 527337b0be by @npetruzzelli)

[AppVeyorBot]

✅ Build karma 2938 completed (commit 527337b0be by @npetruzzelli)

[karmarunnerbot]

✅ Build karma 540 completed (commit 527337b0be by @npetruzzelli)

[AppVeyorBot]

✅ Build karma 2939 completed (commit ff32cf5f74 by @npetruzzelli)

[karmarunnerbot]

✅ Build karma 541 completed (commit ff32cf5f74 by @npetruzzelli)

[npetruzzelli]

ASIDE: Should the process and parseClientArgs functions from cli.js be made a part of the public API for anyone using parseConfig and the public API methods directly? The intent being that they may want to parse command line arguments in a way that is consistent with Karma (this deserves its own issue for discussion prior to a PR and is not in the scope of this PR.)

[npetruzzelli]

It still needs new tests to be written, but I am marking this as ready for review to address feedback prior to putting time into tests so that I can avoid writing tests that may be made obsolete by review feedback.

[karmarunnerbot]

✅ Build karma 542 completed (commit ff32cf5f74 by @npetruzzelli)

[devoto13]

I'm not sure if I will be able to see this PR through to completion (quickly), but I was able to at least start the work so that the conversation/review can start.

No worries, take your time. And thanks for the work you're putting into implementing this!

Would this need the ability for users to pass their own promise implementation, or will the default Promise meet the needs of all environments we expect this to serve in?

Default implementation is enough.

Should the process and parseClientArgs functions from cli.js be made a part of the public API for anyone using parseConfig and the public API methods directly? The intent being that they may want to parse command line arguments in a way that is consistent with Karma (this deserves its own issue for discussion prior to a PR and is not in the scope of this PR.)

I'm not sure how useful is this. Better have strong real use case for this and discuss it before doing anything.

It still needs new tests to be written, but I am marking this as ready for review to address feedback prior to putting time into tests so that I can avoid writing tests that may be made obsolete by review feedback.

I'll take a look right away.

[devoto13]

It looks great @npetruzzelli! Thanks a lot for doing this.

I left a couple of tiny comments. Let me know if they make sense to you.

I've also checked point (4). Looking at the PR code, I don't think we need to do anything about it.

[karmarunnerbot]

✅ Build karma 546 completed (commit b08ab63426 by @npetruzzelli)

[AppVeyorBot]

✅ Build karma 2943 completed (commit b08ab63426 by @npetruzzelli)

[karmarunnerbot]

✅ Build karma 545 completed (commit b08ab63426 by @npetruzzelli)

[AppVeyorBot]

✅ Build karma 2959 completed (commit cbdb405e59 by @npetruzzelli)

[karmarunnerbot]

✅ Build karma 561 completed (commit cbdb405e59 by @npetruzzelli)

[devoto13]

Overall looks good! Left couple of comments to cleanup.

[karmarunnerbot]

✅ Build karma 565 completed (commit 936aa92bba by @npetruzzelli)

[AppVeyorBot]

✅ Build karma 2962 completed (commit 936aa92bba by @npetruzzelli)

[karmarunnerbot]

✅ Build karma 564 completed (commit 936aa92bba by @npetruzzelli)

[karmarunnerbot]

✅ Build karma 565 completed (commit 65f3b8fbbd by @npetruzzelli)

[karmarunnerbot]

✅ Build karma 566 completed (commit 65f3b8fbbd by @npetruzzelli)

[npetruzzelli]

@devoto13 - While working on updates for this round of review, I was running into an issue where random tests in config.spec.js would log WARN [config]: urlRoot normalized to "/".

• Sometimes it happens, sometimes it doesn't (making it difficult to replicate)
• Sometimes it was the same test, sometimes it wasn't
• It always only happened once per run
• It was never any of the parseConfig tests
• Using .only to reduce the number of tests made it much less likely to occur, though using .only on the top level describe didn't seem to affect it
• The only console message that (currently) interrupts mocha's status reporter is: (node:21192) Warning: Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to '0' makes TLS connections and HTTPS requests insecure by disabling certificate verification. which has been the norm for me since I started contributing.

I haven't been able to replicate the problem as of the latest commit. It may not be an issue anymore, but I wanted to make you aware of it.

[devoto13]

I've updated the title and description to provide some useful information to future readers. It looks good to go now.

@npetruzzelli Thanks for doing all the hard work! 💪

[johnjbarton]

awesome, thanks

[karmarunnerbot]

🎉 This PR is included in version 6.3.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[npetruzzelli]

You're most welcome! Glad to help!