feat(stryker): #438 Extensive config validation #549
Conversation
| const logLevel = this.strykerConfig.logLevel; | ||
| const VALID_LOG_LEVEL_VALUES = ['fatal', 'error', 'warn', 'info', 'debug', 'trace', 'all', 'off']; | ||
| if (VALID_LOG_LEVEL_VALUES.indexOf(logLevel) < 0) { | ||
| this.invalidate('\`logLevel\` is invalid, expected one of \`fatal\`, \`error\`, \`warn\`, \`info\`, \`debug\`, \`trace\`, \`all\` and \`off\`'); |
There was a problem hiding this comment.
You could use VALID_LOG_LEVEL_VALUES .join(',') here to reduce duplication.
There was a problem hiding this comment.
This is actually done in validateCoverageAnalysis. I'm a fan on using the join here as well.
| this.validateIsString('mutator', this.strykerConfig.mutator); | ||
| this.validateIsStringArray('plugins', this.strykerConfig.plugins); | ||
| this.validateIsStringArray('reporter', this.strykerConfig.reporter); | ||
| this.validateIsStringArray('transpilers', this.strykerConfig.transpilers); |
There was a problem hiding this comment.
I love this, very much readable!
There was a problem hiding this comment.
Awesome job! I love how you set up the code :)
I have a small remark: I noticed that there is the invalidation messages differ quite a bit. For example. If the coverageanalsysis property is invalid. You log:
`Value "${coverageAnalysis}" is invalid for \`coverageAnalysis\`. Expected one of the folowing: .......
When the general logLevel is invalid, you don't log which value was actually entered. Similar small inconsistencies can also be found in other logging statements. Two examples of this are the use of the join function on the array of loglevels that @nicojs mentioned and in the quotes that you put around property/values (logLevel is logged as `logLevel` and maxConcurrentTestRunners is logged as maxConcurrentTestRunners for example)
I also think that the logging statements could be a bit more readable in TypeScript if you don't use backticks inside of a interpolated string.
|
@simondel I went ahead and changed as many of the error messages as possible to follow the format "Value {value} is invalid for {field}. Expected {expectation}." I left one pair of backticks in the messages (around the field) for back consistency. |
|
Awesome! Looking good :) @nicojs Shall we merge this? |
|
Great job! 🥇 |
No description provided.