Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Sep 20, 2019

What changes were proposed in this pull request?

This PR aims to add tag ExtendedSQLTest for SQLQueryTestSuite.
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?

SQLQueryTestSuite takes 45 mins alone because it has many SQL scripts to run.

time

Does this PR introduce any user-facing change?

No.

How was this patch tested?

build/sbt "sql/test-only *.SQLQueryTestSuite" -Dtest.exclude.tags=org.apache.spark.tags.ExtendedSQLTest
...
[info] SQLQueryTestSuite:
[info] ScalaTest
[info] Run completed in 3 seconds, 147 milliseconds.
[info] Total number of tests run: 0
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 0, failed 0, canceled 0, ignored 0, pending 0
[info] No tests were executed.
[info] Passed: Total 0, Failed 0, Errors 0, Passed 0
[success] Total time: 22 s, completed Sep 20, 2019 12:23:13 PM

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Sep 20, 2019

@srowen
Copy link
Member

srowen commented Sep 20, 2019

Is it not possible to exclude a single suite? OK if this is necessary to achieve that, it's just a tag.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Sep 20, 2019

Right, it's impossible in Maven. So, we have been using this way.

@SparkQA
Copy link

SparkQA commented Sep 20, 2019

Test build #111087 has finished for PR 25872 at commit 3d85e88.

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

@maropu
Copy link
Member

maropu commented Sep 21, 2019

What does Extended mean? This tag is only used for SQLQueryTestSuite?
Also, we need [SQL] in the title?

import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@TagAnnotation
Copy link
Member

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?

Copy link
Member Author

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.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29191][TESTS] Add tag ExtendedSQLTest for SQLQueryTestSuite [SPARK-29191][TESTS][SQL] Add tag ExtendedSQLTest for SQLQueryTestSuite Sep 21, 2019
@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Sep 21, 2019

@maropu . Tag can be reused later. For example, ThriftServerQueryTestSuite extends SQLQueryTestSuite. Historically, see ExtendedHiveTest tag case. We added it once and reuse it later other places.

@maropu
Copy link
Member

maropu commented Sep 21, 2019

Ah, I see.

@dongjoon-hyun
Copy link
Member Author

Hi, @srowen and @maropu .
Could you review this PR once more? Thanks!

@dongjoon-hyun
Copy link
Member Author

Thank you for approval, @srowen .
Merged to master.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-29191 branch September 22, 2019 20:53
@maropu
Copy link
Member

maropu commented Sep 22, 2019

Late LGTM

@dongjoon-hyun
Copy link
Member Author

Thank you, @maropu !

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.

LGTM from me too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants