bitfireAT/ical4android

Tests should work under every system time zone

sunkup opened this issue · 7 comments

sunkup commented

I noticed some tests fail when run in certain time zones. This is due to flipping dates when time of a DateTime object is close to midnight. For now I found the following tests fail with time zone America/New_York:

  • testRecurrenceSetsToAndroidString_TimeAlthoughAllDay
  • testRecurrenceSetsToAndroidString_Date
  • testHasUntilBeforeDtStart_DtStartDate_RRuleUntil_TimeBeforeDtStart_noTimezone
rfc2822 commented

@ArnyminerZ Could you please try to reproduce this? I think it should be enough to set the emulator to America/New_York and then run the ical4android tests.

  • 👍1
ArnyminerZ commented

After setting the timezone of the emulator to GMT+04:00 Eastern Daylight Time:

  • testHasUntilBeforeDtStart_DtStartDate_RRuleUntil_TimeBeforeDtStart_noTimezone is passing.
  • testRecurrenceSetsToAndroidString_Date fails with: 
    org.junit.ComparisonFailure: expected:<201[50101T000000Z,20150702]T000000Z> but was:<201[41231T000000Z,20150701]T000000Z>
    at org.junit.Assert.assertEquals(Assert.java:117)
    at org.junit.Assert.assertEquals(Assert.java:146)
    at at.bitfire.ical4android.util.AndroidTimeUtilsTest.testRecurrenceSetsToAndroidString_Date(AndroidTimeUtilsTest.kt:325)
    at java.lang.reflect.Method.invoke(Native Method)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
    at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
    at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
    at org.junit.runners.Suite.runChild(Suite.java:128)
    at org.junit.runners.Suite.runChild(Suite.java:27)
    at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
    at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
    at androidx.test.internal.runner.TestExecutor.execute(TestExecutor.java:67)
    at androidx.test.internal.runner.TestExecutor.execute(TestExecutor.java:58)
    at androidx.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:446)
    at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2248)
  • testRecurrenceSetsToAndroidString_TimeAlthoughAllDay fails with: 
    org.junit.ComparisonFailure: expected:<...0101T000000Z,2015070[2]T000000Z> but was:<...0101T000000Z,2015070[1]T000000Z>
    at org.junit.Assert.assertEquals(Assert.java:117)
    at org.junit.Assert.assertEquals(Assert.java:146)
    at at.bitfire.ical4android.util.AndroidTimeUtilsTest.testRecurrenceSetsToAndroidString_TimeAlthoughAllDay(AndroidTimeUtilsTest.kt:342)
    at java.lang.reflect.Method.invoke(Native Method)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
    at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
    at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
    at org.junit.runners.Suite.runChild(Suite.java:128)
    at org.junit.runners.Suite.runChild(Suite.java:27)
    at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
    at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
    at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:115)
    at androidx.test.internal.runner.TestExecutor.execute(TestExecutor.java:67)
    at androidx.test.internal.runner.TestExecutor.execute(TestExecutor.java:58)
    at androidx.test.runner.AndroidJUnitRunner.onStart(AndroidJUnitRunner.java:446)
    at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2248)
ArnyminerZ commented

I'm not sure that it's a correct approach, but I've tried to pass the tests by adding tzDefault when creating the DateList, and then Regex-ing the response ignoring the current timezone:

@Test
fun testRecurrenceSetsToAndroidString_Date() {
    // DATEs (without time) have to be converted to <date>T000000Z for Android
    val list = ArrayList<DateListProperty>(1)
    list.add(RDate(DateList("20150101,20150702", Value.DATE, tzDefault)))
    val androidString = AndroidTimeUtils.recurrenceSetsToAndroidString(list, true)
    // We ignore the timezone
    assertTrue("^.*/.*;20150101T000000Z,20150702T000000Z$".toRegex().matches(androidString))
}

@Test
fun testRecurrenceSetsToAndroidString_TimeAlthoughAllDay() {
    // DATE-TIME (floating time or UTC) recurrences for all-day events have to converted to <date>T000000Z for Android
    val list = ArrayList<DateListProperty>(1)
    list.add(RDate(DateList("20150101T000000,20150702T000000Z", Value.DATE_TIME, tzDefault)))
    val androidString = AndroidTimeUtils.recurrenceSetsToAndroidString(list, true)
    // We ignore the timezone
    assertTrue("^.*/.*;20150101T000000Z,20150702T000000Z$".toRegex().matches(androidString))
}
ArnyminerZ commented

After running all the tests on the GMT+04:00 Eastern Daylight Time, testMinifyVTimezone_keepFutureObservances is failing for me.

Logs:

at.bitfire.ical4android.ICalendarTest > testMinifyVTimezone_keepFutureObservances[Pixel_4_API_31(AVD) - 12] FAILED 
	java.lang.AssertionError: expected:<19160501T000000> but was:<19160501T010000>
	at org.junit.Assert.fail(Assert.java:89)

Which is strange, because testMinifyVTimezone_keepFutureObservances explicitely specifies timezone...

Edit, the failing assert is:

assertEquals(DateTime("19160430T230000"), minified.observances[2].startDate.date)
sunkup commented

Yes, timezones are always fun. Were you able to reproduce it with America/New_York ?

This is btw a nice relevant tool which I have grown to use for this stuff: https://currentmillis.com/

  • 👀1
rfc2822 commented

It would be good to know whether the tests should run on all time zones or whether a specific time zone is just required because of the means of the test (for instance because of DST expectations).

Then we could either make the tested code independent from the time zone or set the required time zones in the tests.

  • 👍1
ArnyminerZ commented

Okay, so I have been taking a look at the different tests and:

  • testRecurrenceSetsToAndroidString_TimeAlthoughAllDay should work on every timezone. There's no reason why it shouldn't work on an specific one. However, since we are "hardcoding" the date, if the timezone specified is different than the one that set the date, it will fail. I suggest specifying the correct timezone, and directly ignoring it on the test, something like: 
    @Test
fun testRecurrenceSetsToAndroidString_TimeAlthoughAllDay() {
    // DATE-TIME (floating time or UTC) recurrences for all-day events have to converted to <date>T000000Z for Android
    val list = ArrayList<DateListProperty>(1)
    list.add(RDate(DateList("20150101T000000,20150702T000000Z", Value.DATE_TIME, tzDefault)))
    val androidString = AndroidTimeUtils.recurrenceSetsToAndroidString(list, true)
    // We ignore the timezone
    assertTrue("^.*/.*;20150101T000000Z,20150702T000000Z$".toRegex().matches(androidString))
}
  • testRecurrenceSetsToAndroidString_Date: Same goes for this one, the scope of the test is almost the same, so it should be able to run correctly on all timezones. The same approach can be used: 
    @Test
fun testRecurrenceSetsToAndroidString_Date() {
    // DATEs (without time) have to be converted to <date>T000000Z for Android
    val list = ArrayList<DateListProperty>(1)
    list.add(RDate(DateList("20150101,20150702", Value.DATE, tzDefault)))
    val androidString = AndroidTimeUtils.recurrenceSetsToAndroidString(list, true)
    // We ignore the timezone
    assertTrue("^.*/.*;20150101T000000Z,20150702T000000Z$".toRegex().matches(androidString))
}
  • testMinifyVTimezone_keepFutureObservances: it's currently passing for me on the New York timezone, so not sure what happened before...