Skip to content

Commit

Permalink
[SPARK-10300] [BUILD] [TESTS] Add support for test tags in run-tests.py.
Browse files Browse the repository at this point in the history
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.

Author: Marcelo Vanzin <vanzin@cloudera.com>

Closes #8437 from vanzin/test-tags.
  • Loading branch information
Marcelo Vanzin committed Sep 15, 2015
1 parent c35fdcb commit 8abef21
Show file tree
Hide file tree
Showing 26 changed files with 124 additions and 147 deletions.
10 changes: 0 additions & 10 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -331,16 +331,6 @@
<artifactId>scalacheck_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.curator</groupId>
<artifactId>curator-test</artifactId>
Expand Down
19 changes: 17 additions & 2 deletions dev/run-tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ def determine_modules_to_test(changed_modules):
return modules_to_test.union(set(changed_modules))


def determine_tags_to_exclude(changed_modules):
tags = []
for m in modules.all_modules:
if m not in changed_modules:
tags += m.test_tags
return tags


# -------------------------------------------------------------------------------------------------
# Functions for working with subprocesses and shell tools
# -------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -369,6 +377,7 @@ def detect_binary_inop_with_mima():

def run_scala_tests_maven(test_profiles):
mvn_test_goals = ["test", "--fail-at-end"]

profiles_and_goals = test_profiles + mvn_test_goals

print("[info] Running Spark tests using Maven with these arguments: ",
Expand All @@ -392,7 +401,7 @@ def run_scala_tests_sbt(test_modules, test_profiles):
exec_sbt(profiles_and_goals)


def run_scala_tests(build_tool, hadoop_version, test_modules):
def run_scala_tests(build_tool, hadoop_version, test_modules, excluded_tags):
"""Function to properly execute all tests passed in as a set from the
`determine_test_suites` function"""
set_title_and_block("Running Spark unit tests", "BLOCK_SPARK_UNIT_TESTS")
Expand All @@ -401,6 +410,10 @@ def run_scala_tests(build_tool, hadoop_version, test_modules):

test_profiles = get_hadoop_profiles(hadoop_version) + \
list(set(itertools.chain.from_iterable(m.build_profile_flags for m in test_modules)))

if excluded_tags:
test_profiles += ['-Dtest.exclude.tags=' + ",".join(excluded_tags)]

if build_tool == "maven":
run_scala_tests_maven(test_profiles)
else:
Expand Down Expand Up @@ -500,8 +513,10 @@ def main():
target_branch = os.environ["ghprbTargetBranch"]
changed_files = identify_changed_files_from_git_commits("HEAD", target_branch=target_branch)
changed_modules = determine_modules_for_files(changed_files)
excluded_tags = determine_tags_to_exclude(changed_modules)
if not changed_modules:
changed_modules = [modules.root]
excluded_tags = []
print("[info] Found the following changed modules:",
", ".join(x.name for x in changed_modules))

Expand Down Expand Up @@ -541,7 +556,7 @@ def main():
detect_binary_inop_with_mima()

# run the test suites
run_scala_tests(build_tool, hadoop_version, test_modules)
run_scala_tests(build_tool, hadoop_version, test_modules, excluded_tags)

modules_with_python_tests = [m for m in test_modules if m.python_test_goals]
if modules_with_python_tests:
Expand Down
24 changes: 23 additions & 1 deletion dev/sparktestsupport/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class Module(object):

def __init__(self, name, dependencies, source_file_regexes, build_profile_flags=(), environ={},
sbt_test_goals=(), python_test_goals=(), blacklisted_python_implementations=(),
should_run_r_tests=False):
test_tags=(), should_run_r_tests=False):
"""
Define a new module.
Expand All @@ -50,6 +50,8 @@ def __init__(self, name, dependencies, source_file_regexes, build_profile_flags=
:param blacklisted_python_implementations: A set of Python implementations that are not
supported by this module's Python components. The values in this set should match
strings returned by Python's `platform.python_implementation()`.
:param test_tags A set of tags that will be excluded when running unit tests if the module
is not explicitly changed.
:param should_run_r_tests: If true, changes in this module will trigger all R tests.
"""
self.name = name
Expand All @@ -60,6 +62,7 @@ def __init__(self, name, dependencies, source_file_regexes, build_profile_flags=
self.environ = environ
self.python_test_goals = python_test_goals
self.blacklisted_python_implementations = blacklisted_python_implementations
self.test_tags = test_tags
self.should_run_r_tests = should_run_r_tests

self.dependent_modules = set()
Expand All @@ -85,6 +88,9 @@ def contains_file(self, filename):
"catalyst/test",
"sql/test",
"hive/test",
],
test_tags=[
"org.apache.spark.sql.hive.ExtendedHiveTest"
]
)

Expand Down Expand Up @@ -398,6 +404,22 @@ def contains_file(self, filename):
)


yarn = Module(
name="yarn",
dependencies=[],
source_file_regexes=[
"yarn/",
"network/yarn/",
],
sbt_test_goals=[
"yarn/test",
"network-yarn/test",
],
test_tags=[
"org.apache.spark.deploy.yarn.ExtendedYarnTest"
]
)

# The root module is a dummy module which is used to run all of the tests.
# No other modules should directly depend on this module.
root = Module(
Expand Down
10 changes: 0 additions & 10 deletions external/flume/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,6 @@
<artifactId>scalacheck_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
Expand Down
10 changes: 0 additions & 10 deletions external/kafka/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,6 @@
<artifactId>scalacheck_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
Expand Down
10 changes: 0 additions & 10 deletions external/mqtt/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,6 @@
<artifactId>scalacheck_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.activemq</groupId>
<artifactId>activemq-core</artifactId>
Expand Down
10 changes: 0 additions & 10 deletions external/twitter/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,6 @@
<artifactId>scalacheck_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
Expand Down
10 changes: 0 additions & 10 deletions external/zeromq/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,6 @@
<artifactId>scalacheck_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
Expand Down
10 changes: 0 additions & 10 deletions extras/java8-tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,6 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<profiles>
Expand Down
5 changes: 0 additions & 5 deletions extras/kinesis-asl/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,6 @@
<artifactId>scalacheck_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
Expand Down
5 changes: 0 additions & 5 deletions launcher/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@
<artifactId>log4j</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
10 changes: 0 additions & 10 deletions mllib/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,6 @@
<artifactId>scalacheck_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
10 changes: 0 additions & 10 deletions network/common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,6 @@
</dependency>

<!-- Test dependencies -->
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
Expand Down
10 changes: 0 additions & 10 deletions network/shuffle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,6 @@
<type>test-jar</type>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
Expand Down
17 changes: 15 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@
<libthrift.version>0.9.2</libthrift.version>

<test.java.home>${java.home}</test.java.home>
<test.exclude.tags></test.exclude.tags>

<!--
Dependency scopes that can be overridden by enabling certain profiles. These profiles are
Expand Down Expand Up @@ -339,6 +340,16 @@
<artifactId>scalatest_${scala.binary.version}</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<dependencyManagement>
<dependencies>
Expand Down Expand Up @@ -742,7 +753,7 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.10</version>
<version>4.11</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -760,7 +771,7 @@
<dependency>
<groupId>com.novocode</groupId>
<artifactId>junit-interface</artifactId>
<version>0.10</version>
<version>0.11</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down Expand Up @@ -1915,6 +1926,7 @@
<test.src.tables>src</test.src.tables>
</systemProperties>
<failIfNoTests>false</failIfNoTests>
<excludedGroups>${test.exclude.tags}</excludedGroups>
</configuration>
</plugin>
<!-- Scalatest runs all Scala tests -->
Expand Down Expand Up @@ -1952,6 +1964,7 @@
<!-- Needed by sql/hive tests. -->
<test.src.tables>__not_used__</test.src.tables>
</systemProperties>
<tagsToExclude>${test.exclude.tags}</tagsToExclude>
</configuration>
<executions>
<execution>
Expand Down
13 changes: 11 additions & 2 deletions project/SparkBuild.scala
Original file line number Diff line number Diff line change
Expand Up @@ -567,11 +567,20 @@ object TestSettings {
javaOptions in Test ++= "-Xmx3g -Xss4096k -XX:PermSize=128M -XX:MaxNewSize=256m -XX:MaxPermSize=1g"
.split(" ").toSeq,
javaOptions += "-Xmx3g",
// Exclude tags defined in a system property
testOptions in Test += Tests.Argument(TestFrameworks.ScalaTest,
sys.props.get("test.exclude.tags").map { tags =>
tags.split(",").flatMap { tag => Seq("-l", tag) }.toSeq
}.getOrElse(Nil): _*),
testOptions in Test += Tests.Argument(TestFrameworks.JUnit,
sys.props.get("test.exclude.tags").map { tags =>
Seq("--exclude-categories=" + tags)
}.getOrElse(Nil): _*),
// Show full stack trace and duration in test cases.
testOptions in Test += Tests.Argument("-oDF"),
testOptions += Tests.Argument(TestFrameworks.JUnit, "-v", "-a"),
testOptions in Test += Tests.Argument(TestFrameworks.JUnit, "-v", "-a"),
// Enable Junit testing.
libraryDependencies += "com.novocode" % "junit-interface" % "0.9" % "test",
libraryDependencies += "com.novocode" % "junit-interface" % "0.11" % "test",
// Only allow one test at a time, even across projects, since they run in the same JVM
parallelExecution in Test := false,
// Make sure the test temp directory exists.
Expand Down
Loading

0 comments on commit 8abef21

Please sign in to comment.