-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-29191][TESTS][SQL] Add tag ExtendedSQLTest for SQLQueryTestSuite #25872
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.
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
Conversation
|
cc @gengliangwang , @gatorsmile , @wangyum , @HyukjinKwon , @srowen |
|
Is it not possible to exclude a single suite? OK if this is necessary to achieve that, it's just a tag. |
|
Right, it's impossible in Maven. So, we have been using this way. |
|
Test build #111087 has finished for PR 25872 at commit
|
|
What does |
| import java.lang.annotation.RetentionPolicy; | ||
| import java.lang.annotation.Target; | ||
|
|
||
| @TagAnnotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about describing here what's this tag is used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't do that for tags ExtendedHiveTest, ExtendedYarnTest, DockerTest.
|
@maropu . Tag can be reused later. For example, |
|
Ah, I see. |
|
Thank you for approval, @srowen . |
|
Late LGTM |
|
Thank you, @maropu ! |
HyukjinKwon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM from me too
What changes were proposed in this pull request?
This PR aims to add tag
ExtendedSQLTestforSQLQueryTestSuite.This doesn't affect our Jenkins test coverage.
Instead, this tag gives us an ability to parallelize them by splitting this test suite and the other suites.
Why are the changes needed?
SQLQueryTestSuitetakes 45 mins alone because it has many SQL scripts to run.Does this PR introduce any user-facing change?
No.
How was this patch tested?