Perhaps we can randomly pick a timezone from a smaller, predefined list?
That should make it more straightforward to check how the random zone was
picked on the CI.
The list could include UTC, along with one zone each from the
Northern/Southern/Eastern/Western hemispheres (including at least one zone
with half-hour offset) and the boundary time zones such as UTC+13 and
UTC-11.
I think this approach could also catch errors related to our handling of
daylight savings time as well.
Marton
On Tue, 2 Mar 2021 at 14:28, Peter Vary <pv...@cloudera.com.invalid> wrote:
> I would try to avoid randomness in any CI runs.
> I might be able to accept it if it is very easy/straightforward to repro
> the random generation.
>
> Otherwise we might end up with randomly failing CI tests which we can not
> repro.
>
> On Mar 2, 2021, at 13:40, russell.spit...@gmail.com wrote:
>
> What if we had the Ci time zone be random?
>
> Sent from my iPhone
>
> On Mar 2, 2021, at 5:54 AM, Peter Vary <pv...@cloudera.com.invalid> wrote:
>
> Maybe separating the unit tests for this would not worth the effort.
>
> I was thinking we should run the tests like this:
>
> *diff --git a/build.gradle b/build.gradle*
> *index fd716e12..9b361ea7 100644*
> *--- a/build.gradle*
> *+++ b/build.gradle*
> @@ -133,6 +133,9 @@ subprojects {
> }
>
>
> test {
> + if (project.hasProperty("isCI")) {
> + systemProperty "user.timezone", "Asia/Colombo"
> + }
> def logDir = "${rootDir}/build/testlogs"
> def logFile = "${logDir}/${project.name}.log"
> mkdir("${logDir}")
>
>
> This would set the timezone to a specific non-UTC TZ on the CI server only.
> Pros:
>
> - We know that at least once we run the tests with non-UTC timezone
> - By default every dev runs the tests in their own TZ, so we still
> test with multiple TZs
>
> Cons:
>
> - The TZ most often tested will change from UTC to the selected one
>
>
> For me it looks like a simple change which should be ok if we want to
> change the TZ.
>
> Thanks,
> Peter
>
> On Mar 1, 2021, at 22:34, Owen O'Malley <owen.omal...@gmail.com> wrote:
>
> In ORC, the timezone tests vary the default timezone through multiple
> values using the Java APIs. (They do restore the initial value when the
> test exits.) :)
>
> .. Owen
>
> On Mon, Mar 1, 2021 at 9:25 PM Edgar Rodriguez <
> edgar.rodrig...@airbnb.com.invalid> wrote:
>
>> Hi folks,
>>
>> Thanks Peter for the quick fix!
>>
>> I do think it'd be a good idea to have this kind of coverage to some
>> extent. Usually, a workflow some users follow is to only run locally the
>> modules that they modify and rely on the CI to run the full check which
>> takes longer, which makes room for these issues to land in master while
>> eventually someone will find the broken test. However, I do agree that we
>> probably should not spend a large amount of time on this - ideally if this
>> is possible in CI that'd be great e.g. having two CI jobs, one for UTC and
>> another for a different TZ.
>>
>> Cheers,
>>
>> On Mon, Mar 1, 2021 at 2:52 PM Ryan Blue <rb...@netflix.com.invalid>
>> wrote:
>>
>>> I'm not sure it would be worth separating out the timezone tests to do
>>> this. I think we catch these problems pretty quickly with the number of
>>> users building in different zones. Is this something we want to spend time
>>> on?
>>>
>>> On Mon, Mar 1, 2021 at 10:29 AM Russell Spitzer <
>>> russell.spit...@gmail.com> wrote:
>>>
>>>> In the Spark Cassandra Connector we had a similar issue, we would
>>>> specifically spawn test JVM's with different default local time zones to
>>>> make sure we handled these use cases, I also would make our test dates ones
>>>> on gregorian calendar boundaries so being an hour off with result in a
>>>> timestamp that would end up actually being several days off so it was
>>>> clear.
>>>>
>>>> So maybe it makes sense to break out some timestamp specific tests and
>>>> have them run with different local timezones? Then you have a UTC, PST, CEU
>>>> or whatever test suites to run. If we scope this to just timestamp specific
>>>> tests it shouldn't be that much more expensive and I do think the coverage
>>>> is important.
>>>>
>>>> On Mon, Mar 1, 2021 at 12:25 PM Peter Vary <pv...@cloudera.com.invalid>
>>>> wrote:
>>>>
>>>>> Hi Team,
>>>>>
>>>>> Last weekend I caused a little bit of stir by pushing changes which
>>>>> had a green run on CI, but was failing locally if the default TZ was
>>>>> different than UTC.
>>>>>
>>>>> Do we want to set the TZ of the CI tests to some random non-UTC TZ to
>>>>> catch these errors?
>>>>>
>>>>> Pros:
>>>>>
>>>>> - We can catch tests which are only working in UTC
>>>>>
>>>>>
>>>>> Cons:
>>>>>
>>>>> - I think the typical TZ is UTC in our target environments, so
>>>>> catching UTC problems might be more important
>>>>>
>>>>>
>>>>> I am interested in your thoughts about this.
>>>>>
>>>>> Thanks,
>>>>> Peter
>>>>>
>>>>
>>>
>>> --
>>> Ryan Blue
>>> Software Engineer
>>> Netflix
>>>
>>
>>
>> --
>> Edgar R
>>
>
>
>
