From 61afb7b267cc8a8513c081aae7f80935ecc59171 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 11:53:22 +0200 Subject: [PATCH 1/9] Add test facilities for catching a flaky test --- core/native/src/internal/TimeZoneRules.kt | 15 +++++++++++++++ core/windows/test/TimeZoneRulesCompleteTest.kt | 7 ++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/core/native/src/internal/TimeZoneRules.kt b/core/native/src/internal/TimeZoneRules.kt index c712ff4a..a5f4e964 100644 --- a/core/native/src/internal/TimeZoneRules.kt +++ b/core/native/src/internal/TimeZoneRules.kt @@ -115,6 +115,21 @@ internal class TimeZoneRules( val offsetAfter = offsets[transitionIndex + 1] return OffsetInfo(transitionInstant, offsetBefore, offsetAfter) } + + override fun toString(): String = buildString { + for (i in transitionEpochSeconds.indices) { + append(offsets[i]) + append(" until ") + append(Instant.fromEpochSeconds(transitionEpochSeconds[i])) + append(", ") + } + append("then ") + append(offsets.last()) + if (recurringZoneRules != null) { + append(", after that") + append(recurringZoneRules) + } + } } internal class RecurringZoneRules( diff --git a/core/windows/test/TimeZoneRulesCompleteTest.kt b/core/windows/test/TimeZoneRulesCompleteTest.kt index c08bbbbc..3d37d663 100644 --- a/core/windows/test/TimeZoneRulesCompleteTest.kt +++ b/core/windows/test/TimeZoneRulesCompleteTest.kt @@ -42,7 +42,12 @@ class TimeZoneRulesCompleteTest { val ldt = instant.toLocalDateTime(dtzi, inputSystemtime.ptr, outputSystemtime.ptr) val offset = rules.infoAtInstant(instant) val ourLdt = instant.toLocalDateTime(offset) - assertEquals(ldt, ourLdt, "in zone $windowsName, at $instant (our guess at the offset is $offset)") + assertEquals( + ldt, + ourLdt, + "in zone $windowsName at $instant (our guess at the offset is $offset). " + + "The rules are $rules" + ) } fun checkTransition(instant: Instant) { checkAtInstant(instant - 2.milliseconds) From 4a2b3cff179ec1b7faf4aedae0cfad9bf868ec4d Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 12:39:02 +0200 Subject: [PATCH 2/9] Fix a discrepancy with Windows --- core/native/src/internal/TimeZoneRules.kt | 2 +- core/windows/src/internal/TzdbInRegistry.kt | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/core/native/src/internal/TimeZoneRules.kt b/core/native/src/internal/TimeZoneRules.kt index a5f4e964..0d56aceb 100644 --- a/core/native/src/internal/TimeZoneRules.kt +++ b/core/native/src/internal/TimeZoneRules.kt @@ -126,7 +126,7 @@ internal class TimeZoneRules( append("then ") append(offsets.last()) if (recurringZoneRules != null) { - append(", after that") + append(", after that ") append(recurringZoneRules) } } diff --git a/core/windows/src/internal/TzdbInRegistry.kt b/core/windows/src/internal/TzdbInRegistry.kt index e0c0bd31..f5781da6 100644 --- a/core/windows/src/internal/TzdbInRegistry.kt +++ b/core/windows/src/internal/TzdbInRegistry.kt @@ -60,7 +60,16 @@ internal class TzdbInRegistry: TimeZoneDatabase { } } } - if (offsets.isEmpty()) { offsets.add(recurring.offsetAtYearStart()) } + offsets.lastOrNull()?.let { lastOffset -> + /* If there are already some offsets, we can not add a new offset without defining a transition to + it. The moment when we start using the recurring rules is the first year that does not have any + historic data provided. */ + val firstYearWithRecurringRules = historic.last().first + 1 + val newYearInLastOffset = LocalDate(firstYearWithRecurringRules, Month.JANUARY, 1).atTime(0, 0) + .toInstant(lastOffset) + transitionEpochSeconds.add(newYearInLastOffset.epochSeconds) + } + offsets.add(recurring.offsetAtYearStart()) TimeZoneRules(transitionEpochSeconds, offsets, recurringRules) } put(name, rules) From 3eff33a9e8fd47fcbf45393ffbf0e19ff174fa6d Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 13:24:07 +0200 Subject: [PATCH 3/9] Attempt 2 --- core/windows/src/internal/TzdbInRegistry.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/windows/src/internal/TzdbInRegistry.kt b/core/windows/src/internal/TzdbInRegistry.kt index f5781da6..9d0aad9e 100644 --- a/core/windows/src/internal/TzdbInRegistry.kt +++ b/core/windows/src/internal/TzdbInRegistry.kt @@ -60,7 +60,10 @@ internal class TzdbInRegistry: TimeZoneDatabase { } } } - offsets.lastOrNull()?.let { lastOffset -> + val lastOffset = offsets.lastOrNull() + if (lastOffset == null) { + offsets.add(recurring.offsetAtYearStart()) + } else { /* If there are already some offsets, we can not add a new offset without defining a transition to it. The moment when we start using the recurring rules is the first year that does not have any historic data provided. */ @@ -68,8 +71,8 @@ internal class TzdbInRegistry: TimeZoneDatabase { val newYearInLastOffset = LocalDate(firstYearWithRecurringRules, Month.JANUARY, 1).atTime(0, 0) .toInstant(lastOffset) transitionEpochSeconds.add(newYearInLastOffset.epochSeconds) + offsets.add(offsets.last()) } - offsets.add(recurring.offsetAtYearStart()) TimeZoneRules(transitionEpochSeconds, offsets, recurringRules) } put(name, rules) From c8ee9221eedf8e618f7ce70b9e67b298513a5cc2 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 13:46:46 +0200 Subject: [PATCH 4/9] More diagnostic info --- core/windows/src/internal/TzdbInRegistry.kt | 11 ++++----- .../windows/test/TimeZoneRulesCompleteTest.kt | 24 ++++++++++++++----- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/core/windows/src/internal/TzdbInRegistry.kt b/core/windows/src/internal/TzdbInRegistry.kt index 9d0aad9e..e279228e 100644 --- a/core/windows/src/internal/TzdbInRegistry.kt +++ b/core/windows/src/internal/TzdbInRegistry.kt @@ -60,10 +60,7 @@ internal class TzdbInRegistry: TimeZoneDatabase { } } } - val lastOffset = offsets.lastOrNull() - if (lastOffset == null) { - offsets.add(recurring.offsetAtYearStart()) - } else { + offsets.lastOrNull()?.let { lastOffset -> /* If there are already some offsets, we can not add a new offset without defining a transition to it. The moment when we start using the recurring rules is the first year that does not have any historic data provided. */ @@ -71,8 +68,8 @@ internal class TzdbInRegistry: TimeZoneDatabase { val newYearInLastOffset = LocalDate(firstYearWithRecurringRules, Month.JANUARY, 1).atTime(0, 0) .toInstant(lastOffset) transitionEpochSeconds.add(newYearInLastOffset.epochSeconds) - offsets.add(offsets.last()) } + offsets.add(recurring.offsetAtYearStart()) TimeZoneRules(transitionEpochSeconds, offsets, recurringRules) } put(name, rules) @@ -335,7 +332,9 @@ private class PerYearZoneRulesDataWithTransitions( val standardTransition get() = RecurringZoneRules.Rule(standardTransitionTime, offsetBefore = daylightOffset, offsetAfter = standardOffset) - override fun offsetAtYearStart(): UtcOffset = standardOffset // TODO: not true in all years + all zones + override fun offsetAtYearStart(): UtcOffset = + if (daylightTransitionTime.toLocalDateTime(2030) < standardTransitionTime.toLocalDateTime(2030)) + standardOffset else daylightOffset override fun toString(): String = "standard offset is $standardOffset" + ", daylight offset is $daylightOffset" + diff --git a/core/windows/test/TimeZoneRulesCompleteTest.kt b/core/windows/test/TimeZoneRulesCompleteTest.kt index 3d37d663..000dfac0 100644 --- a/core/windows/test/TimeZoneRulesCompleteTest.kt +++ b/core/windows/test/TimeZoneRulesCompleteTest.kt @@ -42,12 +42,24 @@ class TimeZoneRulesCompleteTest { val ldt = instant.toLocalDateTime(dtzi, inputSystemtime.ptr, outputSystemtime.ptr) val offset = rules.infoAtInstant(instant) val ourLdt = instant.toLocalDateTime(offset) - assertEquals( - ldt, - ourLdt, - "in zone $windowsName at $instant (our guess at the offset is $offset). " + - "The rules are $rules" - ) + if (ldt != ourLdt) { + val offsetsAccordingToWindows = buildList { + var date = LocalDate(ldt.year, Month.JANUARY, 1) + while (date.year == ldt.year) { + val instant = date.atTime(0, 0).toInstant(UtcOffset.ZERO) + val ldtAccordingToWindows = + instant.toLocalDateTime(dtzi, inputSystemtime.ptr, outputSystemtime.ptr) + val offsetAccordingToWindows = UtcOffset(null, null, + (ldtAccordingToWindows.toInstant(UtcOffset.ZERO) - instant).inWholeSeconds + ) + add(instant to offsetAccordingToWindows) + } + } + throw AssertionError( + "Expected $ldt, got $ourLdt in zone $windowsName at $instant (our guess at the offset is $offset)." + + "The rules are $rules, and the offsets throughout the year according to Windows are: $offsetsAccordingToWindows" + ) + } } fun checkTransition(instant: Instant) { checkAtInstant(instant - 2.milliseconds) From 92290e4a035bd2d5144c275ee1a2e805a0e35274 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 14:09:22 +0200 Subject: [PATCH 5/9] Something strange --- core/windows/test/TimeZoneRulesCompleteTest.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/windows/test/TimeZoneRulesCompleteTest.kt b/core/windows/test/TimeZoneRulesCompleteTest.kt index 000dfac0..cb429f43 100644 --- a/core/windows/test/TimeZoneRulesCompleteTest.kt +++ b/core/windows/test/TimeZoneRulesCompleteTest.kt @@ -49,10 +49,9 @@ class TimeZoneRulesCompleteTest { val instant = date.atTime(0, 0).toInstant(UtcOffset.ZERO) val ldtAccordingToWindows = instant.toLocalDateTime(dtzi, inputSystemtime.ptr, outputSystemtime.ptr) - val offsetAccordingToWindows = UtcOffset(null, null, + val offsetAccordingToWindows = (ldtAccordingToWindows.toInstant(UtcOffset.ZERO) - instant).inWholeSeconds - ) - add(instant to offsetAccordingToWindows) + add(date to offsetAccordingToWindows) } } throw AssertionError( From b1b7b5bc6f3ef50ec03c157e1279290e654fd2e1 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 14:21:29 +0200 Subject: [PATCH 6/9] Fix --- core/windows/test/TimeZoneRulesCompleteTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/core/windows/test/TimeZoneRulesCompleteTest.kt b/core/windows/test/TimeZoneRulesCompleteTest.kt index cb429f43..5a9c3083 100644 --- a/core/windows/test/TimeZoneRulesCompleteTest.kt +++ b/core/windows/test/TimeZoneRulesCompleteTest.kt @@ -52,6 +52,7 @@ class TimeZoneRulesCompleteTest { val offsetAccordingToWindows = (ldtAccordingToWindows.toInstant(UtcOffset.ZERO) - instant).inWholeSeconds add(date to offsetAccordingToWindows) + date = date.plus(1, DateTimeUnit.DAY) } } throw AssertionError( From 1d49eb14a409e7abbc61d84ffd93f07c2e38001d Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 14:42:16 +0200 Subject: [PATCH 7/9] Get MORE data --- .../windows/test/TimeZoneRulesCompleteTest.kt | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/core/windows/test/TimeZoneRulesCompleteTest.kt b/core/windows/test/TimeZoneRulesCompleteTest.kt index 5a9c3083..b00999d1 100644 --- a/core/windows/test/TimeZoneRulesCompleteTest.kt +++ b/core/windows/test/TimeZoneRulesCompleteTest.kt @@ -16,6 +16,7 @@ import kotlin.time.Duration.Companion.milliseconds class TimeZoneRulesCompleteTest { /** Tests that all transitions that our system recognizes are actually there. */ + @OptIn(ExperimentalStdlibApi::class) @Test fun iterateOverAllTimezones() { val tzdb = TzdbInRegistry() @@ -55,9 +56,21 @@ class TimeZoneRulesCompleteTest { date = date.plus(1, DateTimeUnit.DAY) } } + val rawData = memScoped { + alloc().withRegistryKey(HKEY_LOCAL_MACHINE!!, "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Time Zones\\$windowsName", { err -> + return@memScoped null + }) { hKey -> + val cbDataBuffer = alloc() + val SIZE_BYTES = 44 + val zoneInfoBuffer = allocArray(SIZE_BYTES) + cbDataBuffer.value = SIZE_BYTES.convert() + RegQueryValueExW(hKey, "TZI", null, null, zoneInfoBuffer, cbDataBuffer.ptr) + zoneInfoBuffer.readBytes(SIZE_BYTES).toHexString() + } + } throw AssertionError( "Expected $ldt, got $ourLdt in zone $windowsName at $instant (our guess at the offset is $offset)." + - "The rules are $rules, and the offsets throughout the year according to Windows are: $offsetsAccordingToWindows" + "The rules are $rules, and the offsets throughout the year according to Windows are: $offsetsAccordingToWindows; the raw data for the recurring rules is $rawData" ) } } @@ -139,3 +152,15 @@ private fun SYSTEMTIME.toLocalDateTime(): LocalDateTime = private val strangeTimeZones = listOf( "Morocco Standard Time", "West Bank Standard Time", "Iran Standard Time", "Syria Standard Time" ) + + +private inline fun HKEYVar.withRegistryKey(hKey: HKEY, subKeyName: String, onError: (Int) -> T, block: (HKEY) -> T): T { + val err = RegOpenKeyExW(hKey, subKeyName, 0u, KEY_READ.toUInt(), ptr) + return if (err != ERROR_SUCCESS) { onError(err) } else { + try { + block(value!!) + } finally { + RegCloseKey(value) + } + } +} From db467392b89db0d9db2a831bd5abec4840689c52 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 15:15:33 +0200 Subject: [PATCH 8/9] Give up --- core/windows/test/TimeZoneRulesCompleteTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/windows/test/TimeZoneRulesCompleteTest.kt b/core/windows/test/TimeZoneRulesCompleteTest.kt index b00999d1..d8924fe9 100644 --- a/core/windows/test/TimeZoneRulesCompleteTest.kt +++ b/core/windows/test/TimeZoneRulesCompleteTest.kt @@ -150,7 +150,8 @@ private fun SYSTEMTIME.toLocalDateTime(): LocalDateTime = ) private val strangeTimeZones = listOf( - "Morocco Standard Time", "West Bank Standard Time", "Iran Standard Time", "Syria Standard Time" + "Morocco Standard Time", "West Bank Standard Time", "Iran Standard Time", "Syria Standard Time", + "Paraguay Standard Time" ) From 9f01dc0300c20760480d48cc7d734038ef35b175 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 9 Aug 2024 15:19:15 +0200 Subject: [PATCH 9/9] Cleanup --- core/windows/src/internal/TzdbInRegistry.kt | 4 +--- .../windows/test/TimeZoneRulesCompleteTest.kt | 23 ++++++------------- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/core/windows/src/internal/TzdbInRegistry.kt b/core/windows/src/internal/TzdbInRegistry.kt index e279228e..f5781da6 100644 --- a/core/windows/src/internal/TzdbInRegistry.kt +++ b/core/windows/src/internal/TzdbInRegistry.kt @@ -332,9 +332,7 @@ private class PerYearZoneRulesDataWithTransitions( val standardTransition get() = RecurringZoneRules.Rule(standardTransitionTime, offsetBefore = daylightOffset, offsetAfter = standardOffset) - override fun offsetAtYearStart(): UtcOffset = - if (daylightTransitionTime.toLocalDateTime(2030) < standardTransitionTime.toLocalDateTime(2030)) - standardOffset else daylightOffset + override fun offsetAtYearStart(): UtcOffset = standardOffset // TODO: not true in all years + all zones override fun toString(): String = "standard offset is $standardOffset" + ", daylight offset is $daylightOffset" + diff --git a/core/windows/test/TimeZoneRulesCompleteTest.kt b/core/windows/test/TimeZoneRulesCompleteTest.kt index d8924fe9..34d3b868 100644 --- a/core/windows/test/TimeZoneRulesCompleteTest.kt +++ b/core/windows/test/TimeZoneRulesCompleteTest.kt @@ -7,6 +7,7 @@ package kotlinx.datetime.test import kotlinx.cinterop.* +import kotlinx.cinterop.ptr import kotlinx.datetime.* import kotlinx.datetime.internal.* import platform.windows.* @@ -57,15 +58,17 @@ class TimeZoneRulesCompleteTest { } } val rawData = memScoped { - alloc().withRegistryKey(HKEY_LOCAL_MACHINE!!, "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Time Zones\\$windowsName", { err -> - return@memScoped null - }) { hKey -> + val hKey = alloc() + RegOpenKeyExW(HKEY_LOCAL_MACHINE!!, "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Time Zones\\$windowsName", 0u, KEY_READ.toUInt(), hKey.ptr) + try { val cbDataBuffer = alloc() val SIZE_BYTES = 44 val zoneInfoBuffer = allocArray(SIZE_BYTES) cbDataBuffer.value = SIZE_BYTES.convert() - RegQueryValueExW(hKey, "TZI", null, null, zoneInfoBuffer, cbDataBuffer.ptr) + RegQueryValueExW(hKey.value, "TZI", null, null, zoneInfoBuffer, cbDataBuffer.ptr) zoneInfoBuffer.readBytes(SIZE_BYTES).toHexString() + } finally { + RegCloseKey(hKey.value) } } throw AssertionError( @@ -153,15 +156,3 @@ private val strangeTimeZones = listOf( "Morocco Standard Time", "West Bank Standard Time", "Iran Standard Time", "Syria Standard Time", "Paraguay Standard Time" ) - - -private inline fun HKEYVar.withRegistryKey(hKey: HKEY, subKeyName: String, onError: (Int) -> T, block: (HKEY) -> T): T { - val err = RegOpenKeyExW(hKey, subKeyName, 0u, KEY_READ.toUInt(), ptr) - return if (err != ERROR_SUCCESS) { onError(err) } else { - try { - block(value!!) - } finally { - RegCloseKey(value) - } - } -}