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-10300] [build] [tests] Add support for test tags in run-tests.py. #8437

Closed
wants to merge 13 commits into from
Closed

[SPARK-10300] [build] [tests] Add support for test tags in run-tests.py. #8437

wants to merge 13 commits into from

Conversation

vanzin
Copy link
Contributor

@vanzin vanzin commented Aug 25, 2015

This change does two things:

  • tag a few tests and adds the mechanism in the build to be able to disable those tags,
    both in maven and sbt, for both junit and scalatest suites.
  • add some logic to run-tests.py to disable some tags depending on what files have
    changed; that's used to disable expensive tests when a module hasn't explicitly
    been changed, to speed up testing for changes that don't directly affect those
    modules.

@SparkQA
Copy link

SparkQA commented Aug 25, 2015

Test build #41566 has finished for PR 8437 at commit 4e26e43.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class FPGrowthModel[Item: ClassTag] @Since("1.3.0") (
    • class FreqItemset[Item] @Since("1.3.0") (
    • class FreqSequence[Item] @Since("1.5.0") (
    • class PrefixSpanModel[Item] @Since("1.5.0") (

@vanzin
Copy link
Contributor Author

vanzin commented Aug 25, 2015

retest this please

@SparkQA
Copy link

SparkQA commented Aug 25, 2015

Test build #41567 has finished for PR 8437 at commit 833502f.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor Author

vanzin commented Aug 25, 2015

retest this please

@SparkQA
Copy link

SparkQA commented Aug 26, 2015

Test build #41568 has finished for PR 8437 at commit ef737c0.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 26, 2015

Test build #41572 has finished for PR 8437 at commit 3a2979f.

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

Marcelo Vanzin added 5 commits August 25, 2015 20:01
Add a tag for yarn integration tests.
a02c07a
Add tag exclusion to junit also.
05304a6 
Based on squito:SPARK-4746
Remove scalatest object tags since they're not used.
ce92266 
From squito:SPARK-4746 it seems like they might not even be needed.
Declare common test dependencies in root pom only.
27749cf
Tag a random sample of tests in HiveCompatibilitySuite with ExtendedH…
b7d0507 
…iveTest.

This allows a small set of tests to run for every PR, while runnign the
whole test suite when changes to sql code are made (as decided by
run-tests.py).
@vanzin
Copy link
Contributor Author

vanzin commented Aug 26, 2015

@pwendell @marmbrus what do you think of this approach? It's partly based on code that @squito wrote before to add tags for long-running tests. Main change is that I hooked things up to run-tests.py to automate enabling / disabling some tags.

Note that the tests for this PR will run everything (since I'm touching files in the respective modules), but I tested locally that things are disabled properly when they should be.

@SparkQA
Copy link

SparkQA commented Aug 26, 2015

Test build #41639 has finished for PR 8437 at commit b7d0507.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

Marcelo Vanzin added 2 commits August 26, 2015 10:52
Workaround for an ivy issue?
5c107a5
Use same junit version for maven and sbt.
85fed20 
This makes sbt happy now that I put the junit assembly
in the shared dependencies in the root pom.
@SparkQA
Copy link

SparkQA commented Aug 26, 2015

Test build #41641 has finished for PR 8437 at commit 5c107a5.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@marmbrus
Copy link
Contributor

The ExtendedHiveTest tag looks good to me. I would consider dropping the random smoke test part of things since we have lot of SQL tests and even hive tests that will still be running, so I'm not sure its worth the complexity. I would be okay just marking the whole test as ExtendedHiveTest.

Thanks for working on this!

@vanzin
Copy link
Contributor Author

vanzin commented Aug 26, 2015

I would be okay just marking the whole test as ExtendedHiveTest.

Easy enough (good thing I made that change in a single commit).

@vanzin vanzin changed the title WIP DO NOT MERGE. Testing out test tags. [SPARK-10300] [build] [tests] Add support for test tags in run-test.py. Aug 26, 2015
@marmbrus
Copy link
Contributor

SQL changes LGTM. I'll let @pwendell comment on build changes.

@vanzin vanzin changed the title [SPARK-10300] [build] [tests] Add support for test tags in run-test.py. [SPARK-10300] [build] [tests] Add support for test tags in run-tests.py. Aug 26, 2015
Merge branch 'master' into test-tags
131e658 
Conflicts:
	dev/run-tests-jenkins
@SparkQA
Copy link

SparkQA commented Aug 26, 2015

Test build #41642 has finished for PR 8437 at commit 85fed20.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor Author

vanzin commented Aug 26, 2015

pyspark failures, seem unrelated. in any case... retest this please

@SparkQA
Copy link

SparkQA commented Aug 26, 2015

Test build #41645 has finished for PR 8437 at commit 83bec7f.

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

@SparkQA
Copy link

SparkQA commented Aug 26, 2015

Test build #41648 has finished for PR 8437 at commit 131e658.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@vanzin
Copy link
Contributor Author

vanzin commented Aug 26, 2015

Another pyspark failure (that I've seen countless times in other PRs).

@vanzin
Copy link
Contributor Author

vanzin commented Aug 27, 2015

retest this please

@SparkQA
Copy link

SparkQA commented Aug 27, 2015

Test build #41649 has finished for PR 8437 at commit 131e658.

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

@SparkQA
Copy link

SparkQA commented Aug 27, 2015

Test build #41657 has finished for PR 8437 at commit 131e658.

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

@SparkQA
Copy link

SparkQA commented Aug 27, 2015

Test build #41658 has finished for PR 8437 at commit 090e1d4.

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

@vanzin vanzin mentioned this pull request Aug 31, 2015
Closed
@andrewor14
Copy link
Contributor

@JoshRosen should look at this

@vanzin
Copy link
Contributor Author

vanzin commented Sep 9, 2015

@JoshRosen any comments?

@vanzin
Copy link
Contributor Author

vanzin commented Sep 14, 2015

No comments? I already got a +1 from Michael, so as far as I can tell, this is good to go.

@pwendell
Copy link
Contributor

LGTM - I've modified that script recently enough to be familiar with how it works. This seems like a good approach and could be useful for us in the future.

@vanzin
Copy link
Contributor Author

vanzin commented Sep 15, 2015

ok, merging.

@asfgit asfgit closed this in 8abef21 Sep 15, 2015
@vanzin vanzin deleted the test-tags branch September 15, 2015 17:50
@vanzin
Copy link
Contributor Author

vanzin commented Sep 15, 2015

This seems to have caused PR tests to start failing... I'll try things locally again and revert the patch if needed.

@mengxr
Copy link
Contributor

mengxr commented Sep 15, 2015

Shall we revert the PR first? It is blocking other PRs.

@vanzin
Copy link
Contributor Author

vanzin commented Sep 15, 2015

Done.

@vanzin
Copy link
Contributor Author

vanzin commented Sep 15, 2015

For the curious, the issue is that the junit test runners (both surefire and the sbt one) need the tag classes around or they'll throw an error, and that happens when building modules where the tag is not available. I'm thinking about a workaround that does not involve removing the junit functionality.

@JoshRosen
Copy link
Contributor

Are there plans to revive this PR and attempt another merge?

@vanzin
Copy link
Contributor Author

vanzin commented Oct 1, 2015

@JoshRosen #8775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
7 participants
@vanzin @SparkQA @marmbrus @andrewor14 @pwendell @mengxr @JoshRosen