Skip to content

[SPARK-31557][SQL][TESTS][FOLLOWUP] Check rebasing in all legacy formatters#28398

Closed
MaxGekk wants to merge 2 commits intoapache:masterfrom
MaxGekk:test-rebasing-in-legacy-date-formatter
Closed

[SPARK-31557][SQL][TESTS][FOLLOWUP] Check rebasing in all legacy formatters#28398
MaxGekk wants to merge 2 commits intoapache:masterfrom
MaxGekk:test-rebasing-in-legacy-date-formatter

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Apr 28, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Round trip tests can hide issues in dates rebasing. For example, if we remove rebasing from legacy parsers (from parse() and format()) the tests will pass.

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running DateFormatterSuite.

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 28, 2020

@bersprockets @cloud-fan While working on the fix for legacy timestamp formatters, I have found that the round trip tests for dates are not enough. Need to test in each direction separately. Also existing tests don't cover all legacy parsers.

@SparkQA
Copy link

SparkQA commented Apr 29, 2020

Test build #122010 has finished for PR 28398 at commit d653213.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master/3.0!

@cloud-fan cloud-fan closed this in 73eac75 Apr 29, 2020
cloud-fan pushed a commit that referenced this pull request Apr 29, 2020
…atters

### What changes were proposed in this pull request?
- Check all available legacy formats in the tests added by #28345
- Check dates rebasing in legacy parsers for only one direction either days -> string or string -> days.

### Why are the changes needed?
Round trip tests can hide issues in dates rebasing. For example, if we remove rebasing from legacy parsers (from `parse()` and `format()`) the tests will pass.

### Does this PR introduce any user-facing change?
No

### How was this patch tested?
By running `DateFormatterSuite`.

Closes #28398 from MaxGekk/test-rebasing-in-legacy-date-formatter.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
(cherry picked from commit 73eac75)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@MaxGekk MaxGekk deleted the test-rebasing-in-legacy-date-formatter branch June 5, 2020 19:48
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