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 3707cf33ccbcb015130de356e4e82d1f54636c6e Author: Adar Dembo <[email protected]> AuthorDate: Thu Apr 18 15:40:25 2019 -0700 dist-test: support test result reporting The goal is to be able to use dist-test in any run of build-and-test.sh, even those that power the flaky test dashboard. Because dist-test runs just one test per invocation, it isn't safe to disown the reporting process; the slave might disappear mid-report. There wasn't an obvious way to know that we're using dist-test from within run-test.sh, so I opted to stop disowning altogether. The Java result reporting is synchronous (and per-test rather than per-class) so this didn't seem like a big deal. I was concerned that the additional parallelism might lead to "thundering herd" behavior in the test result server (i.e. if all tests finished and reported at the same time) so I ran a couple reporting-enabled dist-tests looking for load spikes on the server. I didn't see any, likely because: - Most tests' running time varies. - Tests only start when dist-test slaves become available, not all at once. RUN_FLAKY_ONLY will be addressed in a follow-up. Change-Id: Ie5072e2395376e455f63dac6d3debc88526aed07 Reviewed-on: http://gerrit.cloudera.org:8080/13068 Tested-by: Adar Dembo <[email protected]> Reviewed-by: Grant Henke <[email protected]> --- build-support/dist_test.py | 24 +++++++++++++++ build-support/run-test.sh | 12 +------- .../org/apache/kudu/gradle/DistTestTask.java | 36 ++++++++++++++++++++-- 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/build-support/dist_test.py b/build-support/dist_test.py index 402b6c6..b26ebc9 100755 --- a/build-support/dist_test.py +++ b/build-support/dist_test.py @@ -83,6 +83,7 @@ TEST_SHARD_RE = re.compile("\.\d+$") DEPS_FOR_ALL = \ ["build-support/stacktrace_addr2line.pl", + "build-support/report-test.sh", "build-support/run-test.sh", "build-support/run_dist_test.py", "build-support/java-home-candidates.txt", @@ -273,6 +274,20 @@ def copy_system_library(lib): shutil.copy2(rel_to_abs(lib), dst) return dst +def forward_env_var(command_list, var_name, is_required=True): + """ + Extends 'command_list' with the name and value of the environment variable + given by 'var_name'. + + Does nothing if the environment variable isn't set or is empty, unless + 'is_required' is True, in which case an exception is raised. + """ + if not var_name in os.environ or not os.environ.get(var_name): + if is_required: + raise Exception("required env variable %s is missing" % (var_name,)) + return + command_list.extend(["-e", "%s=%s" % (var_name, os.environ.get(var_name))]) + def create_archive_input(staging, execution, dep_extractor, collect_tmpdir=False): """ @@ -324,6 +339,15 @@ def create_archive_input(staging, execution, dep_extractor, continue command.extend(['-e', '%s=%s' % (k, v)]) + # If test result reporting was requested, forward all relevant environment + # variables into the test process so as to enable reporting. + if os.environ.get('KUDU_REPORT_TEST_RESULTS', 0): + forward_env_var(command, 'KUDU_REPORT_TEST_RESULTS') + forward_env_var(command, 'BUILD_CONFIG') + forward_env_var(command, 'BUILD_TAG') + forward_env_var(command, 'GIT_REVISION') + forward_env_var(command, 'TEST_RESULT_SERVER', is_required=False) + if collect_tmpdir: command += ["--collect-tmpdir"] command.append('--') diff --git a/build-support/run-test.sh b/build-support/run-test.sh index c5b9ab8..21d09d0 100755 --- a/build-support/run-test.sh +++ b/build-support/run-test.sh @@ -215,17 +215,7 @@ for ATTEMPT_NUMBER in $(seq 1 $TEST_EXECUTION_ATTEMPTS) ; do if [ -n "$KUDU_REPORT_TEST_RESULTS" ] && [ "$KUDU_REPORT_TEST_RESULTS" -ne 0 ]; then echo Reporting results - $SOURCE_ROOT/build-support/report-test.sh "$ABS_TEST_PATH" "$LOGFILE" "$STATUS" & - - # On success, we'll do "best effort" reporting, and disown the subprocess. - # On failure, we want to upload the failed test log. So, in that case, - # wait for the report-test.sh job to finish, lest we accidentally run - # a test retry and upload the wrong log. - if [ "$STATUS" -eq "0" ]; then - disown - else - wait - fi + $SOURCE_ROOT/build-support/report-test.sh "$ABS_TEST_PATH" "$LOGFILE" "$STATUS" fi if [ "$STATUS" -eq "0" ]; then 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 4d3f5db..9340b25 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 @@ -149,7 +149,7 @@ public class DistTestTask extends DefaultTask { * * Note: This currently fails OSX because dump_base_deps use ldd. */ - List<String> getBaseDeps() throws IOException { + private List<String> getBaseDeps() throws IOException { Process proc = new ProcessBuilder(distTestBin, "internal", "dump_base_deps") @@ -162,6 +162,35 @@ public class DistTestTask extends DefaultTask { } } + /** + * @return all test result reporting environment variables and their values, + * in a format suitable for consumption by run_dist_test.py. + */ + private List<String> getTestResultReportingEnvironmentVariables() { + ImmutableList.Builder<String> args = new ImmutableList.Builder<>(); + String enabled = System.getenv("KUDU_REPORT_TEST_RESULTS"); + if (enabled != null && Integer.parseInt(enabled) > 0) { + for (String ev : ImmutableList.of("KUDU_REPORT_TEST_RESULTS", + "BUILD_CONFIG", + "BUILD_TAG", + "GIT_REVISION", + "TEST_RESULT_SERVER")) { + String evValue = System.getenv(ev); + if (evValue == null || evValue.isEmpty()) { + if (ev.equals("TEST_RESULT_SERVER")) { + // This one is optional. + continue; + } + throw new RuntimeException( + String.format("Required env variable %s is missing", ev)); + } + args.add("-e"); + args.add(String.format("%s=%s", ev, evValue)); + } + } + return args.build(); + } + private String genIsolate(Path isolateFileDir, Test test, String testClass, List<String> baseDeps) throws IOException { Path rootDir = test.getProject().getRootDir().toPath(); @@ -200,8 +229,9 @@ public class DistTestTask extends DefaultTask { if (collectTmpDir) { cmd.add("--collect-tmpdir"); } - cmd.add("--test-language=java", - "--", + cmd.add("--test-language=java"); + cmd.addAll(getTestResultReportingEnvironmentVariables()); + cmd.add("--", "-ea", "-cp", Joiner.on(":").join(classpath));
