[SPARK-31820][SQL][TESTS] Fix flaky JavaBeanDeserializationSuite #28639
Conversation
|
Test build #123109 has finished for PR 28639 at commit
|
|
jenkins, retest this, please |
|
@HeartSaVioR @srowen @maropu @cloud-fan @HyukjinKwon Please, review this fix. I think it is related to #23854 and #24112 |
| @@ -210,6 +212,17 @@ private static Row createRecordSpark22000Row(Long index) { | |||
| return new GenericRow(values); | |||
| } | |||
|
|
|||
| private static String timestampToString(Timestamp ts) { | |||
There was a problem hiding this comment.
shall we reuse FractionTimestampFormatter?
There was a problem hiding this comment.
We could but if we introduce a bug to the formatter, we could catch it by this test.
There was a problem hiding this comment.
One more thing, if we use the same formatter for expected and actual results, checking current timestamps doesn't make so much sense. In that case, we could test a concrete timestamp.
| @@ -210,6 +212,17 @@ private static Row createRecordSpark22000Row(Long index) { | |||
| return new GenericRow(values); | |||
| } | |||
|
|
|||
| private static String timestampToString(Timestamp ts) { | |||
There was a problem hiding this comment.
|
retest this please |
|
Test build #123112 has finished for PR 28639 at commit
|
|
thanks, merging to master/3.0! |
### What changes were proposed in this pull request? Modified formatting of expected timestamp strings in the test `JavaBeanDeserializationSuite`.`testSpark22000` to correctly format timestamps with **zero** seconds fraction. Current implementation outputs `.0` but must be empty string. From SPARK-31820 failure: - should be `2020-05-25 12:39:17` - but incorrect expected string is `2020-05-25 12:39:17.0` ### Why are the changes needed? To make `JavaBeanDeserializationSuite` stable, and avoid test failures like #28630 (comment) ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? I changed https://github.com/apache/spark/blob/7dff3b125de23a4d6ce834217ee08973b259414c/sql/core/src/test/java/test/org/apache/spark/sql/JavaBeanDeserializationSuite.java#L207 to ```java new java.sql.Timestamp((System.currentTimeMillis() / 1000) * 1000), ``` to force zero seconds fraction. Closes #28639 from MaxGekk/fix-JavaBeanDeserializationSuite. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 87d34e6) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
|
Test build #123114 has finished for PR 28639 at commit
|
What changes were proposed in this pull request?
Modified formatting of expected timestamp strings in the test
JavaBeanDeserializationSuite.testSpark22000to correctly format timestamps with zero seconds fraction. Current implementation outputs.0but must be empty string. From SPARK-31820 failure:2020-05-25 12:39:172020-05-25 12:39:17.0Why are the changes needed?
To make
JavaBeanDeserializationSuitestable, and avoid test failures like #28630 (comment)Does this PR introduce any user-facing change?
No
How was this patch tested?
I changed
spark/sql/core/src/test/java/test/org/apache/spark/sql/JavaBeanDeserializationSuite.java
Line 207 in 7dff3b1
to force zero seconds fraction.