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

Fix Off By One Error In JavaTimeChoose #769

Merged
merged 3 commits into from
Apr 5, 2021

Conversation

isomarcte
Copy link
Member

The minimum value of nanoseconds for both Duration and Instant should be 0, unless the second value == the minimum bound's second value. It was incorrectly encoded as 1.

Fixes #762

The minimum value of nanoseconds for both Duration and Instant should be 0, unless the second value == the minimum bound's second value. It was incorrectly encoded as 1.

Fixes typelevel#762
Base automatically changed from master to main March 26, 2021 18:56
@isomarcte
Copy link
Member Author

@larsrh @ashawley any thoughts here?

Copy link
Contributor

@ashawley ashawley left a comment

Choose a reason for hiding this comment

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

Worth adding a test or improving an exiting one to capture the failure?

@isomarcte
Copy link
Member Author

@ashawley will do

@isomarcte
Copy link
Member Author

@ashawley test added

The tests for the off by one error rely upon adding 1 second to the generated value. This _can_ cause an overflow if the generated Instant/Duration is less than 1 second from the max value the type can support.

This commit adds special generators to the GenSpecification for java.time values to ensure this never occurs. No one likes a flaky test.
@isomarcte isomarcte requested a review from ashawley April 1, 2021 18:48
@ashawley
Copy link
Contributor

ashawley commented Apr 3, 2021

Thanks for adding tests.

@larsrh larsrh merged commit 18718fc into typelevel:main Apr 5, 2021
@isomarcte isomarcte deleted the off-by-one-nanosecond branch April 5, 2021 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid bounds in JavaTimeChoose
3 participants