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

NSDate.toKotlinInstant off by 1 millisecond #439

Closed
jeffdgr8 opened this issue Sep 25, 2024 · 1 comment
Closed

NSDate.toKotlinInstant off by 1 millisecond #439

jeffdgr8 opened this issue Sep 25, 2024 · 1 comment

Comments

@jeffdgr8
Copy link

jeffdgr8 commented Sep 25, 2024

NSDate.toKotlinInstant adds one millisecond to the resulting Instant. This was working in 0.5.0, but broken in 0.6.0-RC. The test does not always fail, but does about 50% of the time.

val now = Clock.System.now()
val nsDate = now.toNSDate()
val instant = nsDate.toKotlinInstant()
assertEquals(now.toEpochMilliseconds(), (nsDate.timeIntervalSince1970 * 1000).toLong())
assertEquals(now.toEpochMilliseconds(), instant.toEpochMilliseconds()) // fails 50% of the time in 0.6.0

kotlin.AssertionError: Expected <1727302719887>, actual <1727302719888>.

@dkhalanskyjb
Copy link
Collaborator

dkhalanskyjb commented Sep 26, 2024

The last change to NSDate.toKotlinInstant was in 6c22ce6, a commit made for v0.1. The logic since the beginning was to round the fractional second portion to whole milliseconds, as the documentation states.

50% is the number of fractional parts that will be rounded up, so the numbers check out.

What did change was Clock.System.now(): now it uses the more robust API, which reliably returns even the nanosecond portion. That's why now in your example didn't have the sub-millisecond portion in 0.5.0, but does now: the behavior that surprised you was always there (and is documented), but now that the precision we're using increased, you can actually observe it.

#427 is probably the issue you're interested in: we are considering being more lenient with rounding and only round to microseconds, or maybe not at all.

@dkhalanskyjb dkhalanskyjb closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2024
jeffdgr8 added a commit to jeffdgr8/kotbase that referenced this issue Sep 26, 2024
Force millisecond timestamps for equality
Kotlin/kotlinx-datetime#439
jeffdgr8 added a commit to jeffdgr8/kotbase that referenced this issue Oct 24, 2024
Force millisecond timestamps for equality
Kotlin/kotlinx-datetime#439
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

No branches or pull requests

2 participants