From da9e3bf92ce09abc7ece42d6c9567e3fae9ed47e Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Mon, 13 Feb 2023 13:28:07 +0100 Subject: [PATCH] Add tests --- .../build/lib/actions/ActionCacheUtils.java | 13 +++ .../remote/ActionInputPrefetcherTestBase.java | 8 +- .../google/devtools/build/lib/remote/BUILD | 1 + .../BuildWithoutTheBytesIntegrationTest.java | 52 +++++++++++- .../remote/RemoteActionInputFetcherTest.java | 20 +++++ .../lib/remote/util/IntegrationTestUtils.java | 79 ++++++++++++------- 6 files changed, 141 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheUtils.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheUtils.java index c8b832d35b1f15..ce7adba66b8be3 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheUtils.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheUtils.java @@ -1,3 +1,16 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. package com.google.devtools.build.lib.actions; import com.google.devtools.build.lib.actions.cache.ActionCache; diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java index 733de24a835916..52c1d03a462dcb 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java @@ -90,7 +90,7 @@ protected Artifact createRemoteArtifact( String contents, @Nullable PathFragment materializationExecPath, Map metadata, - Map cas) { + @Nullable Map cas) { Path p = artifactRoot.getRoot().getRelative(pathFragment); Artifact a = ActionsTestUtil.createArtifact(artifactRoot, p); byte[] contentsBytes = contents.getBytes(UTF_8); @@ -103,7 +103,9 @@ protected Artifact createRemoteArtifact( "action-id", materializationExecPath); metadata.put(a, f); - cas.put(hashCode, contentsBytes); + if (cas != null) { + cas.put(hashCode, contentsBytes); + } return a; } @@ -111,7 +113,7 @@ protected Artifact createRemoteArtifact( String pathFragment, String contents, Map metadata, - Map cas) { + @Nullable Map cas) { return createRemoteArtifact( pathFragment, contents, /* materializationExecPath= */ null, metadata, cas); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index e07286e18653e7..a65bcfc0ba3d25 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -162,6 +162,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/standalone", "//src/main/java/com/google/devtools/build/lib/util:os", + "//src/main/java/com/google/devtools/build/lib/vfs", "//src/test/java/com/google/devtools/build/lib/remote/util:integration_test_utils", "//third_party:guava", "//third_party:junit4", diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java index 8c49c36b9c7d6f..e74aa0c756bc77 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.runtime.BuildSummaryStatsModule; import com.google.devtools.build.lib.standalone.StandaloneModule; import com.google.devtools.build.lib.util.OS; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import java.io.IOException; import org.junit.After; import org.junit.Test; @@ -69,6 +70,7 @@ protected void setDownloadAll() { @Override protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception { return super.getRuntimeBuilder() + .addBlazeModule(new RemoteModule()) .addBlazeModule(new BuildSummaryStatsModule()) .addBlazeModule(new BlockWaitingModule()); } @@ -79,7 +81,6 @@ protected ImmutableList getSpawnModules() { .addAll(super.getSpawnModules()) .add(new StandaloneModule()) .add(new CredentialModule()) - .add(new RemoteModule()) .add(new DynamicExecutionModule()) .build(); } @@ -415,4 +416,53 @@ public void symlinkToNestedDirectory() throws Exception { buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote"); } + + @Test + public void remoteCacheEvictBlobs_incrementalBuildCanContinue() throws Exception { + // Arrange: Prepare workspace and populate remote cache + write( + "a/BUILD", + "genrule(", + " name = 'foo',", + " srcs = ['foo.in'],", + " outs = ['foo.out'],", + " cmd = 'cat $(SRCS) > $@',", + ")", + "genrule(", + " name = 'bar',", + " srcs = ['foo.out', 'bar.in'],", + " outs = ['bar.out'],", + " cmd = 'cat $(SRCS) > $@',", + " tags = ['no-remote-exec'],", + ")"); + write("a/foo.in", "foo"); + write("a/bar.in", "bar"); + + // Populate remote cache + buildTarget("//a:bar"); + var bytes = FileSystemUtils.readContent(getOutputPath("a/foo.out")); + var hashCode = getDigestHashFunction().getHashFunction().hashBytes(bytes); + getOutputPath("a/foo.out").delete(); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + + // Clean build, foo.out isn't downloaded + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo.out"); + + // Evict blobs from remote cache + worker.restart(); + + // trigger build error + write("a/bar.in", "updated bar"); + // Build failed because of remote cache eviction + assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + + // Act: Do an incremental build without "clean" or "shutdown" + buildTarget("//a:bar"); + + // Assert: target was successfully built + assertValidOutputFile("a/bar.out", "foo\nupdated bar\n"); + } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java index 5e32976f4af605..b9aa2d1fdc3084 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.remote; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import build.bazel.remote.execution.v2.CacheCapabilities; import build.bazel.remote.execution.v2.Digest; @@ -21,10 +22,14 @@ import com.google.common.collect.Maps; import com.google.common.eventbus.EventBus; import com.google.common.hash.HashCode; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.InMemoryCacheClient; @@ -127,6 +132,21 @@ public void testStagingEmptyVirtualActionInput() throws Exception { assertThat(actionInputFetcher.downloadsInProgress()).isEmpty(); } + @Test + public void missingInputs_addedToList() { + Map metadata = new HashMap<>(); + Map cas = new HashMap<>(); + Artifact a = createRemoteArtifact("file", "hello world", metadata, /* cas= */ null); + MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); + AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); + + assertThrows( + BulkTransferException.class, + () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider))); + + assertThat(prefetcher.getMissingActionInputs()).contains(a); + } + private RemoteCache newCache( RemoteOptions options, DigestUtil digestUtil, Map cas) { Map cacheEntries = Maps.newHashMapWithExpectedSize(cas.size()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/IntegrationTestUtils.java b/src/test/java/com/google/devtools/build/lib/remote/util/IntegrationTestUtils.java index 44e204f06b53f7..4273f2dc04178e 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/IntegrationTestUtils.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/IntegrationTestUtils.java @@ -27,6 +27,10 @@ import java.net.DatagramSocket; import java.net.ServerSocket; import java.net.SocketException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Comparator; import java.util.Random; /** Integration test utilities. */ @@ -84,30 +88,9 @@ public static WorkerInstance startWorker(boolean useHttp) PathFragment casPath = testTmpDir.getRelative("remote.cas_path"); PathFragment pidPath = testTmpDir.getRelative("remote.pid_file"); int workerPort = pickUnusedRandomPort(); - ensureMkdir(workPath); - ensureMkdir(casPath); - String workerPath = Runfiles.create().rlocation(WORKER_PATH.getSafePathString()); - Subprocess workerProcess = - new SubprocessBuilder() - .setArgv( - ImmutableList.of( - workerPath, - "--work_path=" + workPath.getSafePathString(), - "--cas_path=" + casPath.getSafePathString(), - (useHttp ? "--http_listen_port=" : "--listen_port=") + workerPort, - "--pid_file=" + pidPath)) - .start(); - - File pidFile = new File(pidPath.getSafePathString()); - while (!pidFile.exists()) { - if (!workerProcess.isAlive()) { - String message = new String(workerProcess.getErrorStream().readAllBytes(), UTF_8); - throw new IOException("Failed to start worker: " + message); - } - Thread.sleep(1); - } - - return new WorkerInstance(workerProcess, workerPort, workPath, casPath, pidPath); + var worker = new WorkerInstance(useHttp, workerPort, workPath, casPath, pidPath); + worker.start(); + return worker; } private static void ensureMkdir(PathFragment path) throws IOException { @@ -121,27 +104,67 @@ private static void ensureMkdir(PathFragment path) throws IOException { } public static class WorkerInstance { - private final Subprocess process; + private Subprocess process; + private final boolean useHttp; private final int port; private final PathFragment workPath; private final PathFragment casPath; private final PathFragment pidPath; private WorkerInstance( - Subprocess process, + boolean useHttp, int port, PathFragment workPath, PathFragment casPath, PathFragment pidPath) { - this.process = process; + this.useHttp = useHttp; this.port = port; this.workPath = workPath; this.casPath = casPath; this.pidPath = pidPath; } - public void stop() { + private void start() throws IOException, InterruptedException { + ensureMkdir(workPath); + ensureMkdir(casPath); + + var workerPath = Runfiles.create().rlocation(WORKER_PATH.getSafePathString()); + process = + new SubprocessBuilder() + .setArgv( + ImmutableList.of( + workerPath, + "--work_path=" + workPath.getSafePathString(), + "--cas_path=" + casPath.getSafePathString(), + (useHttp ? "--http_listen_port=" : "--listen_port=") + port, + "--pid_file=" + pidPath)) + .start(); + + File pidFile = new File(pidPath.getSafePathString()); + while (!pidFile.exists()) { + if (!process.isAlive()) { + String message = new String(process.getErrorStream().readAllBytes(), UTF_8); + throw new IOException("Failed to start worker: " + message); + } + Thread.sleep(1); + } + } + + public void restart() throws IOException, InterruptedException { + stop(); + start(); + } + + public void stop() throws IOException { process.destroyAndWait(); + deleteDir(workPath); + deleteDir(casPath); + } + + private static void deleteDir(PathFragment path) throws IOException { + try (var stream = Files.walk(Paths.get(path.getSafePathString()))) { + stream.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete); + } } public int getPort() {