Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-31954][SQL] Delete duplicate testcase in HiveQuerySuite #28782

Closed
wants to merge 17 commits into from
Closed

[SPARK-31954][SQL] Delete duplicate testcase in HiveQuerySuite #28782

wants to merge 17 commits into from

Conversation

GuoPhilipse
Copy link
Member

What changes were proposed in this pull request?

remove duplicate test cases

Why are the changes needed?

improve test quality

Does this PR introduce any user-facing change?

NO

How was this patch tested?

No test

GuoPhilipse and others added 15 commits May 21, 2020 07:36
@GuoPhilipse
Merge pull request #1 from apache/master
bb1efa2 
update masterbranch
@GuoPhilipse
Merge pull request #2 from apache/master
1459d5b 
sync
@GuoPhilipse
Merge pull request #3 from apache/master
88c40fe 
sync
@GuoPhilipse
Merge pull request #4 from apache/master
df22083 
sync
@GuoPhilipse
Merge pull request #5 from apache/master
0436611 
sync
@GuoPhilipse
Merge pull request #6 from apache/master
ed80c84 
sync
@GuoPhilipse
Merge pull request #7 from apache/master
39ca87c 
sync
@GuoPhilipse
Merge pull request #8 from apache/master
c3b3c89 
sync
@GuoPhilipse
Merge pull request #9 from apache/master
c3546eb 
sync
@GuoPhilipse
Merge pull request #10 from apache/master
77a339a 
sync
@GuoPhilipse
Merge pull request #11 from apache/master
a6b4f74 
sync
@GuoPhilipse
Merge pull request #12 from apache/master
664277e 
sync
@GuoPhilipse
Merge pull request #13 from apache/master
fd677c9 
sync
@GuoPhilipse
Merge pull request #14 from apache/master
93b1f63 
sync
@GuoPhilipse
remove duplicate test cases
c4f9a62
@maropu
Copy link
Member

maropu commented Jun 10, 2020

ok to test

maropu
maropu reviewed Jun 10, 2020
View reviewed changes
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala
@@ -564,9 +564,6 @@ class HiveQuerySuite extends HiveComparisonTest with SQLTestUtils with BeforeAnd
assert(1200 == res.getInt(0))
}

createQueryTest("timestamp cast #4",
"SELECT CAST(CAST(1.2 AS TIMESTAMP) AS DOUBLE) FROM src LIMIT 1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, as you suggested, the test looks duplicate. cc: @HyukjinKwon @viirya

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks duplicate.

HyukjinKwon
HyukjinKwon approved these changes Jun 10, 2020
View reviewed changes
Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good if tests pass

@SparkQA
Copy link

SparkQA commented Jun 10, 2020

Test build #123760 has finished for PR 28782 at commit c4f9a62.

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

viirya
viirya reviewed Jun 10, 2020
View reviewed changes
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveQuerySuite.scala
Comment on lines -583 to -585
createQueryTest("timestamp cast #8",
"SELECT CAST(CAST(-1.2 AS TIMESTAMP) AS DOUBLE) FROM src LIMIT 1")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of removing them, maybe we can change them? As you see that these tests are with numbers (#4 and #8), removing them makes the numbers not continuous now. Not strongly option, anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we can rename them to make it prettier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@HyukjinKwon HyukjinKwon changed the title [SPARK-31954][SQL] delete duplicate testcase [SPARK-31954][SQL] Delete duplicate testcase in HiveQuerySuite Jun 11, 2020
@GuoPhilipse
Copy link
Member Author

have adjusted test case order and rename test result file to look more pretty

@SparkQA
Copy link

SparkQA commented Jun 11, 2020

Test build #123819 has finished for PR 28782 at commit c7b81d2.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@GuoPhilipse
Copy link
Member Author

retest this please

1 similar comment
@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Jun 11, 2020

Test build #123838 has finished for PR 28782 at commit c7b81d2.

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

@HyukjinKwon
Copy link
Member

Merged to master, branch-3.0 and branch-2.4.

HyukjinKwon pushed a commit that referenced this pull request Jun 11, 2020
@GuoPhilipse @HyukjinKwon
[SPARK-31954][SQL] Delete duplicate testcase in HiveQuerySuite
250c6fd 
### What changes were proposed in this pull request?
remove duplicate test cases

### Why are the changes needed?
improve test quality

### Does this PR introduce _any_ user-facing change?
NO

### How was this patch tested?
No  test

Closes #28782 from GuoPhilipse/31954-delete-duplicate-testcase.

Lead-authored-by: GuoPhilipse <46367746+GuoPhilipse@users.noreply.github.com>
Co-authored-by: GuoPhilipse <guofei_ok@126.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
(cherry picked from commit 912d45d)
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
HyukjinKwon pushed a commit that referenced this pull request Jun 11, 2020
@GuoPhilipse @HyukjinKwon
[SPARK-31954][SQL] Delete duplicate testcase in HiveQuerySuite
e51eb3a 
### What changes were proposed in this pull request?
remove duplicate test cases

### Why are the changes needed?
improve test quality

### Does this PR introduce _any_ user-facing change?
NO

### How was this patch tested?
No  test

Closes #28782 from GuoPhilipse/31954-delete-duplicate-testcase.

Lead-authored-by: GuoPhilipse <46367746+GuoPhilipse@users.noreply.github.com>
Co-authored-by: GuoPhilipse <guofei_ok@126.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
(cherry picked from commit 912d45d)
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
holdenk pushed a commit to holdenk/spark that referenced this pull request Jun 25, 2020
@GuoPhilipse @dongjoon-hyun
[SPARK-31954][SQL] Delete duplicate testcase in HiveQuerySuite
a988f8a 
### What changes were proposed in this pull request?
remove duplicate test cases

### Why are the changes needed?
improve test quality

### Does this PR introduce _any_ user-facing change?
NO

### How was this patch tested?
No  test

Closes apache#28782 from GuoPhilipse/31954-delete-duplicate-testcase.

Lead-authored-by: GuoPhilipse <46367746+GuoPhilipse@users.noreply.github.com>
Co-authored-by: GuoPhilipse <guofei_ok@126.com>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
(cherry picked from commit 912d45d)
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viirya viirya viirya left review comments

@maropu maropu maropu left review comments

@HyukjinKwon HyukjinKwon HyukjinKwon approved these changes

Assignees
No one assigned
Labels
SQL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@GuoPhilipse @maropu @SparkQA @HyukjinKwon @viirya