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

Restructure stryker CLI options #578

Closed
rouke-broersma opened this issue Jun 12, 2019 · 9 comments · Fixed by #1273
Closed

Restructure stryker CLI options #578

rouke-broersma opened this issue Jun 12, 2019 · 9 comments · Fixed by #1273
Assignees
Labels
🚀 Feature request New feature or request
Milestone

Comments

@rouke-broersma
Copy link
Member

rouke-broersma commented Jun 12, 2019

We should split stryker options and restructure the CLI to provide this in a generic sense because of constructor overload when developing features and because of configuration overload as a user. I propose the following groups of options to start with:

Commandline options - common options when running stryker one-off. Should be quickstart-like

Commandline advanced options - more advanced options for running stryker one-off (think solution path, project under test filter, coverage analysis mode etc)

Configfile options - all commandline options + all other options that do not fit nicely on the commandline

We should probably introduce these as separate concepts with a common interface or something and not shoehorn them all into the same class as we currently have.

Requirements:

  1. Order of importance for options is maintained: CLI > JSON > (OTHERS) > Defaults
    1.1. EXCEPT for commandline flags without values (bool). In that case CLI only takes precedence if it's defined, not when it's missing
  2. No JSON array syntax on commandline
  3. Multiple CLI options allowed for some options
  4. Not all options will be CLI options. Not all options will be JSON options.
  5. CLI options and JSON options don't have the exact same names
  6. CLI options and JSON options don't have the exact same names as the Options properties in Stryker.Core
  7. Stryker.Core owns argument validation
@rouke-broersma rouke-broersma added the 🚀 Feature request New feature or request label Jun 12, 2019
@rouke-broersma
Copy link
Member Author

@riezebosch
Copy link
Contributor

Please, get rid of the JS-like syntax for multiple arguments.

-tp IntegrationTests/IntegratonTests.csproj -tp Tests/Tests.csproj

instead of

-tp "['IntegrationTests/IntegratonTests.csproj', 'Tests/Tests.csproj']"

@rouke-broersma
Copy link
Member Author

@riezebosch would love to, just haven't been able to give it enough priority

@richardwerkman
Copy link
Member

Also that would mean a breaking change, unless we still support the ugly json style...

@riezebosch
Copy link
Contributor

riezebosch commented Mar 2, 2020

I've been trying to refactor it a little, but what a mess is the StrykerOptions. Parsing, validating, merging all in one and 249 places where it is used but only in parts. Sorry, had to let off a little frustration.

FTR: the used framework already supports multiple values for the same argument.

@rouke-broersma
Copy link
Member Author

We know, but that didn't fit our use case at the time. We tried to reuse the same argument parsing for both the json config and the command line options. This worked well until we needed to allow multiple arguments. Unfortunately we had already built the parser and then made the mistake to keep going with it even though it was ugly as all hell.

@riezebosch
Copy link
Contributor

The issue was the trigger for this story: https://medium.com/@MRiezebosch/integrate-command-line-parsers-into-dotnet-core-configuration-providers-9b5f5d1af672

@richardwerkman
Copy link
Member

Awesome! Do you think it can easily be integrated into stryker? That would resolve this long running issue

@riezebosch
Copy link
Contributor

I started working on this only to find out that the AddJsonFile configuration provider also only support direct mapping to the configuration object, meaning you currently cannot have properties like api-key in your json file.

So my next project is a mapping provider where you translate your configuration object to keys and wrap that around whatever provider you use.

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

Successfully merging a pull request may close this issue.

4 participants