From 68aa410229cb36ceedc910c803a0aff2db6d027f Mon Sep 17 00:00:00 2001 From: ulfjack Date: Fri, 15 Jun 2018 01:40:02 -0700 Subject: Add a mechanism for build event protocol events to upload files This should be a no-op, mostly replacing PathConverter with BuildEventArtifactUploader, since none of the implementations perform any upload yet. PiperOrigin-RevId: 200685325 --- src/test/java/com/google/devtools/build/lib/BUILD | 2 + .../BazelBuildEventServiceModuleTest.java | 16 +++----- .../devtools/build/lib/buildeventstream/BUILD | 25 +++++++++++++ .../lib/buildeventstream/PathConverterTest.java | 39 ++++++++++++++++++++ .../transports/BinaryFormatFileTransportTest.java | 16 ++++---- .../transports/BuildEventTransportFactoryTest.java | 43 ++++++++++------------ .../transports/JsonFormatFileTransportTest.java | 4 +- .../transports/TextFormatFileTransportTest.java | 4 +- .../build/lib/remote/CasPathConverterTest.java | 6 +-- 9 files changed, 109 insertions(+), 46 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/buildeventstream/BUILD create mode 100644 src/test/java/com/google/devtools/build/lib/buildeventstream/PathConverterTest.java (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 4d2f69b321..769ac2595e 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -43,6 +43,7 @@ filegroup( "//src/test/java/com/google/devtools/build/lib/bazel/repository/downloader:srcs", "//src/test/java/com/google/devtools/build/lib/bazel/repository:srcs", "//src/test/java/com/google/devtools/build/lib/buildeventservice:srcs", + "//src/test/java/com/google/devtools/build/lib/buildeventstream:srcs", "//src/test/java/com/google/devtools/build/lib/buildeventstream/transports:srcs", "//src/test/java/com/google/devtools/build/lib/buildtool:srcs", "//src/test/java/com/google/devtools/build/lib/profiler/callcounts:srcs", @@ -1248,6 +1249,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:util", "//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/buildeventstream", "//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/remote/blobstore", diff --git a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java index 9811f41d95..37ac8f6c95 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModuleTest.java @@ -26,8 +26,9 @@ import com.google.devtools.build.lib.actions.ActionExecutedEvent; import com.google.devtools.build.lib.actions.ActionExecutedEvent.ErrorTiming; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; +import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader; +import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap; import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions; -import com.google.devtools.build.lib.buildeventstream.PathConverter; import com.google.devtools.build.lib.buildeventstream.transports.BinaryFormatFileTransport; import com.google.devtools.build.lib.buildeventstream.transports.BuildEventStreamOptions; import com.google.devtools.build.lib.buildeventstream.transports.JsonFormatFileTransport; @@ -38,7 +39,6 @@ import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.runtime.BlazeModule.ModuleEnvironment; import com.google.devtools.build.lib.runtime.BuildEventStreamer; import com.google.devtools.build.lib.runtime.Command; -import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParser; @@ -74,14 +74,6 @@ public class BazelBuildEventServiceModuleTest { } }; - private static final PathConverter PATH_CONVERTER = - new PathConverter() { - @Override - public String apply(Path path) { - return path.getPathString(); - } - }; - private Reporter reporter; private BuildEventServiceOptions besOptions; @@ -146,7 +138,9 @@ public class BazelBuildEventServiceModuleTest { commandLineReporter, moduleEnvironment, clock, - PATH_CONVERTER, + new BuildEventArtifactUploaderMap.Builder() + .add("", BuildEventArtifactUploader.LOCAL_FILES_UPLOADER) + .build(), reporter, /* buildRequestId= */ "foo", /* invocationId= */ "bar", diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/BUILD b/src/test/java/com/google/devtools/build/lib/buildeventstream/BUILD new file mode 100644 index 0000000000..586bdff2c0 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/BUILD @@ -0,0 +1,25 @@ +package( + default_testonly = 1, + default_visibility = ["//src:__subpackages__"], +) + +filegroup( + name = "srcs", + testonly = 0, + srcs = glob(["**"]), + visibility = ["//src/test/java/com/google/devtools/build/lib:__pkg__"], +) + +java_test( + name = "BuildEventStreamTest", + srcs = glob(["*.java"]), + test_class = "com.google.devtools.build.lib.AllTests", + runtime_deps = ["//src/test/java/com/google/devtools/build/lib:test_runner"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/buildeventstream", + "//third_party:guava", + "//third_party:junit4", + "//third_party:mockito", + "//third_party:truth", + ], +) diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/PathConverterTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/PathConverterTest.java new file mode 100644 index 0000000000..3601e3746a --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/PathConverterTest.java @@ -0,0 +1,39 @@ +// Copyright 2018 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.buildeventstream; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.lib.buildeventstream.PathConverter.FileUriPathConverter; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link PathConverter}. */ +@RunWith(JUnit4.class) +public class PathConverterTest { + + @Test + public void testPathToUriString() { + assertThat(FileUriPathConverter.pathToUriString("/foo/bar")) + .isEqualTo("file:///foo/bar"); + } + + @Test + public void testWIndowsPathWithWhitespace() { + // See https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/ + assertThat(FileUriPathConverter.pathToUriString("C:/Temp/Foo Bar.txt")) + .isEqualTo("file:///C:/Temp/Foo%20Bar.txt"); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java index 51951e4f8a..1b85701a9b 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BinaryFormatFileTransportTest.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.buildeventstream.transports; import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.LOCAL_FILES_UPLOADER; import static org.mockito.Mockito.when; import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer; @@ -25,7 +26,6 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildStarted; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.Progress; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.TargetComplete; -import com.google.devtools.build.lib.buildeventstream.PathConverter; import com.google.devtools.common.options.Options; import java.io.File; import java.io.FileInputStream; @@ -43,7 +43,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -/** Tests {@link BinaryFormatFileTransport}. **/ +/** Tests {@link BinaryFormatFileTransport}. */ @RunWith(JUnit4.class) public class BinaryFormatFileTransportTest { private final BuildEventProtocolOptions defaultOpts = @@ -53,7 +53,6 @@ public class BinaryFormatFileTransportTest { @Mock public BuildEvent buildEvent; - @Mock public PathConverter pathConverter; @Mock public ArtifactGroupNamer artifactGroupNamer; @Before @@ -76,7 +75,8 @@ public class BinaryFormatFileTransportTest { .build(); when(buildEvent.asStreamProto(Matchers.any())).thenReturn(started); BinaryFormatFileTransport transport = - new BinaryFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter); + new BinaryFormatFileTransport( + output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {}); transport.sendBuildEvent(buildEvent, artifactGroupNamer); BuildEventStreamProtos.BuildEvent progress = @@ -113,7 +113,7 @@ public class BinaryFormatFileTransportTest { .build(); when(buildEvent.asStreamProto(Matchers.any())).thenReturn(started); BinaryFormatFileTransport transport = - new BinaryFormatFileTransport(path, defaultOpts, pathConverter); + new BinaryFormatFileTransport(path, defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {}); transport.sendBuildEvent(buildEvent, artifactGroupNamer); transport.close().get(); @@ -134,7 +134,8 @@ public class BinaryFormatFileTransportTest { when(buildEvent.asStreamProto(Matchers.any())).thenReturn(started); BinaryFormatFileTransport transport = - new BinaryFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter); + new BinaryFormatFileTransport( + output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {}); // Close the stream. transport.out.close(); @@ -161,7 +162,8 @@ public class BinaryFormatFileTransportTest { when(buildEvent.asStreamProto(Matchers.any())).thenReturn(started); BinaryFormatFileTransport transport = - new BinaryFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter); + new BinaryFormatFileTransport( + output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {}); transport.sendBuildEvent(buildEvent, artifactGroupNamer); Future closeFuture = transport.close(); diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java index e5d5133cca..5494684b3f 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/BuildEventTransportFactoryTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer; import com.google.devtools.build.lib.buildeventstream.BuildEvent; +import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploaderMap; import com.google.devtools.build.lib.buildeventstream.BuildEventContext; import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions; import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; @@ -31,7 +32,7 @@ import com.google.devtools.build.lib.buildeventstream.PathConverter; import com.google.devtools.common.options.Options; import java.io.File; import java.io.IOException; -import java.util.concurrent.Future; +import java.util.concurrent.ExecutionException; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -44,7 +45,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -/** Tests {@link BuildEventTransportFactory}. **/ +/** Tests {@link BuildEventTransportFactory}. */ @RunWith(JUnit4.class) public class BuildEventTransportFactoryTest { @@ -85,14 +86,18 @@ public class BuildEventTransportFactoryTest { Mockito.validateMockitoUsage(); } + private BuildEventArtifactUploaderMap localFilesOnly() { + return new BuildEventArtifactUploaderMap.Builder().build(); + } + @Test - public void testCreatesTextFormatFileTransport() throws IOException { + public void testCreatesTextFormatFileTransport() throws Exception { File textFile = tmp.newFile(); when(options.getBuildEventTextFile()).thenReturn(textFile.getAbsolutePath()); - when(options.getBuildEventTextFilePathConversion()).thenReturn(true); when(options.getBuildEventBinaryFile()).thenReturn(""); ImmutableSet transports = - BuildEventTransportFactory.createFromOptions(options, protocolOpts, pathConverter); + BuildEventTransportFactory.createFromOptions( + options, protocolOpts, localFilesOnly(), (e) -> {}); assertThat(FluentIterable.from(transports).transform(GET_CLASS)) .containsExactly(TextFormatFileTransport.class); sendEventsAndClose(buildEvent, transports); @@ -100,13 +105,13 @@ public class BuildEventTransportFactoryTest { } @Test - public void testCreatesBinaryFormatFileTransport() throws IOException { + public void testCreatesBinaryFormatFileTransport() throws Exception { File binaryFile = tmp.newFile(); when(options.getBuildEventTextFile()).thenReturn(""); when(options.getBuildEventBinaryFile()).thenReturn(binaryFile.getAbsolutePath()); - when(options.getBuildEventBinaryFilePathConversion()).thenReturn(true); ImmutableSet transports = - BuildEventTransportFactory.createFromOptions(options, protocolOpts, pathConverter); + BuildEventTransportFactory.createFromOptions( + options, protocolOpts, localFilesOnly(), (e) -> {}); assertThat(FluentIterable.from(transports).transform(GET_CLASS)) .containsExactly(BinaryFormatFileTransport.class); sendEventsAndClose(buildEvent, transports); @@ -114,15 +119,14 @@ public class BuildEventTransportFactoryTest { } @Test - public void testCreatesAllTransports() throws IOException { + public void testCreatesAllTransports() throws Exception { File textFile = tmp.newFile(); File binaryFile = tmp.newFile(); when(options.getBuildEventTextFile()).thenReturn(textFile.getAbsolutePath()); when(options.getBuildEventBinaryFile()).thenReturn(binaryFile.getAbsolutePath()); - when(options.getBuildEventBinaryFilePathConversion()).thenReturn(true); - when(options.getBuildEventTextFilePathConversion()).thenReturn(true); ImmutableSet transports = - BuildEventTransportFactory.createFromOptions(options, protocolOpts, pathConverter); + BuildEventTransportFactory.createFromOptions( + options, protocolOpts, localFilesOnly(), (e) -> {}); assertThat(FluentIterable.from(transports).transform(GET_CLASS)) .containsExactly(TextFormatFileTransport.class, BinaryFormatFileTransport.class); sendEventsAndClose(buildEvent, transports); @@ -134,23 +138,16 @@ public class BuildEventTransportFactoryTest { public void testCreatesNoTransports() throws IOException { when(options.getBuildEventTextFile()).thenReturn(""); ImmutableSet transports = - BuildEventTransportFactory.createFromOptions(options, protocolOpts, pathConverter); + BuildEventTransportFactory.createFromOptions( + options, protocolOpts, localFilesOnly(), (e) -> {}); assertThat(transports).isEmpty(); } - @Test - public void testPathToUriString() { - // See https://blogs.msdn.microsoft.com/ie/2006/12/06/file-uris-in-windows/ - assertThat(BuildEventTransportFactory.pathToUriString("C:/Temp/Foo Bar.txt")) - .isEqualTo("file:///C:/Temp/Foo%20Bar.txt"); - } - private void sendEventsAndClose(BuildEvent event, Iterable transports) - throws IOException{ + throws InterruptedException, ExecutionException { for (BuildEventTransport transport : transports) { transport.sendBuildEvent(event, artifactGroupNamer); - @SuppressWarnings({"unused", "nullness"}) - Future possiblyIgnoredError = transport.close(); + transport.close().get(); } } } diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java index e667a7d787..656f87c289 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/JsonFormatFileTransportTest.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.buildeventstream.transports; import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.LOCAL_FILES_UPLOADER; import static org.mockito.Mockito.when; import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer; @@ -76,7 +77,8 @@ public class JsonFormatFileTransportTest { .build(); when(buildEvent.asStreamProto(Matchers.any())).thenReturn(started); JsonFormatFileTransport transport = - new JsonFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter); + new JsonFormatFileTransport( + output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {}); transport.sendBuildEvent(buildEvent, artifactGroupNamer); transport.close().get(); diff --git a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java index c3ad19d0d6..df84539c3a 100644 --- a/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildeventstream/transports/TextFormatFileTransportTest.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.buildeventstream.transports; import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.LOCAL_FILES_UPLOADER; import static org.mockito.Mockito.when; import com.google.common.base.Joiner; @@ -77,7 +78,8 @@ public class TextFormatFileTransportTest { .build(); when(buildEvent.asStreamProto(Matchers.any())).thenReturn(started); TextFormatFileTransport transport = - new TextFormatFileTransport(output.getAbsolutePath(), defaultOpts, pathConverter); + new TextFormatFileTransport( + output.getAbsolutePath(), defaultOpts, LOCAL_FILES_UPLOADER, (e) -> {}); transport.sendBuildEvent(buildEvent, artifactGroupNamer); BuildEventStreamProtos.BuildEvent progress = 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 885ae95698..21a5bb39a2 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 @@ -37,20 +37,20 @@ public class CasPathConverterTest { @Test public void noOptionsShouldntCrash() { converter.digestUtil = new DigestUtil(HashFunction.SHA256); - assertThat(converter.apply(fs.getPath("/foo"))).isNull(); + assertThat(converter.apply(fs.getPath("/foo"))).isEqualTo("file:///foo"); } @Test public void noDigestUtilShouldntCrash() { converter.options = Options.getDefaults(RemoteOptions.class); - assertThat(converter.apply(fs.getPath("/foo"))).isNull(); + assertThat(converter.apply(fs.getPath("/foo"))).isEqualTo("file:///foo"); } @Test public void disabledRemote() { converter.options = Options.getDefaults(RemoteOptions.class); converter.digestUtil = new DigestUtil(HashFunction.SHA256); - assertThat(converter.apply(fs.getPath("/foo"))).isNull(); + assertThat(converter.apply(fs.getPath("/foo"))).isEqualTo("file:///foo"); } @Test -- cgit v1.2.3