This is an automated email from the ASF dual-hosted git repository. adar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 10ea0ce5a636a050a1207f7ab5ecf63d178683f5 Author: Adar Dembo <[email protected]> AuthorDate: Sun Apr 21 21:17:04 2019 -0700 dist-test: support RUN_FLAKY_ONLY The flaky test list is somewhat short so there's no real reason for the tests to be distributed. However, over the years we've observed that the dist-test runtime environment is different enough from non-dist-test environments that a test may be flaky in one but not the other. Since the purpose of flaky test resistance is to improve the precommit experience, and since all precommit tests run in dist-test, the test runs that power the flaky test dashboard (i.e. RUN_FLAKY_ONLY=1) should follow suit and also run in dist-test. Change-Id: Id5a063512f82341a48031911de12c44f902a2723 Reviewed-on: http://gerrit.cloudera.org:8080/13072 Tested-by: Adar Dembo <[email protected]> Reviewed-by: Grant Henke <[email protected]> --- build-support/dist_test.py | 34 +++++++++++++++++----- build-support/jenkins/build-and-test.sh | 7 ----- .../org/apache/kudu/gradle/DistTestTask.java | 7 ++--- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/build-support/dist_test.py b/build-support/dist_test.py index b26ebc9..2cadad5 100755 --- a/build-support/dist_test.py +++ b/build-support/dist_test.py @@ -59,6 +59,9 @@ MAX_TASKS_PER_JOB=10000 # of retries, so we have to subtract 1. FLAKY_TEST_RETRIES = int(os.environ.get('KUDU_FLAKY_TEST_ATTEMPTS', 1)) - 1 +# Whether to only run flaky tests or to run all tests. +RUN_FLAKY_ONLY = int(os.environ.get('RUN_FLAKY_ONLY', 0)) + # Whether to retry all failed C++ tests, rather than just known flaky tests. # Since Java flaky tests are not reported by the test server, Java tests are # always retried, regardless of this value. @@ -478,9 +481,16 @@ def run_tests(parser, options): Gets all of the test command lines from 'ctest', isolates them, creates a task list, and submits the tasks to the testing service. """ - executions = get_test_executions(options.tests_regex) + flakies = get_flakies() + if RUN_FLAKY_ONLY: + if options.tests_regex: + raise Exception("Cannot use RUN_FLAKY_ONLY with --tests-regex") + tests_regex = "|".join(["^" + re.escape(f) for f in flakies]) + else: + tests_regex = options.tests_regex + executions = get_test_executions(tests_regex) if not executions: - raise Exception("No matching tests found for pattern %s" % options.tests_regex) + raise Exception("No matching tests found for pattern %s" % tests_regex) if options.extra_args: if options.extra_args[0] == '--': del options.extra_args[0] @@ -494,7 +504,7 @@ def run_tests(parser, options): run_isolate(staging) retry_all = RETRY_ALL_TESTS > 0 create_task_json(staging, - flaky_test_set=get_flakies(), + flaky_test_set=flakies, replicate_tasks=options.num_instances, retry_all_tests=retry_all) submit_tasks(staging, options) @@ -591,13 +601,21 @@ def get_gradle_cmd_line(options): return cmd def run_java_tests(parser, options): - subprocess.check_call(get_gradle_cmd_line(options), - cwd=rel_to_abs("java")) + flakies = get_flakies() + cmd = get_gradle_cmd_line(options) + if RUN_FLAKY_ONLY: + for f in flakies: + # As per the Gradle docs[1], test classes are included by filename, so we + # need to convert the class names into file paths. + # + # 1. https://docs.gradle.org/current/javadoc/org/gradle/api/tasks/testing/Test.html + cmd.extend([ "--classes", "%s.class" % f.replace(".", "/") ]) + subprocess.check_call(cmd, cwd=rel_to_abs("java")) staging = StagingDir(rel_to_abs("java/build/dist-test")) run_isolate(staging) retry_all = RETRY_ALL_TESTS > 0 create_task_json(staging, - flaky_test_set=get_flakies(), + flaky_test_set=flakies, replicate_tasks=1, retry_all_tests=retry_all) submit_tasks(staging, options) @@ -609,7 +627,9 @@ def loop_java_test(parser, options): if options.num_instances < 1: parser.error("--num-instances must be >= 1") cmd = get_gradle_cmd_line(options) - cmd.extend([ "--classes", "**/%s" % options.pattern ]) + # Test classes are matched by filename, so unless we convert dots into forward + # slashes, package name prefixes will never match anything. + cmd.extend([ "--classes", "**/%s.class" % options.pattern.replace(".", "/") ]) subprocess.check_call(cmd, cwd=rel_to_abs("java")) staging = StagingDir(rel_to_abs("java/build/dist-test")) run_isolate(staging) diff --git a/build-support/jenkins/build-and-test.sh b/build-support/jenkins/build-and-test.sh index 4eaf556..a4e5e31 100755 --- a/build-support/jenkins/build-and-test.sh +++ b/build-support/jenkins/build-and-test.sh @@ -223,13 +223,6 @@ if [ "$RUN_FLAKY_ONLY" == "1" -o "$KUDU_FLAKY_TEST_ATTEMPTS" -gt 1 ]; then fi if [ "$RUN_FLAKY_ONLY" == "1" ]; then - # TODO(adar): we can't yet pass the flaky test list into the dist-test - # machinery (in order to control which tests are executed). - if [ "$ENABLE_DIST_TEST" == "1" ]; then - echo "Distributed testing is incompatible with RUN_FLAKY_ONLY=1" - exit 1 - fi - test_regex=$(perl -e ' chomp(my @lines = <>); print join("|", map { "^" . quotemeta($_) } @lines); diff --git a/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java b/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java index 9340b25..f05aacc 100644 --- a/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java +++ b/java/buildSrc/src/main/groovy/org/apache/kudu/gradle/DistTestTask.java @@ -74,8 +74,7 @@ public class DistTestTask extends DefaultTask { private boolean collectTmpDir = false; /** - * Called by the build file to add test tasks to be considered for - * dist-tests. + * Called by build.gradle to add test tasks to be considered for dist-tests. */ public void addTestTask(Test t) { testTasks.add(t); @@ -85,7 +84,7 @@ public class DistTestTask extends DefaultTask { description = "Sets test class to be included, '*' is supported.") public DistTestTask setClassPattern(List<String> classPattern) { for (Test t : testTasks) { - // TODO: this is currently requiring a glob like **/*Foo* instead of just *Foo* + // TODO: this requires a glob like **/*Foo* instead of just *Foo* t.setIncludes(classPattern); } return this; @@ -110,7 +109,7 @@ public class DistTestTask extends DefaultTask { @InputFiles public FileCollection getInputClasses() { - FileCollection fc = getProject().files(); // Create and empty FileCollection. + FileCollection fc = getProject().files(); // Create an empty FileCollection. for (Test t : testTasks) { fc = fc.plus(t.getCandidateClassFiles()); }
