[SPARK-16531][SQL][TEST] Remove timezone setting from DataFrameTimeWindowingSuite

[brkyvz]

What changes were proposed in this pull request?

It's unnecessary. QueryTest already sets it.

[brkyvz]

cc @marmbrus

[SparkQA]

Test build #62206 has finished for PR 14170 at commit 01dbc42.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[marmbrus]

LGTM, can you make a JIRA? Its a little scary to change tests w/o one in case there is flakiness.

[marmbrus]

We should put this in 2.0 for whoever merges.

[brkyvz]

Created JIRA SPARK-16531

[marmbrus]

Thanks, merging to master and 2.0

[srowen]

It may be a secondary point, but QueryTest should reset the timezone like the code being removed does. Right now it means that the timezone default stays changed for any other tests in the JVM.

[marmbrus]

All the tests in SQL are written to assume Los_Angeles, so I think this is actually desired. Otherwise people have to configure their machine specially to run spark tests. The issue here was it was UTC, which I think was causing some flakyness when tests are run in parallel.

[srowen]

They should extend a test that explicitly sets that if so, or else it only happens to be set because of test ordering. The issue really is that it gets set for all remaining tests not in SQL. That could be OK; set to something fixed is better than defaulting to the local value. But then even that depends on whether SQL tests ran, and when and in what JVM.

[marmbrus]

I think thats where we are today. All query tests use LA and the harness configures that. The problem before this PR was this one suite was setting LA (due to its base class), and then UTC (due to its own before/after all), and that was messing with other tests that assume LA.

[srowen]

Yes, I got that, the problem is (potentially) in the non-SQL tests. If nothing's failing today, probably is entirely fine in practice, but this could have been made more robust while we're here.

[marmbrus]

Oh, sorry, I see what you are saying, although I'm not sure I agree with the conclusion. Given that tests can run in parallel I don't think you actually want to toggle back and forth between timezones. I actually think that was causing the flakeyness we are trying to fix.

In contrast, setting the default timezone to something fixed across all suites in Spark is idempotent. So, if any suite that cares about the default timezone just sets it to LA you should be fine even in the face of concurrency.

[srowen]

Agree. I'd rather fix a timezone for all tests. Some tests don't happen to set one; I know some use GMT elsewhere. Well, it's an issue for another day, to standardize it if it causes enough pain at some point.