From 559a07d2dd88a53d6dac8bf2d77a28db44fd7659 Mon Sep 17 00:00:00 2001 From: buchgr Date: Thu, 30 Nov 2017 11:09:35 -0800 Subject: Refactor the FileSystem API to allow for different hash functions. Refactor the FileSystem class to include the hash function as an instance field. This allows us to have a different hash function per FileSystem and removes technical debt, as currently that's somewhat accomplished by a horrible hack that has a static method to set the hash function for all FileSystem instances. The FileSystem's default hash function remains MD5. RELNOTES: None PiperOrigin-RevId: 177479772 --- src/test/java/com/google/devtools/build/lib/BUILD | 1 + .../build/lib/actions/DigestUtilsTest.java | 61 +++++++++++----------- .../build/lib/remote/ByteStreamUploaderTest.java | 27 +++++----- .../build/lib/remote/CasPathConverterTest.java | 11 ++++ .../devtools/build/lib/remote/ChunkerTest.java | 17 +++--- .../build/lib/remote/FakeActionInputFileCache.java | 4 +- .../build/lib/remote/GrpcRemoteCacheTest.java | 46 ++++++++-------- .../lib/remote/GrpcRemoteExecutionClientTest.java | 30 ++++++----- .../build/lib/remote/RemoteSpawnCacheTest.java | 17 ++++-- .../build/lib/remote/RemoteSpawnRunnerTest.java | 51 ++++++++++++------ .../build/lib/remote/TreeNodeRepositoryTest.java | 10 ++-- 11 files changed, 168 insertions(+), 107 deletions(-) (limited to 'src/test/java/com/google/devtools/build') diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 90cb70680a..4cb175a461 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -1067,6 +1067,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:io", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/authandtls", + "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", diff --git a/src/test/java/com/google/devtools/build/lib/actions/DigestUtilsTest.java b/src/test/java/com/google/devtools/build/lib/actions/DigestUtilsTest.java index fddab806b9..259e0f2088 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/DigestUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/DigestUtilsTest.java @@ -55,28 +55,28 @@ public class DigestUtilsTest { final CountDownLatch barrierLatch = new CountDownLatch(2); // Used to block test threads. final CountDownLatch readyLatch = new CountDownLatch(1); // Used to block main thread. - FileSystem myfs = new InMemoryFileSystem(BlazeClock.instance()) { - @Override - protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException { - try { - barrierLatch.countDown(); - readyLatch.countDown(); - // Either both threads will be inside getDigest at the same time or they - // both will be blocked. - barrierLatch.await(); - } catch (Exception e) { - throw new IOException(e); + FileSystem myfs = + new InMemoryFileSystem(BlazeClock.instance(), hf) { + @Override + protected byte[] getDigest(Path path, HashFunction hashFunction) throws IOException { + try { + barrierLatch.countDown(); + readyLatch.countDown(); + // Either both threads will be inside getDigest at the same time or they + // both will be blocked. + barrierLatch.await(); + } catch (Exception e) { + throw new IOException(e); + } + return super.getDigest(path, hashFunction); } - return super.getDigest(path, hashFunction); - } - @Override - protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException { - return fastDigest ? super.getDigest(path, hashFunction) : null; - } - }; + @Override + protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException { + return fastDigest ? super.getDigest(path, hashFunction) : null; + } + }; - FileSystem.setDigestFunctionForTesting(hf); final Path myFile1 = myfs.getPath("/f1.dat"); final Path myFile2 = myfs.getPath("/f2.dat"); FileSystemUtils.writeContentAsLatin1(myFile1, Strings.repeat("a", fileSize1)); @@ -126,18 +126,19 @@ public class DigestUtilsTest { } public void assertRecoverFromMalformedDigest(HashFunction... hashFunctions) throws Exception { - final byte[] malformed = {0, 0, 0}; - FileSystem myFS = new InMemoryFileSystem(BlazeClock.instance()) { - @Override - protected byte[] getFastDigest(Path path, HashFunction hashFunction) throws IOException { - // Digest functions have more than 3 bytes, usually at least 16. - return malformed; - } - }; - Path path = myFS.getPath("/file"); - FileSystemUtils.writeContentAsLatin1(path, "a"); for (HashFunction hf : hashFunctions) { - FileSystem.setDigestFunctionForTesting(hf); + final byte[] malformed = {0, 0, 0}; + FileSystem myFS = + new InMemoryFileSystem(BlazeClock.instance(), hf) { + @Override + protected byte[] getFastDigest(Path path, HashFunction hashFunction) + throws IOException { + // Digest functions have more than 3 bytes, usually at least 16. + return malformed; + } + }; + Path path = myFS.getPath("/file"); + FileSystemUtils.writeContentAsLatin1(path, "a"); byte[] result = DigestUtils.getDigestOrFail(path, 1); assertThat(result).isEqualTo(path.getDigest()); assertThat(result).isNotSameAs(malformed); diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java index e809b74b7e..0b70f36623 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java @@ -26,6 +26,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.remoteexecution.v1test.Digest; import com.google.devtools.remoteexecution.v1test.RequestMetadata; import com.google.protobuf.ByteString; @@ -75,6 +76,8 @@ import org.mockito.MockitoAnnotations; @RunWith(JUnit4.class) public class ByteStreamUploaderTest { + private static final DigestUtil DIGEST_UTIL = new DigestUtil(HashFunction.SHA256); + private static final int CHUNK_SIZE = 10; private static final String INSTANCE_NAME = "foo"; @@ -99,7 +102,7 @@ public class ByteStreamUploaderTest { channel = InProcessChannelBuilder.forName(serverName).build(); withEmptyMetadata = TracingMetadataUtils.contextWithMetadata( - "none", "none", Digests.unsafeActionKeyFromDigest(Digest.getDefaultInstance())); + "none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance())); // Needs to be repeated in every test that uses the timeout setting, since the tests run // on different threads than the setUp. withEmptyMetadata.attach(); @@ -121,7 +124,7 @@ public class ByteStreamUploaderTest { byte[] blob = new byte[CHUNK_SIZE * 2 + 1]; new Random().nextBytes(blob); - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); serviceRegistry.addService(new ByteStreamImplBase() { @Override @@ -195,7 +198,7 @@ public class ByteStreamUploaderTest { int blobSize = rand.nextInt(CHUNK_SIZE * 10) + CHUNK_SIZE; byte[] blob = new byte[blobSize]; rand.nextBytes(blob); - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); builders.add(chunker); blobsByHash.put(chunker.digest().getHash(), blob); } @@ -281,7 +284,7 @@ public class ByteStreamUploaderTest { List builders = new ArrayList<>(toUpload.size()); Map uploadsFailed = new HashMap<>(); for (String s : toUpload) { - Chunker chunker = new Chunker(s.getBytes(UTF_8), /* chunkSize=*/ 3); + Chunker chunker = new Chunker(s.getBytes(UTF_8), /* chunkSize=*/ 3, DIGEST_UTIL); builders.add(chunker); uploadsFailed.put(chunker.digest().getHash(), 0); } @@ -342,7 +345,7 @@ public class ByteStreamUploaderTest { for (Chunker chunker : builders) { Context ctx = TracingMetadataUtils.contextWithMetadata( - "build-req-id", "command-id", Digests.unsafeActionKeyFromDigest(chunker.digest())); + "build-req-id", "command-id", DIGEST_UTIL.asActionKey(chunker.digest())); ctx.call( () -> { uploads.add(uploader.uploadBlobAsync(chunker)); @@ -367,7 +370,7 @@ public class ByteStreamUploaderTest { new ByteStreamUploader(INSTANCE_NAME, channel, null, 3, retrier, retryService); byte[] blob = new byte[CHUNK_SIZE * 10]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); AtomicInteger numWriteCalls = new AtomicInteger(); CountDownLatch blocker = new CountDownLatch(1); @@ -426,7 +429,7 @@ public class ByteStreamUploaderTest { new ByteStreamUploader(INSTANCE_NAME, channel, null, 3, retrier, retryService); byte[] blob = new byte[CHUNK_SIZE]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); serviceRegistry.addService(new ByteStreamImplBase() { @Override @@ -477,10 +480,10 @@ public class ByteStreamUploaderTest { serviceRegistry.addService(service); byte[] blob1 = new byte[CHUNK_SIZE]; - Chunker chunker1 = new Chunker(blob1, CHUNK_SIZE); + Chunker chunker1 = new Chunker(blob1, CHUNK_SIZE, DIGEST_UTIL); byte[] blob2 = new byte[CHUNK_SIZE + 1]; - Chunker chunker2 = new Chunker(blob2, CHUNK_SIZE); + Chunker chunker2 = new Chunker(blob2, CHUNK_SIZE, DIGEST_UTIL); ListenableFuture f1 = uploader.uploadBlobAsync(chunker1); ListenableFuture f2 = uploader.uploadBlobAsync(chunker2); @@ -519,7 +522,7 @@ public class ByteStreamUploaderTest { assertThat(retryService.isShutdown()).isTrue(); byte[] blob = new byte[1]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); try { uploader.uploadBlob(chunker); fail("Should have thrown an exception."); @@ -560,7 +563,7 @@ public class ByteStreamUploaderTest { }); byte[] blob = new byte[1]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); uploader.uploadBlob(chunker); } @@ -585,7 +588,7 @@ public class ByteStreamUploaderTest { }); byte[] blob = new byte[1]; - Chunker chunker = new Chunker(blob, CHUNK_SIZE); + Chunker chunker = new Chunker(blob, CHUNK_SIZE, DIGEST_UTIL); try { uploader.uploadBlob(chunker); diff --git a/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java b/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java index 5ba4ef0435..493041b5dd 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/CasPathConverterTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.devtools.build.lib.remote.RemoteModule.CasPathConverter; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; @@ -34,12 +35,20 @@ public class CasPathConverterTest { @Test public void noOptionsShouldntCrash() { + converter.digestUtil = new DigestUtil(HashFunction.SHA256); + assertThat(converter.apply(fs.getPath("/foo"))).isNull(); + } + + @Test + public void noDigestUtilShouldntCrash() { + converter.options = Options.getDefaults(RemoteOptions.class); assertThat(converter.apply(fs.getPath("/foo"))).isNull(); } @Test public void disabledRemote() { converter.options = Options.getDefaults(RemoteOptions.class); + converter.digestUtil = new DigestUtil(HashFunction.SHA256); assertThat(converter.apply(fs.getPath("/foo"))).isNull(); } @@ -48,6 +57,7 @@ public class CasPathConverterTest { OptionsParser parser = OptionsParser.newOptionsParser(RemoteOptions.class); parser.parse("--remote_cache=machine"); converter.options = parser.getOptions(RemoteOptions.class); + converter.digestUtil = new DigestUtil(HashFunction.SHA256); Path path = fs.getPath("/foo"); FileSystemUtils.writeContentAsLatin1(path, "foobar"); assertThat(converter.apply(fs.getPath("/foo"))) @@ -59,6 +69,7 @@ public class CasPathConverterTest { OptionsParser parser = OptionsParser.newOptionsParser(RemoteOptions.class); parser.parse("--remote_cache=machine", "--remote_instance_name=projects/bazel"); converter.options = parser.getOptions(RemoteOptions.class); + converter.digestUtil = new DigestUtil(HashFunction.SHA256); Path path = fs.getPath("/foo"); FileSystemUtils.writeContentAsLatin1(path, "foobar"); assertThat(converter.apply(fs.getPath("/foo"))) diff --git a/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java b/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java index fd0cbade33..ed1ccf6ed2 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ChunkerTest.java @@ -17,6 +17,7 @@ import static com.google.common.truth.Truth.assertThat; import static junit.framework.TestCase.fail; import com.google.devtools.build.lib.remote.Chunker.Chunk; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.remoteexecution.v1test.Digest; import com.google.protobuf.ByteString; import java.io.ByteArrayInputStream; @@ -36,14 +37,16 @@ import org.mockito.Mockito; @RunWith(JUnit4.class) public class ChunkerTest { + private final DigestUtil digestUtil = new DigestUtil(HashFunction.SHA256); + @Test public void chunkingShouldWork() throws IOException { Random rand = new Random(); byte[] expectedData = new byte[21]; rand.nextBytes(expectedData); - Digest expectedDigest = Digests.computeDigest(expectedData); + Digest expectedDigest = digestUtil.compute(expectedData); - Chunker chunker = new Chunker(expectedData, 10); + Chunker chunker = new Chunker(expectedData, 10, digestUtil); ByteArrayOutputStream actualData = new ByteArrayOutputStream(); @@ -76,7 +79,7 @@ public class ChunkerTest { @Test public void nextShouldThrowIfNoMoreData() throws IOException { byte[] data = new byte[10]; - Chunker chunker = new Chunker(data, 10); + Chunker chunker = new Chunker(data, 10, digestUtil); assertThat(chunker.hasNext()).isTrue(); assertThat(chunker.next()).isNotNull(); @@ -94,7 +97,7 @@ public class ChunkerTest { @Test public void emptyData() throws Exception { byte[] data = new byte[0]; - Chunker chunker = new Chunker(data); + Chunker chunker = new Chunker(data, digestUtil); assertThat(chunker.hasNext()).isTrue(); @@ -117,7 +120,7 @@ public class ChunkerTest { @Test public void reset() throws Exception { byte[] data = new byte[]{1, 2, 3}; - Chunker chunker = new Chunker(data, 1); + Chunker chunker = new Chunker(data, 1, digestUtil); assertNextEquals(chunker, (byte) 1); assertNextEquals(chunker, (byte) 2); @@ -144,9 +147,9 @@ public class ChunkerTest { in.set(Mockito.spy(new ByteArrayInputStream(data))); return in.get(); }; - Digest digest = Digests.computeDigest(data); + Digest digest = digestUtil.compute(data); - Chunker chunker = new Chunker(supplier, digest, 1); + Chunker chunker = new Chunker(supplier, digest, 1, digestUtil); assertThat(in.get()).isNull(); assertNextEquals(chunker, (byte) 1); Mockito.verify(in.get(), Mockito.never()).close(); diff --git a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java index 565e5e927d..3f509684f2 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java +++ b/src/test/java/com/google/devtools/build/lib/remote/FakeActionInputFileCache.java @@ -32,9 +32,11 @@ import javax.annotation.Nullable; final class FakeActionInputFileCache implements ActionInputFileCache { private final Path execRoot; private final BiMap cas = HashBiMap.create(); + private final DigestUtil digestUtil; FakeActionInputFileCache(Path execRoot) { this.execRoot = execRoot; + this.digestUtil = new DigestUtil(execRoot.getFileSystem().getDigestFunction()); } @Override @@ -70,7 +72,7 @@ final class FakeActionInputFileCache implements ActionInputFileCache { Path inputFile = execRoot.getRelative(input.getExecPath()); FileSystemUtils.createDirectoryAndParents(inputFile.getParentDirectory()); FileSystemUtils.writeContentAsLatin1(inputFile, content); - Digest digest = Digests.computeDigest(inputFile); + Digest digest = digestUtil.compute(inputFile); setDigest(input, digest.getHash()); return digest; } diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java index 5cc03be824..5664ea37cc 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java @@ -29,7 +29,8 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; import com.google.devtools.build.lib.authandtls.GrpcUtils; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.clock.JavaClock; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; @@ -75,6 +76,9 @@ import org.mockito.stubbing.Answer; /** Tests for {@link GrpcRemoteCache}. */ @RunWith(JUnit4.class) public class GrpcRemoteCacheTest { + + private static final DigestUtil DIGEST_UTIL = new DigestUtil(HashFunction.SHA256); + private FileSystem fs; private Path execRoot; private FileOutErr outErr; @@ -85,7 +89,6 @@ public class GrpcRemoteCacheTest { @Before public final void setUp() throws Exception { - FileSystem.setDigestFunctionForTesting(HashFunction.SHA1); // Use a mutable service registry for later registering the service impl for each test case. fakeServer = InProcessServerBuilder.forName(fakeServerName) @@ -94,7 +97,7 @@ public class GrpcRemoteCacheTest { .build() .start(); Chunker.setDefaultChunkSizeForTesting(1000); // Enough for everything to be one chunk. - fs = new InMemoryFileSystem(); + fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256); execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -106,7 +109,7 @@ public class GrpcRemoteCacheTest { outErr = new FileOutErr(stdout, stderr); Context withEmptyMetadata = TracingMetadataUtils.contextWithMetadata( - "none", "none", Digests.unsafeActionKeyFromDigest(Digest.getDefaultInstance())); + "none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance())); withEmptyMetadata.attach(); } @@ -156,13 +159,14 @@ public class GrpcRemoteCacheTest { ImmutableList.of(new CallCredentialsInterceptor(creds))), creds, remoteOptions, - retrier); + retrier, + DIGEST_UTIL); } @Test public void testDownloadEmptyBlob() throws Exception { GrpcRemoteCache client = newClient(); - Digest emptyDigest = Digests.computeDigest(new byte[0]); + Digest emptyDigest = DIGEST_UTIL.compute(new byte[0]); // Will not call the mock Bytestream interface at all. assertThat(client.downloadBlob(emptyDigest)).isEmpty(); } @@ -170,7 +174,7 @@ public class GrpcRemoteCacheTest { @Test public void testDownloadBlobSingleChunk() throws Exception { final GrpcRemoteCache client = newClient(); - final Digest digest = Digests.computeDigestUtf8("abcdefg"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg"); serviceRegistry.addService( new ByteStreamImplBase() { @Override @@ -187,7 +191,7 @@ public class GrpcRemoteCacheTest { @Test public void testDownloadBlobMultipleChunks() throws Exception { final GrpcRemoteCache client = newClient(); - final Digest digest = Digests.computeDigestUtf8("abcdefg"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg"); serviceRegistry.addService( new ByteStreamImplBase() { @Override @@ -208,9 +212,9 @@ public class GrpcRemoteCacheTest { @Test public void testDownloadAllResults() throws Exception { GrpcRemoteCache client = newClient(); - Digest fooDigest = Digests.computeDigestUtf8("foo-contents"); - Digest barDigest = Digests.computeDigestUtf8("bar-contents"); - Digest emptyDigest = Digests.computeDigest(new byte[0]); + Digest fooDigest = DIGEST_UTIL.computeAsUtf8("foo-contents"); + Digest barDigest = DIGEST_UTIL.computeAsUtf8("bar-contents"); + Digest emptyDigest = DIGEST_UTIL.compute(new byte[0]); serviceRegistry.addService( new FakeImmutableCacheByteStreamImpl(fooDigest, "foo-contents", barDigest, "bar-contents")); @@ -219,16 +223,16 @@ public class GrpcRemoteCacheTest { result.addOutputFilesBuilder().setPath("b/empty").setDigest(emptyDigest); result.addOutputFilesBuilder().setPath("a/bar").setDigest(barDigest).setIsExecutable(true); client.download(result.build(), execRoot, null); - assertThat(Digests.computeDigest(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); - assertThat(Digests.computeDigest(execRoot.getRelative("b/empty"))).isEqualTo(emptyDigest); - assertThat(Digests.computeDigest(execRoot.getRelative("a/bar"))).isEqualTo(barDigest); + assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); + assertThat(DIGEST_UTIL.compute(execRoot.getRelative("b/empty"))).isEqualTo(emptyDigest); + assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/bar"))).isEqualTo(barDigest); assertThat(execRoot.getRelative("a/bar").isExecutable()).isTrue(); } @Test public void testUploadBlobCacheHitWithRetries() throws Exception { final GrpcRemoteCache client = newClient(); - final Digest digest = Digests.computeDigestUtf8("abcdefg"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg"); serviceRegistry.addService( new ContentAddressableStorageImplBase() { private int numErrors = 4; @@ -251,7 +255,7 @@ public class GrpcRemoteCacheTest { @Test public void testUploadBlobSingleChunk() throws Exception { final GrpcRemoteCache client = newClient(); - final Digest digest = Digests.computeDigestUtf8("abcdefg"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg"); serviceRegistry.addService( new ContentAddressableStorageImplBase() { @Override @@ -302,7 +306,7 @@ public class GrpcRemoteCacheTest { this.responseObserver = responseObserver; this.contents = contents; try { - chunker = new Chunker(contents.getBytes(UTF_8), chunkSizeBytes); + chunker = new Chunker(contents.getBytes(UTF_8), chunkSizeBytes, DIGEST_UTIL); } catch (IOException e) { fail("An error occurred:" + e); } @@ -357,7 +361,7 @@ public class GrpcRemoteCacheTest { @Test public void testUploadBlobMultipleChunks() throws Exception { - final Digest digest = Digests.computeDigestUtf8("abcdef"); + final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdef"); serviceRegistry.addService( new ContentAddressableStorageImplBase() { @Override @@ -446,7 +450,7 @@ public class GrpcRemoteCacheTest { } }); - ActionKey emptyKey = Digests.computeActionKey(Action.getDefaultInstance()); + ActionKey emptyKey = DIGEST_UTIL.computeActionKey(Action.getDefaultInstance()); Path fooFile = execRoot.getRelative("a/foo"); Path barFile = execRoot.getRelative("bar"); client.upload(emptyKey, execRoot, ImmutableList.of(fooFile, barFile), outErr, false); @@ -464,7 +468,7 @@ public class GrpcRemoteCacheTest { final Path fooFile = execRoot.getRelative("a/foo"); final Path barFile = execRoot.getRelative("bar"); final Path bazFile = execRoot.getRelative("baz"); - ActionKey actionKey = Digests.unsafeActionKeyFromDigest(fooDigest); // Could be any key. + ActionKey actionKey = DIGEST_UTIL.asActionKey(fooDigest); // Could be any key. barFile.setExecutable(true); serviceRegistry.addService( new ContentAddressableStorageImplBase() { @@ -576,7 +580,7 @@ public class GrpcRemoteCacheTest { @Test public void testGetCachedActionResultWithRetries() throws Exception { final GrpcRemoteCache client = newClient(); - ActionKey actionKey = Digests.unsafeActionKeyFromDigest(Digests.computeDigestUtf8("key")); + ActionKey actionKey = DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")); serviceRegistry.addService( new ActionCacheImplBase() { private int numErrors = 4; diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java index 35bd1950e3..2e06805539 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; import com.google.devtools.build.lib.authandtls.GrpcUtils; +import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.exec.SpawnExecException; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; @@ -102,6 +103,9 @@ import org.mockito.stubbing.Answer; /** Tests for {@link RemoteSpawnRunner} in combination with {@link GrpcRemoteExecutor}. */ @RunWith(JUnit4.class) public class GrpcRemoteExecutionClientTest { + + private static final DigestUtil DIGEST_UTIL = new DigestUtil(HashFunction.SHA256); + private static final ArtifactExpander SIMPLE_ARTIFACT_EXPANDER = new ArtifactExpander() { @Override @@ -176,7 +180,6 @@ public class GrpcRemoteExecutionClientTest { @Before public final void setUp() throws Exception { - FileSystem.setDigestFunctionForTesting(HashFunction.SHA1); String fakeServerName = "fake server for " + getClass(); // Use a mutable service registry for later registering the service impl for each test case. fakeServer = @@ -187,7 +190,7 @@ public class GrpcRemoteExecutionClientTest { .start(); Chunker.setDefaultChunkSizeForTesting(1000); // Enough for everything to be one chunk. - fs = new InMemoryFileSystem(); + fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256); execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -236,7 +239,7 @@ public class GrpcRemoteExecutionClientTest { CallCredentials creds = GrpcUtils.newCallCredentials(Options.getDefaults(AuthAndTLSOptions.class)); GrpcRemoteCache remoteCache = - new GrpcRemoteCache(channel, creds, options, retrier); + new GrpcRemoteCache(channel, creds, options, retrier, DIGEST_UTIL); client = new RemoteSpawnRunner( execRoot, @@ -247,7 +250,8 @@ public class GrpcRemoteExecutionClientTest { "build-req-id", "command-id", remoteCache, - executor); + executor, + DIGEST_UTIL); inputDigest = fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz"); } @@ -277,8 +281,8 @@ public class GrpcRemoteExecutionClientTest { @Test public void cacheHitWithOutput() throws Exception { - final Digest stdOutDigest = Digests.computeDigestUtf8("stdout"); - final Digest stdErrDigest = Digests.computeDigestUtf8("stderr"); + final Digest stdOutDigest = DIGEST_UTIL.computeAsUtf8("stdout"); + final Digest stdErrDigest = DIGEST_UTIL.computeAsUtf8("stderr"); serviceRegistry.addService( new ActionCacheImplBase() { @Override @@ -326,7 +330,7 @@ public class GrpcRemoteExecutionClientTest { } private Answer> blobWriteAnswer(final byte[] data) { - final Digest digest = Digests.computeDigest(data); + final Digest digest = DIGEST_UTIL.compute(data); return new Answer>() { @Override public StreamObserver answer(InvocationOnMock invocation) { @@ -444,7 +448,7 @@ public class GrpcRemoteExecutionClientTest { .setValue("value") .build()) .build(); - final Digest cmdDigest = Digests.computeDigest(command); + final Digest cmdDigest = DIGEST_UTIL.compute(command); BindableService cas = new ContentAddressableStorageImplBase() { @Override @@ -633,7 +637,7 @@ public class GrpcRemoteExecutionClientTest { .setValue("value") .build()) .build(); - final Digest cmdDigest = Digests.computeDigest(command); + final Digest cmdDigest = DIGEST_UTIL.compute(command); serviceRegistry.addService( new ContentAddressableStorageImplBase() { private int numErrors = 4; @@ -738,7 +742,7 @@ public class GrpcRemoteExecutionClientTest { responseObserver.onError(Status.NOT_FOUND.asRuntimeException()); } }); - Digest stdOutDigest = Digests.computeDigestUtf8("bla"); + Digest stdOutDigest = DIGEST_UTIL.computeAsUtf8("bla"); final ActionResult actionResult = ActionResult.newBuilder().setStdoutDigest(stdOutDigest).build(); serviceRegistry.addService( @@ -788,7 +792,7 @@ public class GrpcRemoteExecutionClientTest { @Test public void remotelyReExecuteOrphanedCachedActions() throws Exception { - final Digest stdOutDigest = Digests.computeDigestUtf8("stdout"); + final Digest stdOutDigest = DIGEST_UTIL.computeAsUtf8("stdout"); final ActionResult actionResult = ActionResult.newBuilder().setStdoutDigest(stdOutDigest).build(); serviceRegistry.addService( @@ -858,7 +862,9 @@ public class GrpcRemoteExecutionClientTest { fail("Expected an exception"); } catch (ExecException expected) { assertThat(expected).hasMessageThat().contains("Missing digest"); - assertThat(expected).hasMessageThat().contains("476d9ec701e2de6a6c37ab5211117a7cb8333a27"); + assertThat(expected) + .hasMessageThat() + .contains(DIGEST_UTIL.computeAsUtf8("stdout").toString()); } } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 3295df658c..7ac5cfa190 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; +import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; @@ -42,10 +43,11 @@ import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; import com.google.devtools.build.lib.exec.util.FakeOwner; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -80,6 +82,7 @@ public class RemoteSpawnCacheTest { }; private FileSystem fs; + private DigestUtil digestUtil; private Path execRoot; private SimpleSpawn simpleSpawn; private FakeActionInputFileCache fakeFileCache; @@ -147,7 +150,8 @@ public class RemoteSpawnCacheTest { @Before public final void setUp() throws Exception { MockitoAnnotations.initMocks(this); - fs = new InMemoryFileSystem(); + fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256); + digestUtil = new DigestUtil(HashFunction.SHA256); execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -172,7 +176,14 @@ public class RemoteSpawnCacheTest { reporter.addHandler(eventHandler); cache = new RemoteSpawnCache( - execRoot, options, remoteCache, "build-req-id", "command-id", false, reporter); + execRoot, + options, + remoteCache, + "build-req-id", + "command-id", + false, + reporter, + digestUtil); fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz"); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index 5fec618cc8..31681906d8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.actions.SimpleSpawn; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.SpawnResult.Status; +import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Reporter; @@ -48,10 +49,11 @@ import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; import com.google.devtools.build.lib.exec.util.FakeOwner; -import com.google.devtools.build.lib.remote.Digests.ActionKey; +import com.google.devtools.build.lib.remote.DigestUtil.ActionKey; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -81,6 +83,7 @@ public class RemoteSpawnRunnerTest { ImmutableMap.of(ExecutionRequirements.NO_CACHE, ""); private Path execRoot; + private DigestUtil digestUtil; private FakeActionInputFileCache fakeFileCache; private FileOutErr outErr; @@ -96,8 +99,8 @@ public class RemoteSpawnRunnerTest { @Before public final void setUp() throws Exception { MockitoAnnotations.initMocks(this); - - FileSystem fs = new InMemoryFileSystem(); + digestUtil = new DigestUtil(HashFunction.SHA256); + FileSystem fs = new InMemoryFileSystem(new JavaClock(), HashFunction.SHA256); execRoot = fs.getPath("/exec/root"); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); @@ -130,7 +133,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ExecuteResponse succeeded = ExecuteResponse.newBuilder().setResult( ActionResult.newBuilder().setExitCode(0).build()).build(); @@ -186,7 +190,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null); + null, + digestUtil); // Throw an IOException to trigger the local fallback. when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(IOException.class); @@ -237,7 +242,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null)); + null, + digestUtil)); Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -284,7 +290,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null)); + null, + digestUtil)); try { runner.exec(spawn, policy); @@ -317,7 +324,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null); + null, + digestUtil); Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -367,7 +375,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null); + null, + digestUtil); Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -405,7 +414,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - null); + null, + digestUtil); Spawn spawn = newSimpleSpawn(); SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); @@ -440,7 +450,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException()); @@ -477,7 +488,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(cachedResult); @@ -517,7 +529,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); @@ -552,7 +565,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); @@ -585,7 +599,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); @@ -624,7 +639,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new IOException()); @@ -659,7 +675,8 @@ public class RemoteSpawnRunnerTest { "build-req-id", "command-id", cache, - executor); + executor, + digestUtil); when(cache.getCachedActionResult(any(ActionKey.class))).thenThrow(new IOException()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java b/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java index 94374f6364..9c0f43afc8 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java @@ -21,13 +21,14 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputFileCache; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; +import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.exec.SingleBuildFileCache; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; import com.google.devtools.build.lib.testutil.Scratch; -import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystem.HashFunction; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.remoteexecution.v1test.Digest; import com.google.devtools.remoteexecution.v1test.Directory; import java.util.ArrayList; @@ -42,13 +43,14 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class TreeNodeRepositoryTest { private Scratch scratch; + private DigestUtil digestUtil; private Root rootDir; private Path rootPath; @Before public final void setRootDir() throws Exception { - FileSystem.setDigestFunctionForTesting(HashFunction.SHA1); - scratch = new Scratch(); + digestUtil = new DigestUtil(HashFunction.SHA256); + scratch = new Scratch(new InMemoryFileSystem(BlazeClock.instance(), HashFunction.SHA256)); rootDir = Root.asDerivedRoot(scratch.dir("/exec/root")); rootPath = rootDir.getPath(); } @@ -56,7 +58,7 @@ public class TreeNodeRepositoryTest { private TreeNodeRepository createTestTreeNodeRepository() { ActionInputFileCache inputFileCache = new SingleBuildFileCache(rootPath.getPathString(), scratch.getFileSystem()); - return new TreeNodeRepository(rootPath, inputFileCache); + return new TreeNodeRepository(rootPath, inputFileCache, digestUtil); } @Test -- cgit v1.2.3