-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Config: add tests for the --generator=
argument
#765
Config: add tests for the --generator=
argument
#765
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @rodrigoprimo .
I presume a test with the argument being passed an invalid value will follow in the PR to address #709 ?
I've left some small nitpicks inline. Also noticed the @dataProvider
and @see
tags being at the end of the docblocks, which is not consistent with their use elsewhere, but not blocking (as we don't enforce a specific order, which I would love to do, but that will need to wait for a proper Docs standard).
Co-authored-by: Juliette <[email protected]>
Thanks for the review, @jrfnl!
Yes, I opted not to include it, as the code does not currently check for invalid values. But now I see that I could have included it to document the code's current behavior and then update the test in the subsequent PR.
I used This PR is ready for another review. |
Do you still want to do that or would you prefer for this PR to be merged and the next one to document handling of invalid values ? |
The easiest for me would be to have this PR merged and include the other test in the subsequent PR. But I'm happy to update this PR if you prefer. |
Okay, done. Looking forward to the follow-up PR. |
Description
In preparation for a PR to address #709, this PR adds tests to cover the current behavior of the code in the
Config
class that handles setting thegenerator
property when the--generator=
argument is passed in the command line.Related issues/external references
Related to #709
Types of changes
PR checklist