Skip to content

Conversation

@javabrett
Copy link
Contributor

What changes were proposed in this pull request?

Stop using the abbreviated and ambiguous timezone "EST" in a test, since it is machine-local default timezone dependent, and fails in different timezones. Fixed SPARK-15723.

How was this patch tested?

Note that to reproduce this problem in any locale/timezone, you can modify the scalatest-maven-plugin argLine to add a timezone:

<argLine>-ea -Xmx3g -XX:MaxPermSize=${MaxPermGen} -XX:ReservedCodeCacheSize=${CodeCacheSize} -Duser.timezone="Australia/Sydney"</argLine>

and run

$ mvn test -DwildcardSuites=org.apache.spark.status.api.v1.SimpleDateParamSuite -Dtest=none. Equally this will fix it in an effected timezone:

<argLine>-ea -Xmx3g -XX:MaxPermSize=${MaxPermGen} -XX:ReservedCodeCacheSize=${CodeCacheSize} -Duser.timezone="America/New_York"</argLine>

To test the fix, apply the above change to pom.xml to set test TZ to Australia/Sydney, and confirm the test now passes.

…ambiguous. Use -0500 instead. Fixed SPARK-15723.
@srowen
Copy link
Member

srowen commented Jun 2, 2016

That much seems reasonable, but should we also try to fix the TimeZone with which Spark does the parsing to begin with? it shouldn't really be up to the machine's environment.

@srowen
Copy link
Member

srowen commented Jun 2, 2016

Also have a look at https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark
The title should for example include SPARK-15723

@srowen
Copy link
Member

srowen commented Jun 2, 2016

Jenkins test this please

@SparkQA
Copy link

SparkQA commented Jun 2, 2016

Test build #59849 has finished for PR 13462 at commit 7dfc17b.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@javabrett javabrett changed the title Fixed local-timezone-brittle test where short-timezone form "EST" is … SPARK-15723: Fixed local-timezone-brittle test where short-timezone form "EST" is … Jun 3, 2016
@srowen
Copy link
Member

srowen commented Jun 4, 2016

@javabrett have a look at my comment on the JIRA, what do you think? this might be more fix-able.

@asfgit asfgit closed this in 4e767d0 Jun 5, 2016
asfgit pushed a commit that referenced this pull request Jun 5, 2016
…form "EST" is …

## What changes were proposed in this pull request?

Stop using the abbreviated and ambiguous timezone "EST" in a test, since it is machine-local default timezone dependent, and fails in different timezones.  Fixed [SPARK-15723](https://issues.apache.org/jira/browse/SPARK-15723).

## How was this patch tested?

Note that to reproduce this problem in any locale/timezone, you can modify the scalatest-maven-plugin argLine to add a timezone:

    <argLine>-ea -Xmx3g -XX:MaxPermSize=${MaxPermGen} -XX:ReservedCodeCacheSize=${CodeCacheSize} -Duser.timezone="Australia/Sydney"</argLine>

and run

    $ mvn test -DwildcardSuites=org.apache.spark.status.api.v1.SimpleDateParamSuite -Dtest=none. Equally this will fix it in an effected timezone:

    <argLine>-ea -Xmx3g -XX:MaxPermSize=${MaxPermGen} -XX:ReservedCodeCacheSize=${CodeCacheSize} -Duser.timezone="America/New_York"</argLine>

To test the fix, apply the above change to `pom.xml` to set test TZ to `Australia/Sydney`, and confirm the test now passes.

Author: Brett Randall <[email protected]>

Closes #13462 from javabrett/SPARK-15723-SimpleDateParamSuite.

(cherry picked from commit 4e767d0)
Signed-off-by: Sean Owen <[email protected]>
asfgit pushed a commit that referenced this pull request Jun 5, 2016
…form "EST" is …

## What changes were proposed in this pull request?

Stop using the abbreviated and ambiguous timezone "EST" in a test, since it is machine-local default timezone dependent, and fails in different timezones.  Fixed [SPARK-15723](https://issues.apache.org/jira/browse/SPARK-15723).

## How was this patch tested?

Note that to reproduce this problem in any locale/timezone, you can modify the scalatest-maven-plugin argLine to add a timezone:

    <argLine>-ea -Xmx3g -XX:MaxPermSize=${MaxPermGen} -XX:ReservedCodeCacheSize=${CodeCacheSize} -Duser.timezone="Australia/Sydney"</argLine>

and run

    $ mvn test -DwildcardSuites=org.apache.spark.status.api.v1.SimpleDateParamSuite -Dtest=none. Equally this will fix it in an effected timezone:

    <argLine>-ea -Xmx3g -XX:MaxPermSize=${MaxPermGen} -XX:ReservedCodeCacheSize=${CodeCacheSize} -Duser.timezone="America/New_York"</argLine>

To test the fix, apply the above change to `pom.xml` to set test TZ to `Australia/Sydney`, and confirm the test now passes.

Author: Brett Randall <[email protected]>

Closes #13462 from javabrett/SPARK-15723-SimpleDateParamSuite.

(cherry picked from commit 4e767d0)
Signed-off-by: Sean Owen <[email protected]>
@srowen
Copy link
Member

srowen commented Jun 5, 2016

Merged to master/2.0/1.6. I admit I made an error above, in that I thought this test had passed the PR builder, but actually I was mixed up and it had not. However I verified locally and on the Amplab builds that this test does in fact pass, so, got away with it.

@javabrett javabrett deleted the SPARK-15723-SimpleDateParamSuite branch June 5, 2016 16:08
zzcclp pushed a commit to zzcclp/spark that referenced this pull request Jun 6, 2016
…form "EST" is …

## What changes were proposed in this pull request?

Stop using the abbreviated and ambiguous timezone "EST" in a test, since it is machine-local default timezone dependent, and fails in different timezones.  Fixed [SPARK-15723](https://issues.apache.org/jira/browse/SPARK-15723).

## How was this patch tested?

Note that to reproduce this problem in any locale/timezone, you can modify the scalatest-maven-plugin argLine to add a timezone:

    <argLine>-ea -Xmx3g -XX:MaxPermSize=${MaxPermGen} -XX:ReservedCodeCacheSize=${CodeCacheSize} -Duser.timezone="Australia/Sydney"</argLine>

and run

    $ mvn test -DwildcardSuites=org.apache.spark.status.api.v1.SimpleDateParamSuite -Dtest=none. Equally this will fix it in an effected timezone:

    <argLine>-ea -Xmx3g -XX:MaxPermSize=${MaxPermGen} -XX:ReservedCodeCacheSize=${CodeCacheSize} -Duser.timezone="America/New_York"</argLine>

To test the fix, apply the above change to `pom.xml` to set test TZ to `Australia/Sydney`, and confirm the test now passes.

Author: Brett Randall <[email protected]>

Closes apache#13462 from javabrett/SPARK-15723-SimpleDateParamSuite.

(cherry picked from commit 4e767d0)
Signed-off-by: Sean Owen <[email protected]>
(cherry picked from commit 6a9f19d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants