-
Notifications
You must be signed in to change notification settings - Fork 680
Remove TimeMarkCompat / MonotonicTimeSourceCompat / timeInMillis #4105
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
Conversation
7154db3
to
6ad1a78
Compare
TimeMarkCompat was only necessary because of a breaking change in Kotlin 1.7 - but that was 2 years ago and Kotest now requires more recent versions, so TimeMarkCompat can now be removed. timeInMillis is only used for measuring times, so it can be replaced with stdlib `measureTime {}`/`measureTimedValue {}`
6ad1a78
to
13ddc72
Compare
# Conflicts: # kotest-common/api/kotest-common.api
nice update |
… they're pretty-printed using Duration#toString)
try { | ||
result = f() | ||
listener?.invoke(ContinuallyState(result, start, end, times)) | ||
listener?.invoke(ContinuallyState(result, 0L, duration.inWholeMilliseconds, times)) |
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.
Just to highlight, there's a potentially breaking change here.
The start and end of a ContinuallyState will no longer reflect the system time, but start will always be 0, and end will always be the configured duration.
I think this change is acceptable because it's a minor change, and this code has been deprecated for ~3 years.
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.
agreed, all deprecated functions are coming out for 6.0
# Conflicts: # kotest-assertions/kotest-assertions-core/src/jvmTest/kotlin/io/kotest/assertions/nondeterministic/EventuallyTest.kt # kotest-assertions/kotest-assertions-shared/src/jvmTest/kotlin/com/sksamuel/kotest/assertions/until/UntilTest.kt
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.
Excellent!
Use a test TimeSource in `continually` tests. - The test TimeSource uses a virtual time, so the tests run faster. - The elapsed duration is reproducible, so the tests can assert the specific elapsed time. - Use CoroutineContext to store & retrieve the TimeSource (This helps with testing both `continually(duration)` and `continually(config)` functions, and doesn't require adding an option in `ContinuallyConfiguration`) ### Dependencies * #4105
Use a test TimeSource in `eventually` tests. - The test TimeSource uses a virtual time, so the tests run faster. - The elapsed duration is reproducible, so the tests can assert the specific elapsed time. - Use CoroutineContext to store & retrieve the TimeSource (This helps with testing both `eventually(duration)` and `eventually(config)` functions, and doesn't require adding an option in `EventuallyConfiguration`) Depends on: #4105 Related: #4109 --------- Co-authored-by: OliverO2 <Oliver.O456i@gmail.com>
TimeMarkCompat was only necessary because of a breaking change in Kotlin 1.7 - but that was 2 years ago and Kotest now requires more recent versions, so TimeMarkCompat can now be removed. Slack discussion.
timeInMillis()
is only used for measuring times, so it can be replaced with stdlibmeasureTime {}
/measureTimedValue {}