Do not override reporter option if not defined in config file (issue #369) #372
Conversation
|
Do I need to do something to start the test library workflow ? |
It needs a maintainer to approve. I'm in the same situation with pending PRs and I haven't committed to this repo before. |
|
@tcort any chance you could merge this? This is breaking builds downstream and I have confirmed that this fix works for us. I believe this issue is breaking all cases of using a config file as far as I can tell and there are no current ways to work around this issue for users. |
This comment was marked as resolved.
This comment was marked as resolved.
|
@MikeMcC399 just did it |
|
@tcort would it be possible to review changes and valide them if it's OK for you ? |
| @@ -288,7 +288,7 @@ async function processInput(filenameForOutput, stream, opts) { | |||
| opts.retryCount = config.retryCount; | |||
| opts.fallbackRetryDelay = config.fallbackRetryDelay; | |||
| opts.aliveStatusCodes = config.aliveStatusCodes; | |||
| opts.reporters = config.reporters; | |||
| opts.reporters = config.reporters ?? opts.reporters; | |||
There was a problem hiding this comment.
@smainil I wonder whether this fix might be applicable to the config options?
Also, the behaviour of that whole if (opts.config) block can be deleted and no tests fail, we might want to increase the test coverage.
There was a problem hiding this comment.
@WillGibson definitely agree about the test coverage. I wanted to quickly fix this regression because lots of people are impacted
There was a problem hiding this comment.
I accept that pragmatism.
I think a lot of these values are defaulted here and the others should use the same null coalescing where that is the case. Otherwise the same problem is likely to occur with them.
Fix #369