You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@iceberg.apache.org by Peter Vary <pv...@cloudera.com.INVALID> on 2021/03/01 18:24:53 UTC

Default TimeZone for unit tests

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

Re: Default TimeZone for unit tests

Posted by Ryan Blue <rb...@netflix.com.INVALID>.
I agree with not randomizing CI runs. It's frustrating when CI failures
aren't reproducible. We could possibly have a base class for tests that
alters the JVM time zone and runs parameterized. Seems like some work, but
it would probably be worth it in the end.

On Tue, Mar 2, 2021 at 5:38 AM Marton Bod <ma...@gmail.com> wrote:

> 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.spitzer@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 <ow...@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.rodriguez@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.spitzer@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
>>>
>>
>>
>>

-- 
Ryan Blue
Software Engineer
Netflix

Re: Default TimeZone for unit tests

Posted by Marton Bod <ma...@gmail.com>.
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.spitzer@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 <ow...@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.rodriguez@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.spitzer@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
>>
>
>
>

Re: Default TimeZone for unit tests

Posted by Peter Vary <pv...@cloudera.com.INVALID>.
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.spitzer@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.omalley@gmail.com <ma...@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.rodriguez@airbnb.com.invalid <ma...@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 <rblue@netflix.com.invalid <ma...@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.spitzer@gmail.com <ma...@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 <pvary@cloudera.com.invalid <ma...@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
>> 


Re: Default TimeZone for unit tests

Posted by ru...@gmail.com.
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 <ow...@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 <ed...@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 <ru...@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
> 

Re: Default TimeZone for unit tests

Posted by Peter Vary <pv...@cloudera.com.INVALID>.
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 <ow...@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 <ed...@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.spitzer@gmail.com <ma...@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


Re: Default TimeZone for unit tests

Posted by Owen O'Malley <ow...@gmail.com>.
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
<ed...@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.spitzer@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
>

Re: Default TimeZone for unit tests

Posted by Edgar Rodriguez <ed...@airbnb.com.INVALID>.
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 <ru...@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

Re: Default TimeZone for unit tests

Posted by Ryan Blue <rb...@netflix.com.INVALID>.
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 <ru...@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

Re: Default TimeZone for unit tests

Posted by Russell Spitzer <ru...@gmail.com>.
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
>