Test for date conversion in multiple timezone

[shubham1g5]

Product Description

Mostly creating this to surface issue with Date widget conversion code.

[Test fails at the moment and PR only aims to surface the issue and should not be merged]

Summary by CodeRabbit

• Tests
  • Enhanced date conversion test to validate functionality across multiple timezone configurations. Expanded test coverage to iterate through various timezone settings, ensuring date conversion operations maintain consistency and reliability across different system timezone environments with proper resource cleanup.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

A test method was renamed and enhanced to verify date conversion behavior across multiple timezones by iterating through different TimeZone settings, ensuring consistent conversion and reverse conversion operations, with proper restoration of the original timezone.

Changes

Cohort / File(s)	Summary
Test Enhancement src/test/java/org/commcare/util/DateRangeUtilsTest.java	Method testDateConversion() renamed to testDateConversionInMultipleTimezones() and refactored to iterate over multiple TimeZone settings, running the same conversion assertions within a loop while preserving and restoring the default timezone. TimeZone import added.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A bunny tested dates with care,
Across timezones everywhere!
From PST to UTC so grand,
Each conversion, perfectly planned,
Now clocks tick true in every land! 🕐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: renaming and enhancing the test method to validate date conversion across multiple timezones.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch testDateConversion

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7eccc74 and 1ad0aa9.

📒 Files selected for processing (1)

• src/test/java/org/commcare/util/DateRangeUtilsTest.java

🧰 Additional context used

🧬 Code graph analysis (1)

src/test/java/org/commcare/util/DateRangeUtilsTest.java (1)

src/main/java/org/commcare/util/DateRangeUtils.java (1)

• DateRangeUtils (12-81)

🪛 GitHub Actions: CommCare Core Build

src/test/java/org/commcare/util/DateRangeUtilsTest.java

[error] 29-29: JUnit ComparisonFailure in testDateConversionInMultipleTimezones. 710 tests completed, 1 failed during Gradle test task.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: task-list-completed

🔇 Additional comments (4)

src/test/java/org/commcare/util/DateRangeUtilsTest.java (4)

8-8: LGTM! Necessary import for timezone testing.

The TimeZone import is correctly added to support the multi-timezone test functionality.

---

13-14: LGTM! Good timezone coverage for the test.

The method rename clearly reflects the expanded test scope, and the timezone selection covers diverse geographic locations (UTC, US East/West coast, Europe, and Asia) which will effectively surface timezone-dependent behavior.

---

35-37: LGTM! Proper cleanup in finally block.

The finally block ensures the timezone is always restored even if assertions fail, which is the correct practice for tests that modify global state. Once the originalTz capture is moved outside the loop, this cleanup will work correctly.

---

19-34: Test correctly surfaces the timezone issue in the date range conversion logic.

The test logic is sound and properly validates both forward and reverse conversion across timezones. The diagnostic output (lines 24-27) provides helpful debugging context. This test will correctly fail in non-UTC timezones, surfacing the root cause.

The underlying issue is in DateRangeUtils.getTimeFromDateOffsettingTz() (line 39-40), which applies a double timezone adjustment: SimpleDateFormat parses the input date string according to the default timezone, and then getTimeFromDateOffsettingTz() subtracts the timezone offset again using the deprecated Date.getTimezoneOffset() method. This double adjustment causes date corruption across different timezones.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Critical: Move originalTz capture outside the loop.

The originalTz variable is captured inside the loop, which means it will be overwritten on each iteration after the first. After the first iteration, the "original" timezone being restored will actually be the previous test's timezone, not the actual system default. This could lead to cascading failures or incorrect test behavior.

🔎 Proposed fix

-        for (String tzId : timezones) {
-            TimeZone originalTz = TimeZone.getDefault();
+        TimeZone originalTz = TimeZone.getDefault();
+        for (String tzId : timezones) {
             try {
                 TimeZone.setDefault(TimeZone.getTimeZone(tzId));

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	for (String tzId : timezones) {
	TimeZone originalTz = TimeZone.getDefault();
	TimeZone originalTz = TimeZone.getDefault();
	for (String tzId : timezones) {
	try {
	TimeZone.setDefault(TimeZone.getTimeZone(tzId));

🤖 Prompt for AI Agents

In src/test/java/org/commcare/util/DateRangeUtilsTest.java around lines 16 to
17, the TimeZone originalTz is captured inside the for-loop so it gets
overwritten each iteration; move the capture of TimeZone originalTz =
TimeZone.getDefault() to immediately before the loop so it reflects the true
system default, then inside the loop set the test TimeZone and restore
TimeZone.setDefault(originalTz) after each iteration (and/or finally) to ensure
each test resets to the real original timezone.