Skip to content

Commit

Permalink
Report individual undeclared outputs in the BEP.
Browse files Browse the repository at this point in the history
Fixes bazelbuild#17911.

RELNOTES: Undeclared test outputs are now reported individually in the BEP, unless zipping is enabled via `--zip_undeclared_test_outputs`.
PiperOrigin-RevId: 673435793
Change-Id: I0f3df695bca3c1a785795d9a0cfd9500f0287e0a
  • Loading branch information
tjgq authored and copybara-github committed Sep 11, 2024
1 parent db9263a commit 9d5ad2b
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,11 @@
import com.google.devtools.build.lib.server.FailureDetails.TestAction;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.view.test.TestStatus.TestResultData;
import com.google.devtools.common.options.TriState;
import com.google.protobuf.ExtensionRegistry;
Expand All @@ -77,8 +79,10 @@
import java.io.OutputStream;
import java.time.Duration;
import java.util.AbstractCollection;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -401,7 +405,7 @@ public List<ActionInput> getSpawnOutputs() {
*/
// TODO(ulfjack): Instead of going to local disk here, use SpawnResult (add list of files there).
public ImmutableMultimap<String, Path> getTestOutputsMapping(
ArtifactPathResolver resolver, Path execRoot) {
ArtifactPathResolver resolver, Path execRoot) throws IOException {
// TODO(tjgq): The existence checks below will incorrectly return false if the test action was
// reconstructed from the action cache, as we don't populate the output filesystem on an action
// cache hit. This is difficult to fix because some of the files below are produced by test
Expand Down Expand Up @@ -436,9 +440,7 @@ public ImmutableMultimap<String, Path> getTestOutputsMapping(
}
if (!testConfiguration.getZipUndeclaredTestOutputs()
&& resolvedPaths.getUndeclaredOutputsDir().exists()) {

builder.put(
TestFileNameConstants.UNDECLARED_OUTPUTS_DIR, resolvedPaths.getUndeclaredOutputsDir());
addAllFilesInUndeclaredOutputsDirectory(builder, resolvedPaths.getUndeclaredOutputsDir());
}
if (resolvedPaths.getUndeclaredOutputsManifestPath().exists()) {
builder.put(
Expand Down Expand Up @@ -468,6 +470,30 @@ public ImmutableMultimap<String, Path> getTestOutputsMapping(
return builder.build();
}

private static void addAllFilesInUndeclaredOutputsDirectory(
ImmutableMultimap.Builder<String, Path> builder, Path undeclaredOutputsDir)
throws IOException {
ArrayDeque<Path> dirsToVisit = new ArrayDeque<>();
dirsToVisit.add(undeclaredOutputsDir);
while (!dirsToVisit.isEmpty()) {
Path dir = dirsToVisit.pop();
List<Dirent> sortedEntries = new ArrayList<>(dir.readdir(Symlinks.FOLLOW));
sortedEntries.sort(Comparator.comparing(Dirent::getName));
for (Dirent dirent : sortedEntries) {
Path child = dir.getChild(dirent.getName());
if (dirent.getType().equals(Dirent.Type.DIRECTORY)) {
dirsToVisit.add(child);
} else if (dirent.getType().equals(Dirent.Type.FILE)) {
String name =
TestFileNameConstants.UNDECLARED_OUTPUTS_DIR
+ "/"
+ child.relativeTo(undeclaredOutputsDir);
builder.put(name, child);
}
}
}
}

// Test actions are always distinguished by their target name, which must be unique.
@Override
public final boolean isShareable() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,23 @@ public class TestFileNameConstants {
public static final String TEST_STDERR = "test.stderr";
public static final String TEST_WARNINGS = "test.warnings";
public static final String TEST_XML = "test.xml";
public static final String UNUSED_RUNFILES_LOG = "test.unused_runfiles_log";

// Only present for the coverage command.
public static final String TEST_COVERAGE = "test.lcov";
public static final String BASELINE_COVERAGE = "baseline.lcov";

// Present for both --zip_undeclared_outputs and --nozip_undeclared_outputs.
public static final String UNDECLARED_OUTPUTS_ANNOTATIONS = "test.outputs_manifest__ANNOTATIONS";
public static final String UNDECLARED_OUTPUTS_ANNOTATIONS_PB =
"test.outputs_manifest__ANNOTATIONS.pb";
public static final String UNDECLARED_OUTPUTS_MANIFEST = "test.outputs_manifest__MANIFEST";

// Only present for --zip_undeclared_outputs.
public static final String UNDECLARED_OUTPUTS_ZIP = "test.outputs__outputs.zip";

// Only present for --nozip_undeclared_outputs.
// This is a prefix; each file in the undeclared outputs directory is reported individually, e.g.
// test.outputs/path/to/file.txt.
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";
}
31 changes: 30 additions & 1 deletion src/test/shell/bazel/remote/build_without_the_bytes_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,35 @@ EOF
expect_log "uri:.*bytestream://example.com/my-instance-name/blobs"
}

function test_undeclared_test_outputs_bep() {
function test_undeclared_test_outputs_unzipped_bep() {
# Test that when using --remote_download_minimal, undeclared outputs in a test
# are reported by BEP
mkdir -p a
cat > a/BUILD <<EOF
sh_test(
name = "foo",
srcs = ["foo.sh"],
)
EOF
cat > a/foo.sh <<'EOF'
touch $TEST_UNDECLARED_OUTPUTS_DIR/bar.txt
EOF
chmod a+x a/foo.sh

bazel test \
--remote_executor=grpc://localhost:${worker_port} \
--remote_download_minimal \
--build_event_text_file=$TEST_log \
//a:foo || fail "Failed to test //a:foo"

expect_log "test.log"
expect_log "test.xml"
expect_log "test.outputs/bar.txt"
expect_log "test.outputs_manifest__MANIFEST"
expect_not_log "test.outputs__outputs.zip"
}

function test_undeclared_test_outputs_zipped_bep() {
# Test that when using --remote_download_minimal, undeclared outputs in a test
# are reported by BEP
mkdir -p a
Expand All @@ -848,6 +876,7 @@ EOF
expect_log "test.xml"
expect_log "test.outputs__outputs.zip"
expect_log "test.outputs_manifest__MANIFEST"
expect_not_log "test.outputs/bar.txt"
}

function test_undeclared_test_outputs_unzipped() {
Expand Down

0 comments on commit 9d5ad2b

Please sign in to comment.