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

Make String to Boolean parameter conversion more strict #3178

Merged
merged 6 commits into from
Apr 28, 2023
Merged

Make String to Boolean parameter conversion more strict #3178

merged 6 commits into from
Apr 28, 2023

Conversation

s-rwe
Copy link
Contributor

@s-rwe s-rwe commented Feb 27, 2023

Cause use of anything other than 'true' or 'false' (case-insensitive) fail in conversion, to make potential typos explicitly visible (instead of implicitly just treating all else as 'false')

Includes a few tests for failures on invalid values, for conversions handled by StringToBooleanAndCharPrimitiveConverter.

Issue: #3177


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

Cause use of anything other than 'true' or 'false' (case-insensitive)
fail in conversion, to make potential typos explicitly visible (instead
of implicitly just treating all else as 'false')

Includes a few tests for failures on invalid values, for conversions
handled by StringToBooleanAndCharPrimitiveConverter.

Issue: #3177
Copy link
Member

@sbrannen sbrannen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting the PR so quickly!

I requested a few minor changes.

@s-rwe
Copy link
Contributor Author

s-rwe commented Feb 27, 2023

Thanks for submitting the PR so quickly!

I requested a few minor changes.

Sure thing, likewise thanks for the quick review :)

Requested changes pushed accordingly.

Copy link
Member

@sbrannen sbrannen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposal looks good to me from a technical standpoint.

All that's missing is an entry in the release notes and, potentially, a note in the user guide.

Though I think we should hold off on those last two tasks until the rest of the team has voiced their approval.

@marcphilipp
Copy link
Member

Team Decision: Make it more strict by default since it's a more sane behavior.

@marcphilipp
Copy link
Member

@s-rwe Could you please add an entry to the release notes and update the table in the User Guide?

@s-rwe
Copy link
Contributor Author

s-rwe commented Mar 3, 2023

@s-rwe Could you please add an entry to the release notes and update the table in the User Guide?

Ok sure thanks, done accordingly.

@marcphilipp marcphilipp assigned marcphilipp and unassigned sbrannen Apr 28, 2023
@marcphilipp
Copy link
Member

I'll take it from here.

@marcphilipp marcphilipp changed the title Force using 'true' or 'false' for boolean args Make String to Boolean parameter conversion more strict Apr 28, 2023
@marcphilipp marcphilipp merged commit f79e38b into junit-team:main Apr 28, 2023
@marcphilipp
Copy link
Member

@s-rwe Thank you for you contribution! 🙇‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implicit boolean conversion in parameterized tests should be strict and accept only true or false
3 participants