Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
62892fa
feat: initial promise support for `parseConfig` and `Server`
npetruzzelli Feb 28, 2021
39b9f80
feat: add promise support to `parseConfig`'s fail function
npetruzzelli Feb 28, 2021
c6c4314
feat: add async config support to `cli`, `runner`, and `stopper`
npetruzzelli Mar 2, 2021
35c992e
chore: rename `cmdSupportsConfigPromise` to `cmdNeedsConfig`
npetruzzelli Mar 5, 2021
1a8ce58
fix: catch config errors and exit
npetruzzelli Mar 5, 2021
088aa79
chore: remove throwErrors implication from promiseConfig option
npetruzzelli Mar 5, 2021
5d92865
fix: add warning for deprecated use of CLI options
npetruzzelli Mar 5, 2021
8a52b2b
fix: treat rejections the same as exceptions
npetruzzelli Mar 11, 2021
c4fd16e
chore: setup logger early when promises are used
npetruzzelli Mar 11, 2021
e4f4887
chore: remove obsolete `console.error`
npetruzzelli Mar 11, 2021
a43554b
chore: guard `logger.setupFromConfig` calls
npetruzzelli Mar 11, 2021
8490ed6
chore: update documentation for parseConfig
npetruzzelli Mar 11, 2021
63dc58f
fix: remove excess config assignment
npetruzzelli Mar 12, 2021
4c09e03
chore: document only new behavior with promises
npetruzzelli Mar 12, 2021
65e583d
chore: remove reference to example that is no longer accurate
npetruzzelli Mar 12, 2021
765cd0e
chore: update Server constructor docs
npetruzzelli Mar 12, 2021
ae7dbf7
chore: update runner and stopper docs
npetruzzelli Mar 12, 2021
91bf78a
chore: replace redundant formatting in headers
npetruzzelli Mar 12, 2021
e63c3e8
fix: replace header text that was accidentally removed
npetruzzelli Mar 12, 2021
4d8e7ea
chore: remove comment per code review
npetruzzelli Mar 15, 2021
3038717
chore: make async CLI branches testable
npetruzzelli Mar 15, 2021
8dd8d2c
chore: test CLI `run()` commands
npetruzzelli Mar 15, 2021
5a3ad45
chore: test that server logs deprecation warning
npetruzzelli Mar 17, 2021
212e791
chore: initial tests for async parseConfig
npetruzzelli Mar 17, 2021
a547795
fix: isThenable criteria should be consistent with aPlus spec
npetruzzelli Mar 17, 2021
b5ddd20
chore: minor IDE support via JSDoc block
npetruzzelli Mar 17, 2021
027e4b0
chore: simplify async test wrapper
npetruzzelli Mar 22, 2021
e4b1918
chore: reduce async noise in `run`
npetruzzelli Mar 22, 2021
d44b5df
chore: check `instanceof` for Promise object in tests
npetruzzelli Mar 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 42 additions & 20 deletions lib/cli.js
Expand Up @@ -7,6 +7,7 @@ const fs = require('graceful-fs')
const Server = require('./server')
const helper = require('./helper')
const constant = require('./constants')
const cfg = require('./config')

function processArgs (argv, options, fs, path) {
Object.getOwnPropertyNames(argv).forEach(function (name) {
Expand Down Expand Up @@ -278,26 +279,47 @@ exports.process = () => {
}

exports.run = () => {
const config = exports.process()

switch (config.cmd) {
case 'start':
new Server(config).start()
break
case 'run':
require('./runner')
.run(config)
.on('progress', printRunnerProgress)
break
case 'stop':
require('./stopper').stop(config)
break
case 'init':
require('./init').init(config)
break
case 'completion':
require('./completion').completion(config)
break
const cliOptions = exports.process()
const cmd = cliOptions.cmd // prevent config from changing the command
const cmdNeedsConfig = cmd === 'start' || cmd === 'run' || cmd === 'stop'
if (cmdNeedsConfig) {
cfg.parseConfig(
cliOptions.configFile,
cliOptions,
{
promiseConfig: true,
throwErrors: true
}
).then(function onKarmaConfigFulfilled (config) {
switch (cmd) {
case 'start':
new Server(config).start()
break
case 'run':
require('./runner')
.run(config)
.on('progress', printRunnerProgress)
break
case 'stop':
require('./stopper').stop(config)
break
}
}, function onKarmaConfigRejected (reason) {
console.error(reason) // TODO: configure a CLI Logger?
devoto13 marked this conversation as resolved.
Show resolved Hide resolved

// The `run` function is a private application, not a public API. We don't
// need to worry about process.exit vs throw vs promise rejection here.
process.exit(1)
})
} else {
switch (cmd) {
case 'init':
require('./init').init(cliOptions)
break
case 'completion':
require('./completion').completion(cliOptions)
break
}
}
}

Expand Down
73 changes: 52 additions & 21 deletions lib/config.js
Expand Up @@ -352,11 +352,17 @@ const CONFIG_SYNTAX_HELP = ' module.exports = function(config) {\n' +
' };\n'

function parseConfig (configFilePath, cliOptions, parseOptions) {
const promiseConfig = parseOptions && parseOptions.promiseConfig === true
devoto13 marked this conversation as resolved.
Show resolved Hide resolved
const throwErrors = parseOptions && parseOptions.throwErrors === true
function fail () {
log.error(...arguments)
if (parseOptions && parseOptions.throwErrors === true) {
if (throwErrors) {
const errorMessage = Array.from(arguments).join(' ')
throw new Error(errorMessage)
const err = new Error(errorMessage)
if (promiseConfig) {
return Promise.reject(err)
}
throw err
} else {
const warningMessage =
'The `parseConfig()` function historically called `process.exit(1)`' +
Expand Down Expand Up @@ -411,34 +417,59 @@ function parseConfig (configFilePath, cliOptions, parseOptions) {
// add the user's configuration in
config.set(cliOptions)

let configModuleReturn
try {
configModule(config)
configModuleReturn = configModule(config)
} catch (e) {
return fail('Error in config file!\n', e)
}
function finalizeConfig (config) {
// merge the config from config file and cliOptions (precedence)
config.set(cliOptions)

// merge the config from config file and cliOptions (precedence)
config.set(cliOptions)

// if the user changed listenAddress, but didn't set a hostname, warn them
if (config.hostname === null && config.listenAddress !== null) {
log.warn(`ListenAddress was set to ${config.listenAddress} but hostname was left as the default: ` +
// if the user changed listenAddress, but didn't set a hostname, warn them
if (config.hostname === null && config.listenAddress !== null) {
log.warn(`ListenAddress was set to ${config.listenAddress} but hostname was left as the default: ` +
`${defaultHostname}. If your browsers fail to connect, consider changing the hostname option.`)
}
// restore values that weren't overwritten by the user
if (config.hostname === null) {
config.hostname = defaultHostname
}
if (config.listenAddress === null) {
config.listenAddress = defaultListenAddress
}
}
// restore values that weren't overwritten by the user
if (config.hostname === null) {
config.hostname = defaultHostname
}
if (config.listenAddress === null) {
config.listenAddress = defaultListenAddress
}

// configure the logger as soon as we can
logger.setup(config.logLevel, config.colors, config.loggers)
// configure the logger as soon as we can
logger.setup(config.logLevel, config.colors, config.loggers)

log.debug(configFilePath ? `Loading config ${configFilePath}` : 'No config file specified.')
log.debug(configFilePath ? `Loading config ${configFilePath}` : 'No config file specified.')

return normalizeConfig(config, configFilePath)
return normalizeConfig(config, configFilePath)
}
const returnIsThenable = configModuleReturn != null && typeof configModuleReturn === 'object' && typeof configModuleReturn.then === 'function'
if (returnIsThenable) {
if (promiseConfig !== true) {
const errorMessage =
'The `parseOptions.promiseConfig` option must be set to `true` to ' +
'enable promise return values from configuration files. ' +
'Example: `parseConfig(path, cliOptions, { promiseConfig: true })`'
devoto13 marked this conversation as resolved.
Show resolved Hide resolved
return fail(errorMessage)
}
return configModuleReturn.then(function onKarmaConfigModuleFulfilled (/* ignoredResolutionValue */) {
devoto13 marked this conversation as resolved.
Show resolved Hide resolved
return finalizeConfig(config)
})
} else {
if (promiseConfig) {
try {
return Promise.resolve(finalizeConfig(config))
} catch (exception) {
return Promise.reject(exception)
}
} else {
return finalizeConfig(config)
}
}
}

// PUBLIC API
Expand Down
40 changes: 35 additions & 5 deletions lib/runner.js
Expand Up @@ -32,14 +32,44 @@ function parseExitCode (buffer, defaultExitCode, failOnEmptyTestSuite) {
}

// TODO(vojta): read config file (port, host, urlRoot)
function run (config, done) {
config = config || {}

logger.setupFromConfig(config)
function run (cliOptionsOrConfig, done) {
cliOptionsOrConfig = cliOptionsOrConfig || {}

// TODO: Should `const log = logger.create('runner')` be moved into the
// : function for consistency with server.js and stopper.js? or the
// : reverse (make server and stopper consistent with runner?)
logger.setupFromConfig({
colors: cliOptionsOrConfig.colors,
logLevel: cliOptionsOrConfig.logLevel
})

done = helper.isFunction(done) ? done : process.exit
config = cfg.parseConfig(config.configFile, config)

let config
if (cliOptionsOrConfig instanceof cfg.Config) {
config = cliOptionsOrConfig
} else {
const deprecatedCliOptionsMessage =
'Passing raw CLI options to `runner(config, done)` is deprecated. Use ' +
'`parseConfig(configFilePath, cliOptions, {promiseConfig: true, throwErrors: true})` ' +
'to prepare a processed `Config` instance and pass that as the ' +
'`config` argument instead.'
log.warn(deprecatedCliOptionsMessage)
try {
config = cfg.parseConfig(
cliOptionsOrConfig.configFile,
cliOptionsOrConfig,
{
promiseConfig: false,
throwErrors: true
}
)
} catch (parseConfigError) {
// TODO: change how `done` falls back to exit in next major version
// SEE: https://github.com/karma-runner/karma/pull/3635#discussion_r565399378
done(1)
}
}
let exitCode = 1
const emitter = new EventEmitter()
const options = {
Expand Down
43 changes: 32 additions & 11 deletions lib/server.js
Expand Up @@ -55,22 +55,43 @@ function createSocketIoServer (webServer, executor, config) {
}

class Server extends KarmaEventEmitter {
constructor (cliOptions, done) {
constructor (cliOptionsOrConfig, done) {
super()
logger.setupFromConfig(cliOptions)

cliOptionsOrConfig = cliOptionsOrConfig || {}
logger.setupFromConfig({
colors: cliOptionsOrConfig.colors,
logLevel: cliOptionsOrConfig.logLevel
})
this.log = logger.create('karma-server')

done = helper.isFunction(done) ? done : process.exit
this.loadErrors = []

let config
try {
config = cfg.parseConfig(cliOptions.configFile, cliOptions, { throwErrors: true })
} catch (parseConfigError) {
// TODO: change how `done` falls back to exit in next major version
// SEE: https://github.com/karma-runner/karma/pull/3635#discussion_r565399378
(done || process.exit)(1)
return
if (cliOptionsOrConfig instanceof cfg.Config) {
config = cliOptionsOrConfig
} else {
const deprecatedCliOptionsMessage =
'Passing raw CLI options to `new Server(config, done)` is ' +
'deprecated. Use ' +
'`parseConfig(configFilePath, cliOptions, {promiseConfig: true, throwErrors: true})` ' +
'to prepare a processed `Config` instance and pass that as the ' +
'`config` argument instead.'
this.log.warn(deprecatedCliOptionsMessage)
try {
devoto13 marked this conversation as resolved.
Show resolved Hide resolved
config = cfg.parseConfig(
cliOptionsOrConfig.configFile,
cliOptionsOrConfig,
{
promiseConfig: false,
throwErrors: true
}
)
} catch (parseConfigError) {
// TODO: change how `done` falls back to exit in next major version
// SEE: https://github.com/karma-runner/karma/pull/3635#discussion_r565399378
done(1)
return
}
}

this.log.debug('Final config', util.inspect(config, false, /** depth **/ null))
Expand Down
35 changes: 32 additions & 3 deletions lib/stopper.js
Expand Up @@ -3,11 +3,40 @@ const cfg = require('./config')
const logger = require('./logger')
const helper = require('./helper')

exports.stop = function (config, done) {
config = config || {}
logger.setupFromConfig(config)
exports.stop = function (cliOptionsOrConfig, done) {
cliOptionsOrConfig = cliOptionsOrConfig || {}
logger.setupFromConfig({
colors: cliOptionsOrConfig.colors,
logLevel: cliOptionsOrConfig.logLevel
})
const log = logger.create('stopper')
done = helper.isFunction(done) ? done : process.exit

let config
if (cliOptionsOrConfig instanceof cfg.Config) {
config = cliOptionsOrConfig
} else {
const deprecatedCliOptionsMessage =
'Passing raw CLI options to `stopper(config, done)` is deprecated. Use ' +
'`parseConfig(configFilePath, cliOptions, {promiseConfig: true, throwErrors: true})` ' +
'to prepare a processed `Config` instance and pass that as the ' +
'`config` argument instead.'
log.warn(deprecatedCliOptionsMessage)
try {
config = cfg.parseConfig(
cliOptionsOrConfig.configFile,
cliOptionsOrConfig,
{
promiseConfig: false,
throwErrors: true
}
)
} catch (parseConfigError) {
// TODO: change how `done` falls back to exit in next major version
// SEE: https://github.com/karma-runner/karma/pull/3635#discussion_r565399378
done(1)
}
}
config = cfg.parseConfig(config.configFile, config)

const request = http.request({
Expand Down