From 3d85b88609a362857d8ee3c0432a37d30268a8a2 Mon Sep 17 00:00:00 2001 From: Chenchu Kolli Date: Mon, 9 May 2022 16:57:04 -0500 Subject: [PATCH] Add a flag to expose undeclared test outputs in unzipped form. (#15431) Closes #14568. Closes #15199. PiperOrigin-RevId: 440932893 Co-authored-by: Tiago Quelhas --- .../lib/analysis/test/TestConfiguration.java | 14 ++- .../lib/analysis/test/TestRunnerAction.java | 22 +++- .../TestFileNameConstants.java | 1 + src/test/shell/bazel/bazel_test_test.sh | 107 +++++++++++++++++- tools/test/test-setup.sh | 9 +- 5 files changed, 140 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java index f2f5a7450cf3a7..42053f8f2d4353 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestConfiguration.java @@ -298,6 +298,14 @@ public static class TestOptions extends FragmentOptions { help = "If true, then Bazel will run coverage postprocessing for test in a new spawn.") public boolean splitCoveragePostProcessing; + @Option( + name = "zip_undeclared_test_outputs", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.TESTING, + effectTags = {OptionEffectTag.TEST_RUNNER}, + help = "If true, undeclared test outputs will be archived in a zip file.") + public boolean zipUndeclaredTestOutputs; + @Override public FragmentOptions getHost() { TestOptions hostOptions = (TestOptions) getDefault(); @@ -373,7 +381,7 @@ public Label getCoverageSupport(){ return options.coverageSupport; } - public Label getCoverageReportGenerator(){ + public Label getCoverageReportGenerator() { return options.coverageReportGenerator; } @@ -410,6 +418,10 @@ public boolean splitCoveragePostProcessing() { return options.splitCoveragePostProcessing; } + public boolean getZipUndeclaredTestOutputs() { + return options.zipUndeclaredTestOutputs; + } + /** * Option converter that han handle two styles of value for "--runs_per_test": * diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index 327f2d4b9eff62..83385b41a2c827 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -339,7 +339,11 @@ public List getSpawnOutputs() { outputs.add(ActionInputHelper.fromPath(getSplitLogsPath())); outputs.add(ActionInputHelper.fromPath(getUnusedRunfilesLogPath())); outputs.add(ActionInputHelper.fromPath(getInfrastructureFailureFile())); - outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsZipPath())); + if (testConfiguration.getZipUndeclaredTestOutputs()) { + outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsZipPath())); + } else { + outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsDir())); + } outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsManifestPath())); outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsAnnotationsPath())); outputs.add(ActionInputHelper.fromPath(getUndeclaredOutputsAnnotationsPbPath())); @@ -383,12 +387,20 @@ public ImmutableList> getTestOutputsMapping( builder.add( Pair.of(TestFileNameConstants.TEST_WARNINGS, resolvedPaths.getTestWarningsPath())); } - if (resolvedPaths.getUndeclaredOutputsZipPath().exists()) { + if (testConfiguration.getZipUndeclaredTestOutputs() + && resolvedPaths.getUndeclaredOutputsZipPath().exists()) { builder.add( Pair.of( TestFileNameConstants.UNDECLARED_OUTPUTS_ZIP, resolvedPaths.getUndeclaredOutputsZipPath())); } + if (!testConfiguration.getZipUndeclaredTestOutputs() + && resolvedPaths.getUndeclaredOutputsDir().exists()) { + builder.add( + Pair.of( + TestFileNameConstants.UNDECLARED_OUTPUTS_DIR, + resolvedPaths.getUndeclaredOutputsDir())); + } if (resolvedPaths.getUndeclaredOutputsManifestPath().exists()) { builder.add( Pair.of( @@ -451,6 +463,7 @@ protected void computeKey( fp.addInt(runNumber); fp.addInt(executionSettings.getTotalRuns()); fp.addBoolean(configuration.isCodeCoverageEnabled()); + fp.addBoolean(testConfiguration.getZipUndeclaredTestOutputs()); fp.addStringMap(getExecutionInfo()); } @@ -615,7 +628,10 @@ public void setupEnvVariables(Map env, Duration timeout) { env.put("TEST_LOGSPLITTER_OUTPUT_FILE", getSplitLogsPath().getPathString()); - env.put("TEST_UNDECLARED_OUTPUTS_ZIP", getUndeclaredOutputsZipPath().getPathString()); + if (testConfiguration.getZipUndeclaredTestOutputs()) { + env.put("TEST_UNDECLARED_OUTPUTS_ZIP", getUndeclaredOutputsZipPath().getPathString()); + } + env.put("TEST_UNDECLARED_OUTPUTS_DIR", getUndeclaredOutputsDir().getPathString()); env.put("TEST_UNDECLARED_OUTPUTS_MANIFEST", getUndeclaredOutputsManifestPath().getPathString()); env.put( diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/TestFileNameConstants.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/TestFileNameConstants.java index 7d26f6ab2f51c8..1b470564463777 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/TestFileNameConstants.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/TestFileNameConstants.java @@ -33,6 +33,7 @@ public class TestFileNameConstants { "test.outputs_manifest__ANNOTATIONS.pb"; public static final String UNDECLARED_OUTPUTS_MANIFEST = "test.outputs_manifest__MANIFEST"; public static final String UNDECLARED_OUTPUTS_ZIP = "test.outputs__outputs.zip"; + public static final String UNDECLARED_OUTPUTS_DIR = "test.outputs"; public static final String UNUSED_RUNFILES_LOG = "test.unused_runfiles_log"; public static final String TEST_COVERAGE = "test.lcov"; public static final String BASELINE_COVERAGE = "baseline.lcov"; diff --git a/src/test/shell/bazel/bazel_test_test.sh b/src/test/shell/bazel/bazel_test_test.sh index 87f3bbb7d2aad7..c1030e40071316 100755 --- a/src/test/shell/bazel/bazel_test_test.sh +++ b/src/test/shell/bazel/bazel_test_test.sh @@ -695,7 +695,7 @@ EOF assert_equals "fail" "$(awk "NR == $(wc -l < $TEST_log)" $TEST_log)" } -function test_undeclared_outputs_are_zipped_and_manifest_exists() { +function setup_undeclared_outputs_test() { mkdir -p dir cat <<'EOF' > dir/test.sh @@ -714,6 +714,15 @@ sh_test( srcs = [ "test.sh" ], ) EOF +} + +function test_undeclared_outputs_are_zipped() { + setup_undeclared_outputs_test + + local -r outputs_dir=bazel-testlogs/dir/test/test.outputs + local -r outputs_zip=$outputs_dir/outputs.zip + local -r output_text=$outputs_dir/text.txt + local -r output_html=$outputs_dir/fake.html bazel test -s //dir:test &> $TEST_log || fail "expected success" @@ -721,29 +730,115 @@ EOF N=$'\n' # Check that the undeclared outputs zip file exists. - outputs_zip=bazel-testlogs/dir/test/test.outputs/outputs.zip [ -s $outputs_zip ] || fail "$outputs_zip was not present after test" + # Check that the original undeclared outputs no longer exist. + [ -e $output_text ] && fail "$output_text was present after test" + [ -e $output_text ] && fail "$output_text was present after test" + + # Check the contents of the zip file. unzip -q "$outputs_zip" -d unzipped_outputs || fail "failed to unzip $outputs_zip" cat > expected_text < d || fail "unzipped_outputs/text.txt differs from expected:$N$(cat d)$N" + diff "unzipped_outputs/text.txt" expected_text > d || fail "unzipped_outputs/text.txt differs from expected:$N$(cat d)$N" + cat > expected_html < +EOF + diff expected_html "unzipped_outputs/fake.html" > d || fail "unzipped_outputs/fake.html differs from expected:$N$(cat d)$N" +} + +function test_undeclared_outputs_are_not_zipped() { + setup_undeclared_outputs_test + + local -r outputs_dir=bazel-testlogs/dir/test/test.outputs + local -r outputs_zip=$outputs_dir/outputs.zip + local -r output_text=$outputs_dir/text.txt + local -r output_html=$outputs_dir/fake.html + + bazel test -s --nozip_undeclared_test_outputs //dir:test &> $TEST_log || fail "expected success" + + # Newlines are useful around diffs. This helps us get them in bash strings. + N=$'\n' + + # Check that the undeclared outputs zip file does not exist. + [ -e $outputs_zip ] && fail "$outputs_zip was present after test" + + # Check that the undeclared outputs exist. + [ -e $output_text ] || fail "$output_text was not present after test" + [ -e $output_text ] || fail "$output_text was not present after test" + + # Check the contents of the undeclared outputs. + cat > expected_text < d || fail "$outputs_dir/text.txt differs from expected:$N$(cat d)$N" cat > expected_html < EOF -diff expected_html "unzipped_outputs/fake.html" > d || fail "unzipped_outputs/fake.html differs from expected:$N$(cat d)$N" + diff expected_html "$outputs_dir/fake.html" > d || fail "$outputs_dir/fake.html differs from expected:$N$(cat d)$N" +} + +function test_undeclared_outputs_zipped_then_unzipped() { + setup_undeclared_outputs_test + + local -r outputs_dir=bazel-testlogs/dir/test/test.outputs + local -r outputs_zip=$outputs_dir/outputs.zip + local -r output_text=$outputs_dir/text.txt + local -r output_html=$outputs_dir/fake.html + + bazel test -s //dir:test &> $TEST_log || fail "expected success" + + [ -s $output_text ] && fail "$output_text was present after test" + [ -s $output_html ] && fail "$output_html was present after test" + [ -s $outputs_zip ] || fail "$outputs_zip was not present after test" + + bazel test -s --nozip_undeclared_test_outputs //dir:test &> $TEST_log || fail "expected success" + + [ -s $outputs_zip ] && fail "$outputs_zip was present after test" + [ -s $output_text ] || fail "$output_text was not present after test" + [ -s $output_html ] || fail "$output_html was not present after test" +} + +function test_undeclared_outputs_unzipped_then_zipped() { + setup_undeclared_outputs_test + + local -r outputs_dir=bazel-testlogs/dir/test/test.outputs + local -r outputs_zip=$outputs_dir/outputs.zip + local -r output_text=$outputs_dir/text.txt + local -r output_html=$outputs_dir/fake.html + + bazel test -s --nozip_undeclared_test_outputs //dir:test &> $TEST_log || fail "expected success" + + [ -s $outputs_zip ] && fail "$outputs_zip was present after test" + [ -s $output_text ] || fail "$output_text was not present after test" + [ -s $output_html ] || fail "$output_html was not present after test" + + bazel test -s //dir:test &> $TEST_log || fail "expected success" + + [ -s $output_text ] && fail "$output_text was present after test" + [ -s $output_html ] && fail "$output_html was present after test" + [ -s $outputs_zip ] || fail "$outputs_zip was not present after test" +} + +function test_undeclared_outputs_manifest_is_created() { + setup_undeclared_outputs_test + + bazel test -s //dir:test &> $TEST_log || fail "expected success" + + # Newlines are useful around diffs. This helps us get them in bash strings. + N=$'\n' # Check that the undeclared outputs manifest exists and that it has the # correct contents. - outputs_manifest=bazel-testlogs/dir/test/test.outputs_manifest/MANIFEST + local -r outputs_manifest=bazel-testlogs/dir/test/test.outputs_manifest/MANIFEST [ -s $outputs_manifest ] || fail "$outputs_manifest was not present after test" cat > expected_manifest < d || fail "$outputs_manifest differs from expected:$N$(cat d)$N" + diff expected_manifest "$outputs_manifest" > d || fail "$outputs_manifest differs from expected:$N$(cat d)$N" } function test_undeclared_outputs_annotations_are_added() { diff --git a/tools/test/test-setup.sh b/tools/test/test-setup.sh index 171129d02975e3..b4affb2082921d 100755 --- a/tools/test/test-setup.sh +++ b/tools/test/test-setup.sh @@ -54,8 +54,10 @@ is_absolute "$TEST_UNDECLARED_OUTPUTS_DIR" || TEST_UNDECLARED_OUTPUTS_DIR="$PWD/$TEST_UNDECLARED_OUTPUTS_DIR" is_absolute "$TEST_UNDECLARED_OUTPUTS_MANIFEST" || TEST_UNDECLARED_OUTPUTS_MANIFEST="$PWD/$TEST_UNDECLARED_OUTPUTS_MANIFEST" -is_absolute "$TEST_UNDECLARED_OUTPUTS_ZIP" || - TEST_UNDECLARED_OUTPUTS_ZIP="$PWD/$TEST_UNDECLARED_OUTPUTS_ZIP" +if [[ -n "$TEST_UNDECLARED_OUTPUTS_ZIP" ]]; then + is_absolute "$TEST_UNDECLARED_OUTPUTS_ZIP" || + TEST_UNDECLARED_OUTPUTS_ZIP="$PWD/$TEST_UNDECLARED_OUTPUTS_ZIP" +fi is_absolute "$TEST_UNDECLARED_OUTPUTS_ANNOTATIONS" || TEST_UNDECLARED_OUTPUTS_ANNOTATIONS="$PWD/$TEST_UNDECLARED_OUTPUTS_ANNOTATIONS" is_absolute "$TEST_UNDECLARED_OUTPUTS_ANNOTATIONS_DIR" || @@ -420,7 +422,8 @@ if [[ -n "$TEST_UNDECLARED_OUTPUTS_ZIP" ]] && cd "$TEST_UNDECLARED_OUTPUTS_DIR"; # If * found nothing, echo printed the literal *. # Otherwise echo printed the top-level files and directories. # Pass files to zip with *, so paths with spaces aren't broken up. - zip -qr "$TEST_UNDECLARED_OUTPUTS_ZIP" -- * 2>/dev/null || \ + # Remove original files after zipping them. + zip -qrm "$TEST_UNDECLARED_OUTPUTS_ZIP" -- * 2>/dev/null || \ echo >&2 "Could not create \"$TEST_UNDECLARED_OUTPUTS_ZIP\": zip not found or failed" fi fi