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

Improve validation on Stryker options. #2103

Closed
nicojs opened this issue Mar 13, 2020 · 3 comments · Fixed by #2164
Closed

Improve validation on Stryker options. #2103

nicojs opened this issue Mar 13, 2020 · 3 comments · Fixed by #2164
Labels
🚀 Feature request New feature request

Comments

@nicojs
Copy link
Member

nicojs commented Mar 13, 2020

Right now there is no standardized and complete way (for both plugins as well as the core) to do the validation.

For example, this config file will result in no errors at the moment:

{
  "concurrency": 2,
  "reporter": ["clear-text"]
  "thresholds": {
    "error": 80
   }
}

Solution

I want to allow use JSON Schema to validate. We can use AJV as validator. As for human-readable error messages we can use AJV errors. As a bonus, we could let AJV assign defaults.

Plugins could contribute parts of the schema dynamically by adding a schema/stryker-plugin.json (name pending) file in their respective plugins.

Verifying that no additional properties are present can be done by running the schema validation with additionalProperties: false. They should be logged as warning. We should allow JSON comments by not warning about settings with the postfix "comment". We could allow a --noWarnOnAdditionalConfiguration (name pending) to not warn about additional settings (otherwise people might get frustrated).

@nicojs nicojs added the 🚀 Feature request New feature request label Mar 13, 2020
@simondel
Copy link
Member

Sounds good!

@bartekleon
Copy link
Member

I am so happy now that we made this schema :D

@nicojs
Copy link
Member Author

nicojs commented Mar 27, 2020

A start has been made here: #2123 . It will validate the core schema, but not yet warn about additional properties. It also won't validate plugins yet, but it's a start.

Feel free to review 😅

nicojs added a commit that referenced this issue Apr 22, 2020
Add JSON schema for the options of each plugin and base the types on that (using npm run generate).

This is the first step to implement validation on the config options of plugins, see #2103

Co-authored-by: Simon de Lang <simondelang@gmail.com>
nicojs added a commit that referenced this issue May 13, 2020
Add a warning when options where detected that did not contribute to the option validation.

Example: 

```
16:40:37 (23836) WARN markUnknownOptions Unknown stryker config option "concurrency".
16:40:37 (23836) WARN markUnknownOptions
   Possible causes:
   * Is it a typo on your end?
   * Did you only write this property as a comment? If so, please postfix it with "_comment".
   * You might be missing a plugin that is supposed to use it. Stryker loaded plugins from: ["@stryker-mutator/*"]
   * The plugin that is using it did not contribute explicit validation. 
   (disable "warnings.unknownOptions" to ignore this warning)`
```

You can still disable this by setting `warnings.unknownOptions` to false.

Fixes #2103
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 Feature request New feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants