Skip to content
This repository has been archived by the owner on Dec 28, 2023. It is now read-only.

fix: ensure karma.config is passed (#192) #206

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

segrey
Copy link

@segrey segrey commented May 25, 2018

No description provided.

@segrey
Copy link
Author

segrey commented Sep 15, 2020

Seems the change is pretty harmless. @johnjbarton What do you think?

Copy link
Contributor

@johnjbarton johnjbarton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this project needs some work on the package.json and such.

var clientArguments
config = config || {}
clientArguments = config.args
return function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could there be existing callers in other code that expect the argument to be used?

clientArguments = config.args
return function () {
var config = karma.config || {}
var clientArguments = config.args
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move these two lines up before the return statement. They will execute during file-load so the clientArguments will be available then, rather than only later during the load-event handling.

That has no impact, but in the case of karma-jasmine, the config values can be used to reset jasmine.DEFAULT_TIMEOUT_INTERVAL and it must be set during file-load.

@@ -173,12 +173,11 @@ var createMochaReporterConstructor = function (tc, pathname) {
}
}
/* eslint-disable no-unused-vars */
var createMochaStartFn = function (mocha) {
var createMochaStartFn = function (karma, mocha) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make karma the second argument so existing code calling with one argument will work.

config = config || {}
clientArguments = config.args
return function () {
var config = karma.config || {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safer would be to leave the config parameter and

config = config ? config : (karma ? karma.config || {} : {}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants