diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index f787fed3053646..b4cfa3dcadbf6d 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -95,6 +95,7 @@ filegroup( "//src/main/java/com/google/devtools/build/lib/standalone:srcs", "//src/main/java/com/google/devtools/build/lib/supplier:srcs", "//src/main/java/com/google/devtools/build/lib/testing/common:srcs", + "//src/main/java/com/google/devtools/build/lib/testing/vfs:srcs", "//src/main/java/com/google/devtools/build/lib/unix:srcs", "//src/main/java/com/google/devtools/build/lib/unsafe:srcs", "//src/main/java/com/google/devtools/build/lib/util:srcs", diff --git a/src/main/java/com/google/devtools/build/lib/testing/vfs/BUILD b/src/main/java/com/google/devtools/build/lib/testing/vfs/BUILD new file mode 100644 index 00000000000000..0da8ba03a9c7c7 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/testing/vfs/BUILD @@ -0,0 +1,24 @@ +package( + default_testonly = True, + default_visibility = ["//src:__subpackages__"], +) + +licenses(["notice"]) + +filegroup( + name = "srcs", + testonly = False, + srcs = glob(["**"]), + visibility = ["//src:__subpackages__"], +) + +java_library( + name = "spied_filesystem", + srcs = ["SpiedFileSystem.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", + "//third_party:mockito", + ], +) diff --git a/src/main/java/com/google/devtools/build/lib/testing/vfs/SpiedFileSystem.java b/src/main/java/com/google/devtools/build/lib/testing/vfs/SpiedFileSystem.java new file mode 100644 index 00000000000000..3a7d3030402822 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/testing/vfs/SpiedFileSystem.java @@ -0,0 +1,53 @@ +// Copyright 2022 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.testing.vfs; + +import static org.mockito.Mockito.spy; + +import com.google.devtools.build.lib.vfs.DelegateFileSystem; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import java.io.IOException; +import java.io.OutputStream; + +/** + * Delegate file system with the sole purpose of creating a {@link org.mockito.Mockito#spy}. + * + *

This class purposely makes the {@link FileSystem} methods public so that tests can mock those. + * Please feel free to add any methods you need. + */ +public class SpiedFileSystem extends DelegateFileSystem { + + private SpiedFileSystem(FileSystem delegateFs) { + super(delegateFs); + } + + /** + * Create a spied file system instance delegating all calls to the provided {@code fileSystem}. + */ + public static SpiedFileSystem createSpy(FileSystem fileSystem) { + return spy(new SpiedFileSystem(fileSystem)); + } + + public static SpiedFileSystem createInMemorySpy() { + return createSpy(new InMemoryFileSystem(DigestHashFunction.SHA256)); + } + + @Override + public OutputStream getOutputStream(PathFragment path, boolean append) throws IOException { + return super.getOutputStream(path, append); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD index ea4c668e6ddc9f..72937388d8b116 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD @@ -28,6 +28,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:thread_state_receiver", + "//src/main/java/com/google/devtools/build/lib/analysis:actions/deterministic_writer", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", @@ -41,6 +42,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy", "//src/main/java/com/google/devtools/build/lib/exec:bin_tools", "//src/main/java/com/google/devtools/build/lib/exec:execution_options", + "//src/main/java/com/google/devtools/build/lib/exec:file_write_strategy", "//src/main/java/com/google/devtools/build/lib/exec:module_action_context_registry", "//src/main/java/com/google/devtools/build/lib/exec:single_build_file_cache", "//src/main/java/com/google/devtools/build/lib/exec:spawn_cache", @@ -57,8 +59,11 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec:test_log_helper", "//src/main/java/com/google/devtools/build/lib/remote/options", "//src/main/java/com/google/devtools/build/lib/shell", + "//src/main/java/com/google/devtools/build/lib/testing/vfs:spied_filesystem", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", + "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", + "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/util/io:out-err", @@ -80,6 +85,7 @@ java_library( "//third_party:mockito", "//third_party:truth", "//third_party/protobuf:protobuf_java", + "@com_google_testparameterinjector//:testparameterinjector", ], ) diff --git a/src/test/java/com/google/devtools/build/lib/exec/FileWriteStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/FileWriteStrategyTest.java new file mode 100644 index 00000000000000..3b7c0a84652825 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/exec/FileWriteStrategyTest.java @@ -0,0 +1,156 @@ +// Copyright 2022 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.exec; + +import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.google.devtools.build.lib.actions.AbstractAction; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionExecutionException; +import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; +import com.google.devtools.build.lib.actions.EnvironmentalExecException; +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.util.ActionsTestUtil; +import com.google.devtools.build.lib.actions.util.ActionsTestUtil.NullAction; +import com.google.devtools.build.lib.actions.util.DummyExecutor; +import com.google.devtools.build.lib.analysis.actions.DeterministicWriter; +import com.google.devtools.build.lib.events.NullEventHandler; +import com.google.devtools.build.lib.server.FailureDetails.Execution.Code; +import com.google.devtools.build.lib.testing.vfs.SpiedFileSystem; +import com.google.devtools.build.lib.testutil.Scratch; +import com.google.devtools.build.lib.util.DetailedExitCode; +import com.google.devtools.build.lib.util.ExitCode; +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.testing.junit.testparameterinjector.TestParameter; +import com.google.testing.junit.testparameterinjector.TestParameterInjector; +import java.io.IOException; +import java.io.OutputStream; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Tests for {@link FileWriteStrategy}. */ +@RunWith(TestParameterInjector.class) +public final class FileWriteStrategyTest { + private static final IOException INJECTED_EXCEPTION = new IOException("oh no!"); + + private final FileWriteStrategy fileWriteStrategy = new FileWriteStrategy(); + + private final SpiedFileSystem fileSystem = SpiedFileSystem.createInMemorySpy(); + private final Scratch scratch = new Scratch(fileSystem); + private Path execRoot; + private ArtifactRoot outputRoot; + + @Before + public void createOutputRoot() throws IOException { + execRoot = scratch.dir("/execroot"); + outputRoot = ArtifactRoot.asDerivedRoot(execRoot, RootType.Output, "bazel-out"); + outputRoot.getRoot().asPath().createDirectory(); + } + + @Test + public void beginWriteOutputToFile_writesCorrectOutput( + @TestParameter({"", "hello", "hello there"}) String content) throws Exception { + AbstractAction action = createAction("file"); + + var spawnContinuation = + fileWriteStrategy.beginWriteOutputToFile( + action, + createActionExecutionContext(), + out -> out.write(content.getBytes(UTF_8)), + /*makeExecutable=*/ false, + /*isRemotable=*/ false); + + assertThat(spawnContinuation.isDone()).isTrue(); + assertThat(spawnContinuation.get()).isEmpty(); + assertThat(FileSystemUtils.readContent(action.getPrimaryOutput().getPath(), UTF_8)) + .isEqualTo(content); + } + + private enum FailureMode implements DeterministicWriter { + OPEN_FAILURE { + @Override + void setupFileSystem(SpiedFileSystem fileSystem, PathFragment outputPath) throws IOException { + when(fileSystem.getOutputStream(outputPath, /*append=*/ false)) + .thenThrow(INJECTED_EXCEPTION); + } + }, + WRITE_FAILURE { + @Override + public void writeOutputFile(OutputStream out) throws IOException { + throw INJECTED_EXCEPTION; + } + }, + CLOSE_FAILURE { + @Override + void setupFileSystem(SpiedFileSystem fileSystem, PathFragment outputPath) throws IOException { + OutputStream outputStream = mock(OutputStream.class); + doThrow(INJECTED_EXCEPTION).when(outputStream).close(); + when(fileSystem.getOutputStream(outputPath, /*append=*/ false)).thenReturn(outputStream); + } + }; + + void setupFileSystem(SpiedFileSystem fileSystem, PathFragment outputPath) throws IOException {} + + @Override + public void writeOutputFile(OutputStream out) throws IOException {} + } + + @Test + public void beginWriteOutputToFile_errorInWriter_returnsContinuationWithFailure( + @TestParameter FailureMode failureMode) throws Exception { + AbstractAction action = createAction("file"); + failureMode.setupFileSystem(fileSystem, action.getPrimaryOutput().getPath().asFragment()); + + var spawnContinuation = + fileWriteStrategy.beginWriteOutputToFile( + action, + createActionExecutionContext(), + failureMode, + /*makeExecutable=*/ false, + /*isRemotable=*/ false); + + assertThat(spawnContinuation.isDone()).isFalse(); + ExecException e = assertThrows(EnvironmentalExecException.class, spawnContinuation::execute); + assertThat(e).hasCauseThat().isSameInstanceAs(INJECTED_EXCEPTION); + var detailExitCode = getDetailExitCode(e); + assertThat(detailExitCode.getExitCode()).isEqualTo(ExitCode.LOCAL_ENVIRONMENTAL_ERROR); + assertThat(detailExitCode.getFailureDetail().getExecution().getCode()) + .isEqualTo(Code.FILE_WRITE_IO_EXCEPTION); + } + + private DetailedExitCode getDetailExitCode(ExecException e) { + return ActionExecutionException.fromExecException(e, new ActionsTestUtil.NullAction()) + .getDetailedExitCode(); + } + + private ActionExecutionContext createActionExecutionContext() { + return ActionsTestUtil.createContext( + new DummyExecutor(fileSystem, execRoot), NullEventHandler.INSTANCE); + } + + private AbstractAction createAction(String outputRelativePath) { + return new NullAction( + ActionsTestUtil.createArtifactWithRootRelativePath( + outputRoot, PathFragment.create(outputRelativePath))); + } +}