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.

Sign up for GitHub

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

Jump to bottom

LocalDateTime arb should accept localdatetimes as min and max #2704

Closed
Tracked by #2678
LeoColman opened this issue Dec 2, 2021 · 4 comments
Closed
Tracked by #2678

LocalDateTime arb should accept localdatetimes as min and max #2704

LeoColman opened this issue Dec 2, 2021 · 4 comments
Assignees
@pientaa
Labels
enhancement ✨ Suggestions for adding new features or improving existing ones. good-first-issue 👶 Suitable for newcomers looking to contribute to the project. property-testing 📏 Related to the property testing mechanisms within the testing framework.
Milestone
5.1

Comments

@LeoColman
Copy link
Member

kotest/kotest-property/src/jvmMain/kotlin/io/kotest/property/arbitrary/dates.kt

Line 87 in a139907

fun Arb.Companion.localDateTime(

The arbs for both localDate and localTime have an equal type as parameter. LocalDateTime doesn't. As a LocalDateTime is a LocalDate + LocalTime, this should not be an issue.

To fix this:

  1. Deprecate Arb.localDateTime(minYear: Int, maxYear: Int)
  2. Add Arb.localDateTime(minLocalDateTime: LocalDateTime, maxLocalDateTime: LocalDateTime)
  3. Add tests to the new function
@LeoColman LeoColman added enhancement ✨ Suggestions for adding new features or improving existing ones. good-first-issue 👶 Suitable for newcomers looking to contribute to the project. property-testing 📏 Related to the property testing mechanisms within the testing framework. labels Dec 2, 2021
@pientaa
Copy link
Contributor

pientaa commented Dec 2, 2021

I'd like to take that issue

@LeoColman
Copy link
Member Author

Absolutely!
Don't hesitate to ask for help if you need any :)

@sksamuel
Copy link
Member

sksamuel commented Dec 3, 2021

We can keep the old one as well as the new one if we want, since it's much simpler to use if you aren't too fussy on the values.

@LeoColman
Copy link
Member Author

I suggested deprecating it. Maybe keep it not deprecated?

@pientaa pientaa mentioned this issue Dec 5, 2021
Merged
@sksamuel sksamuel added this to the 5.1 milestone Dec 18, 2021
@sksamuel sksamuel mentioned this issue Dec 18, 2021
Closed
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pientaa pientaa

Labels
enhancement ✨ Suggestions for adding new features or improving existing ones. good-first-issue 👶 Suitable for newcomers looking to contribute to the project. property-testing 📏 Related to the property testing mechanisms within the testing framework.
Projects
None yet
Milestone
5.1
Development

No branches or pull requests
3 participants
@sksamuel @LeoColman @pientaa