Merge FilesystemValueCheckerParameterizedTest and FilesystemValueCheckerTestBase into FileSystemValueCheckerTest.
FileSystemValueCheckerTest now has TestParameterInjector to do the parameterizing work. Also there is no need to keep FilesystemValueCheckerTestBase since FileSystemValueCheckerTest is the only class extending from it.
PiperOrigin-RevId: 418790548
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerParameterizedTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerParameterizedTest.java
deleted file mode 100644
index 99381b3..0000000
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerParameterizedTest.java
+++ /dev/null
@@ -1,221 +0,0 @@
-// Copyright 2020 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.skyframe;
-
-import static com.google.common.collect.ImmutableList.toImmutableList;
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.vfs.ModifiedFileSet;
-import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
-import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collection;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
-
-/** {@link FilesystemValueChecker} checker parameterized on {@link BatchStatMode}. */
-// TODO(b/207545453): Merge with FilesystemValueCheckerTest{,Base}.
-@RunWith(Parameterized.class)
-public final class FilesystemValueCheckerParameterizedTest extends FilesystemValueCheckerTestBase {
-
- private static final ActionLookupData ACTION_LOOKUP_DATA = actionLookupData(0);
-
- @Parameter public BatchStatMode batchStat;
-
- @Parameters(name = "batchStat={0}")
- public static ImmutableList<Object[]> batchStatModes() {
- return Arrays.stream(BatchStatMode.values())
- .map(mode -> new BatchStatMode[] {mode})
- .collect(toImmutableList());
- }
-
- @Test
- public void getDirtyActionValues_unchangedEmptyTreeArtifactWithArchivedFile_noDirtyKeys()
- throws Exception {
- SpecialArtifact treeArtifact = createTreeArtifact("dir");
- treeArtifact.getPath().createDirectoryAndParents();
- ActionExecutionValue actionExecutionValue =
- actionValueWithTreeArtifacts(
- ImmutableList.of(),
- ImmutableList.of(createArchivedTreeArtifactWithContent(treeArtifact)));
-
- assertThat(getDirtyActionValues(actionExecutionValue)).isEmpty();
- }
-
- @Test
- public void getDirtyActionValues_unchangedTreeArtifactWithArchivedFile_noDirtyKeys()
- throws Exception {
- SpecialArtifact treeArtifact = createTreeArtifact("dir");
- ActionExecutionValue actionExecutionValue =
- actionValueWithTreeArtifacts(
- ImmutableList.of(
- createTreeFileArtifactWithContent(treeArtifact, "file1", "content"),
- createTreeFileArtifactWithContent(treeArtifact, "file2", "content2")),
- ImmutableList.of(createArchivedTreeArtifactWithContent(treeArtifact)));
-
- assertThat(getDirtyActionValues(actionExecutionValue)).isEmpty();
- }
-
- @Test
- public void getDirtyActionValues_editedArchivedFileForEmptyTreeArtifact_reportsChange()
- throws Exception {
- SpecialArtifact treeArtifact = createTreeArtifact("dir");
- treeArtifact.getPath().createDirectoryAndParents();
- ArchivedTreeArtifact archivedTreeArtifact =
- createArchivedTreeArtifactWithContent(treeArtifact, "old content");
- ActionExecutionValue actionExecutionValue =
- actionValueWithTreeArtifacts(ImmutableList.of(), ImmutableList.of(archivedTreeArtifact));
-
- writeFile(archivedTreeArtifact.getPath(), "new content");
- assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
- }
-
- @Test
- public void getDirtyActionValues_editedArchivedFileForTreeArtifact_reportsChange()
- throws Exception {
- SpecialArtifact treeArtifact = createTreeArtifact("dir");
- ArchivedTreeArtifact archivedTreeArtifact =
- createArchivedTreeArtifactWithContent(treeArtifact, "old content");
- ActionExecutionValue actionExecutionValue =
- actionValueWithTreeArtifacts(
- ImmutableList.of(
- createTreeFileArtifactWithContent(
- treeArtifact, /*parentRelativePath=*/ "file1", "content"),
- createTreeFileArtifactWithContent(
- treeArtifact, /*parentRelativePath=*/ "file2", "content2")),
- ImmutableList.of(archivedTreeArtifact));
-
- writeFile(archivedTreeArtifact.getPath(), "new content");
- assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
- }
-
- @Test
- public void getDirtyActionValues_deletedArchivedFileForTreeArtifact_reportsChange()
- throws Exception {
- SpecialArtifact treeArtifact = createTreeArtifact("dir");
- ArchivedTreeArtifact archivedTreeArtifact = createArchivedTreeArtifactWithContent(treeArtifact);
- ActionExecutionValue actionExecutionValue =
- actionValueWithTreeArtifacts(
- ImmutableList.of(
- createTreeFileArtifactWithContent(
- treeArtifact, /*parentRelativePath=*/ "file1", "content"),
- createTreeFileArtifactWithContent(
- treeArtifact, /*parentRelativePath=*/ "file2", "content2")),
- ImmutableList.of(archivedTreeArtifact));
-
- archivedTreeArtifact.getPath().delete();
- assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
- }
-
- @Test
- public void getDirtyActionValues_deletedArchivedFileForEmptyTreeArtifact_reportsChange()
- throws Exception {
- SpecialArtifact treeArtifact = createTreeArtifact("dir");
- ArchivedTreeArtifact archivedTreeArtifact = createArchivedTreeArtifactWithContent(treeArtifact);
- ActionExecutionValue actionExecutionValue =
- actionValueWithTreeArtifacts(ImmutableList.of(), ImmutableList.of(archivedTreeArtifact));
-
- archivedTreeArtifact.getPath().delete();
- assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
- }
-
- @Test
- public void getDirtyActionValues_editedFileForTreeArtifactWithArchivedFile_reportsChange()
- throws Exception {
- SpecialArtifact treeArtifact = createTreeArtifact("dir");
- TreeFileArtifact child1 =
- createTreeFileArtifactWithContent(
- treeArtifact, /*parentRelativePath=*/ "file1", "old content");
- ActionExecutionValue actionExecutionValue =
- actionValueWithTreeArtifacts(
- ImmutableList.of(
- child1,
- createTreeFileArtifactWithContent(
- treeArtifact, /*parentRelativePath=*/ "file2", "content2")),
- ImmutableList.of(createArchivedTreeArtifactWithContent(treeArtifact)));
-
- writeFile(child1.getPath(), "new content");
- assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
- }
-
- @Test
- public void getDirtyActionValues_treeArtifactWithArchivedArtifact_reportsOnlyChangedKey()
- throws Exception {
- SpecialArtifact unchangedTreeArtifact = createTreeArtifact("dir1");
- ActionExecutionValue unchangedValue =
- actionValueWithTreeArtifacts(
- ImmutableList.of(createTreeFileArtifactWithContent(unchangedTreeArtifact, "child")),
- ImmutableList.of(createArchivedTreeArtifactWithContent(unchangedTreeArtifact)));
- SpecialArtifact changedTreeArtifact = createTreeArtifact("dir2");
- ArchivedTreeArtifact changedArchivedTreeArtifact =
- createArchivedTreeArtifactWithContent(changedTreeArtifact, "old content");
- ActionExecutionValue changedValue =
- actionValueWithTreeArtifacts(
- ImmutableList.of(
- createTreeFileArtifactWithContent(changedTreeArtifact, "file", "content")),
- ImmutableList.of(changedArchivedTreeArtifact));
-
- writeFile(changedArchivedTreeArtifact.getPath(), "new content");
- assertThat(
- getDirtyActionValues(
- ImmutableMap.of(
- actionLookupData(0), unchangedValue, actionLookupData(1), changedValue)))
- .containsExactly(actionLookupData(1));
- }
-
- private Collection<SkyKey> getDirtyActionValues(ActionExecutionValue actionExecutionValue)
- throws InterruptedException {
- return getDirtyActionValues(ImmutableMap.of(ACTION_LOOKUP_DATA, actionExecutionValue));
- }
-
- private Collection<SkyKey> getDirtyActionValues(ImmutableMap<SkyKey, SkyValue> valuesMap)
- throws InterruptedException {
- return new FilesystemValueChecker(
- /*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
- .getDirtyActionValues(
- valuesMap,
- batchStat.getBatchStat(fs),
- ModifiedFileSet.EVERYTHING_MODIFIED,
- /*trustRemoteArtifacts=*/ false);
- }
-
- private TreeFileArtifact createTreeFileArtifactWithContent(
- SpecialArtifact treeArtifact, String parentRelativePath, String... contentLines)
- throws IOException {
- TreeFileArtifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, parentRelativePath);
- writeFile(artifact.getPath(), contentLines);
- return artifact;
- }
-
- private ArchivedTreeArtifact createArchivedTreeArtifactWithContent(
- SpecialArtifact treeArtifact, String... contentLines) throws IOException {
- ArchivedTreeArtifact artifact = ArchivedTreeArtifact.createForTree(treeArtifact);
- writeFile(artifact.getPath(), contentLines);
- return artifact;
- }
-
- private static ActionLookupData actionLookupData(int actionIndex) {
- return ActionLookupData.create(ACTION_LOOKUP_KEY, actionIndex);
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
index 05d81af..5694ebb 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static java.util.concurrent.TimeUnit.SECONDS;
@@ -24,7 +25,9 @@
import com.google.common.util.concurrent.Runnables;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.ActionLookupData;
+import com.google.devtools.build.lib.actions.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
@@ -39,6 +42,7 @@
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ServerDirectories;
import com.google.devtools.build.lib.clock.BlazeClock;
+import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.NullEventHandler;
@@ -49,6 +53,7 @@
import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.BasicFilesystemDirtinessChecker;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
+import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.testutil.TestPackageFactoryBuilderFactory;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -61,6 +66,7 @@
import com.google.devtools.build.lib.vfs.FileStatus;
import com.google.devtools.build.lib.vfs.FileStatusWithDigest;
import com.google.devtools.build.lib.vfs.FileStatusWithDigestAdapter;
+import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.ModifiedFileSet;
import com.google.devtools.build.lib.vfs.Path;
@@ -69,6 +75,7 @@
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.UnixGlob;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
import com.google.devtools.build.skyframe.Differencer.Diff;
import com.google.devtools.build.skyframe.EvaluationContext;
import com.google.devtools.build.skyframe.EvaluationResult;
@@ -101,7 +108,28 @@
/** Tests for {@link FilesystemValueChecker}. */
@RunWith(TestParameterInjector.class)
-public final class FilesystemValueCheckerTest extends FilesystemValueCheckerTestBase {
+public final class FilesystemValueCheckerTest {
+ private static final int FSVC_THREADS_FOR_TEST = 200;
+ private static final ActionLookupKey ACTION_LOOKUP_KEY =
+ new ActionLookupKey() {
+ @Override
+ public SkyFunctionName functionName() {
+ return SkyFunctionName.FOR_TESTING;
+ }
+
+ @Nullable
+ @Override
+ public Label getLabel() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public BuildConfigurationKey getConfigurationKey() {
+ return null;
+ }
+ };
+ private static final ActionLookupData ACTION_LOOKUP_DATA = actionLookupData(0);
private static final EvaluationContext EVALUATION_OPTIONS =
EvaluationContext.newBuilder()
.setKeepGoing(false)
@@ -109,10 +137,128 @@
.setEventHandler(NullEventHandler.INSTANCE)
.build();
+ private final MockFileSystem fs = new MockFileSystem();
private RecordingDifferencer differencer;
private MemoizingEvaluator evaluator;
private SequentialBuildDriver driver;
private Path pkgRoot;
+ @TestParameter private BatchStatMode batchStat;
+
+ private SpecialArtifact createTreeArtifact(String relPath) throws IOException {
+ String outSegment = "bin";
+ Path outputDir = fs.getPath("/" + outSegment);
+ Path outputPath = outputDir.getRelative(relPath);
+ outputDir.createDirectory();
+ ArtifactRoot derivedRoot =
+ ArtifactRoot.asDerivedRoot(fs.getPath("/"), RootType.Output, outSegment);
+ return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
+ derivedRoot,
+ derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)));
+ }
+
+ private static ActionExecutionValue actionValueWithTreeArtifacts(List<TreeFileArtifact> contents)
+ throws IOException {
+ return actionValueWithTreeArtifacts(contents, ImmutableList.of());
+ }
+
+ private static ActionExecutionValue actionValueWithTreeArtifacts(
+ Iterable<TreeFileArtifact> contents, Iterable<ArchivedTreeArtifact> archivedTreeArtifacts)
+ throws IOException {
+ TreeArtifactValue.MultiBuilder treeArtifacts = TreeArtifactValue.newMultiBuilder();
+
+ for (TreeFileArtifact output : contents) {
+ treeArtifacts.putChild(output, createMetadataFromFileSystem(output));
+ }
+
+ for (ArchivedTreeArtifact archivedTreeArtifact : archivedTreeArtifacts) {
+ treeArtifacts.setArchivedRepresentation(
+ archivedTreeArtifact, createMetadataFromFileSystem(archivedTreeArtifact));
+ }
+
+ Map<Artifact, TreeArtifactValue> treeArtifactData = new HashMap<>();
+ treeArtifacts.injectTo(treeArtifactData::put);
+
+ return ActionExecutionValue.createForTesting(
+ /*artifactData=*/ ImmutableMap.of(),
+ ImmutableMap.copyOf(treeArtifactData),
+ /*outputSymlinks=*/ null);
+ }
+
+ private static FileArtifactValue createMetadataFromFileSystem(Artifact artifact)
+ throws IOException {
+ Path path = artifact.getPath();
+ FileArtifactValue noDigest =
+ ActionMetadataHandler.fileArtifactValueFromArtifact(
+ artifact, FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW)), null);
+ return FileArtifactValue.createFromInjectedDigest(noDigest, path.getDigest());
+ }
+
+ void writeFile(Path path, String... lines) throws IOException {
+ // Make sure we advance the clock to detect modifications which do not change the size, which
+ // rely on ctime.
+ fs.advanceClockMillis(1);
+ FileSystemUtils.writeIsoLatin1(path, lines);
+ }
+
+ private static final class MockFileSystem extends InMemoryFileSystem {
+ boolean statThrowsRuntimeException;
+ boolean readlinkThrowsIoException;
+
+ MockFileSystem() {
+ this(new ManualClock());
+ }
+
+ private MockFileSystem(ManualClock clock) {
+ super(clock, DigestHashFunction.SHA256);
+ }
+
+ @Override
+ public FileStatus statIfFound(PathFragment path, boolean followSymlinks) throws IOException {
+ if (statThrowsRuntimeException) {
+ throw new RuntimeException("bork");
+ }
+ return super.statIfFound(path, followSymlinks);
+ }
+
+ @Override
+ protected PathFragment readSymbolicLink(PathFragment path) throws IOException {
+ if (readlinkThrowsIoException) {
+ throw new IOException("readlink failed");
+ }
+ return super.readSymbolicLink(path);
+ }
+
+ void advanceClockMillis(int millis) {
+ ((ManualClock) clock).advanceMillis(millis);
+ }
+ }
+
+ private enum BatchStatMode {
+ DISABLED {
+ @Nullable
+ @Override
+ BatchStat getBatchStat(FileSystem fileSystem) {
+ return null;
+ }
+ },
+ ENABLED {
+ @Override
+ BatchStat getBatchStat(FileSystem fileSystem) {
+ return (useDigest, includeLinks, paths) -> {
+ List<FileStatusWithDigest> stats = new ArrayList<>();
+ for (PathFragment pathFrag : paths) {
+ stats.add(
+ FileStatusWithDigestAdapter.adapt(
+ fileSystem.getPath("/").getRelative(pathFrag).statIfFound(Symlinks.NOFOLLOW)));
+ }
+ return stats;
+ };
+ }
+ };
+
+ @Nullable
+ abstract BatchStat getBatchStat(FileSystem fileSystem);
+ }
@Before
public void setUp() throws Exception {
@@ -187,6 +333,181 @@
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
}
+ public static ImmutableList<Object[]> batchStatModes() {
+ return Arrays.stream(BatchStatMode.values())
+ .map(mode -> new BatchStatMode[] {mode})
+ .collect(toImmutableList());
+ }
+
+ @Test
+ public void getDirtyActionValues_unchangedEmptyTreeArtifactWithArchivedFile_noDirtyKeys()
+ throws Exception {
+ SpecialArtifact treeArtifact = createTreeArtifact("dir");
+ treeArtifact.getPath().createDirectoryAndParents();
+ ActionExecutionValue actionExecutionValue =
+ actionValueWithTreeArtifacts(
+ ImmutableList.of(),
+ ImmutableList.of(createArchivedTreeArtifactWithContent(treeArtifact)));
+
+ assertThat(getDirtyActionValues(actionExecutionValue)).isEmpty();
+ }
+
+ @Test
+ public void getDirtyActionValues_unchangedTreeArtifactWithArchivedFile_noDirtyKeys()
+ throws Exception {
+ SpecialArtifact treeArtifact = createTreeArtifact("dir");
+ ActionExecutionValue actionExecutionValue =
+ actionValueWithTreeArtifacts(
+ ImmutableList.of(
+ createTreeFileArtifactWithContent(treeArtifact, "file1", "content"),
+ createTreeFileArtifactWithContent(treeArtifact, "file2", "content2")),
+ ImmutableList.of(createArchivedTreeArtifactWithContent(treeArtifact)));
+
+ assertThat(getDirtyActionValues(actionExecutionValue)).isEmpty();
+ }
+
+ @Test
+ public void getDirtyActionValues_editedArchivedFileForEmptyTreeArtifact_reportsChange()
+ throws Exception {
+ SpecialArtifact treeArtifact = createTreeArtifact("dir");
+ treeArtifact.getPath().createDirectoryAndParents();
+ ArchivedTreeArtifact archivedTreeArtifact =
+ createArchivedTreeArtifactWithContent(treeArtifact, "old content");
+ ActionExecutionValue actionExecutionValue =
+ actionValueWithTreeArtifacts(ImmutableList.of(), ImmutableList.of(archivedTreeArtifact));
+
+ writeFile(archivedTreeArtifact.getPath(), "new content");
+ assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
+ }
+
+ @Test
+ public void getDirtyActionValues_editedArchivedFileForTreeArtifact_reportsChange()
+ throws Exception {
+ SpecialArtifact treeArtifact = createTreeArtifact("dir");
+ ArchivedTreeArtifact archivedTreeArtifact =
+ createArchivedTreeArtifactWithContent(treeArtifact, "old content");
+ ActionExecutionValue actionExecutionValue =
+ actionValueWithTreeArtifacts(
+ ImmutableList.of(
+ createTreeFileArtifactWithContent(
+ treeArtifact, /*parentRelativePath=*/ "file1", "content"),
+ createTreeFileArtifactWithContent(
+ treeArtifact, /*parentRelativePath=*/ "file2", "content2")),
+ ImmutableList.of(archivedTreeArtifact));
+
+ writeFile(archivedTreeArtifact.getPath(), "new content");
+ assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
+ }
+
+ @Test
+ public void getDirtyActionValues_deletedArchivedFileForTreeArtifact_reportsChange()
+ throws Exception {
+ SpecialArtifact treeArtifact = createTreeArtifact("dir");
+ ArchivedTreeArtifact archivedTreeArtifact = createArchivedTreeArtifactWithContent(treeArtifact);
+ ActionExecutionValue actionExecutionValue =
+ actionValueWithTreeArtifacts(
+ ImmutableList.of(
+ createTreeFileArtifactWithContent(
+ treeArtifact, /*parentRelativePath=*/ "file1", "content"),
+ createTreeFileArtifactWithContent(
+ treeArtifact, /*parentRelativePath=*/ "file2", "content2")),
+ ImmutableList.of(archivedTreeArtifact));
+
+ archivedTreeArtifact.getPath().delete();
+ assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
+ }
+
+ @Test
+ public void getDirtyActionValues_deletedArchivedFileForEmptyTreeArtifact_reportsChange()
+ throws Exception {
+ SpecialArtifact treeArtifact = createTreeArtifact("dir");
+ ArchivedTreeArtifact archivedTreeArtifact = createArchivedTreeArtifactWithContent(treeArtifact);
+ ActionExecutionValue actionExecutionValue =
+ actionValueWithTreeArtifacts(ImmutableList.of(), ImmutableList.of(archivedTreeArtifact));
+
+ archivedTreeArtifact.getPath().delete();
+ assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
+ }
+
+ @Test
+ public void getDirtyActionValues_editedFileForTreeArtifactWithArchivedFile_reportsChange()
+ throws Exception {
+ SpecialArtifact treeArtifact = createTreeArtifact("dir");
+ TreeFileArtifact child1 =
+ createTreeFileArtifactWithContent(
+ treeArtifact, /*parentRelativePath=*/ "file1", "old content");
+ ActionExecutionValue actionExecutionValue =
+ actionValueWithTreeArtifacts(
+ ImmutableList.of(
+ child1,
+ createTreeFileArtifactWithContent(
+ treeArtifact, /*parentRelativePath=*/ "file2", "content2")),
+ ImmutableList.of(createArchivedTreeArtifactWithContent(treeArtifact)));
+
+ writeFile(child1.getPath(), "new content");
+ assertThat(getDirtyActionValues(actionExecutionValue)).containsExactly(ACTION_LOOKUP_DATA);
+ }
+
+ @Test
+ public void getDirtyActionValues_treeArtifactWithArchivedArtifact_reportsOnlyChangedKey()
+ throws Exception {
+ SpecialArtifact unchangedTreeArtifact = createTreeArtifact("dir1");
+ ActionExecutionValue unchangedValue =
+ actionValueWithTreeArtifacts(
+ ImmutableList.of(createTreeFileArtifactWithContent(unchangedTreeArtifact, "child")),
+ ImmutableList.of(createArchivedTreeArtifactWithContent(unchangedTreeArtifact)));
+ SpecialArtifact changedTreeArtifact = createTreeArtifact("dir2");
+ ArchivedTreeArtifact changedArchivedTreeArtifact =
+ createArchivedTreeArtifactWithContent(changedTreeArtifact, "old content");
+ ActionExecutionValue changedValue =
+ actionValueWithTreeArtifacts(
+ ImmutableList.of(
+ createTreeFileArtifactWithContent(changedTreeArtifact, "file", "content")),
+ ImmutableList.of(changedArchivedTreeArtifact));
+
+ writeFile(changedArchivedTreeArtifact.getPath(), "new content");
+ assertThat(
+ getDirtyActionValues(
+ ImmutableMap.of(
+ actionLookupData(0), unchangedValue, actionLookupData(1), changedValue)))
+ .containsExactly(actionLookupData(1));
+ }
+
+ private Collection<SkyKey> getDirtyActionValues(ActionExecutionValue actionExecutionValue)
+ throws InterruptedException {
+ return getDirtyActionValues(ImmutableMap.of(ACTION_LOOKUP_DATA, actionExecutionValue));
+ }
+
+ private Collection<SkyKey> getDirtyActionValues(ImmutableMap<SkyKey, SkyValue> valuesMap)
+ throws InterruptedException {
+ return new FilesystemValueChecker(
+ /*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
+ .getDirtyActionValues(
+ valuesMap,
+ batchStat.getBatchStat(fs),
+ ModifiedFileSet.EVERYTHING_MODIFIED,
+ /*trustRemoteArtifacts=*/ false);
+ }
+
+ private TreeFileArtifact createTreeFileArtifactWithContent(
+ SpecialArtifact treeArtifact, String parentRelativePath, String... contentLines)
+ throws IOException {
+ TreeFileArtifact artifact = TreeFileArtifact.createTreeOutput(treeArtifact, parentRelativePath);
+ writeFile(artifact.getPath(), contentLines);
+ return artifact;
+ }
+
+ private ArchivedTreeArtifact createArchivedTreeArtifactWithContent(
+ SpecialArtifact treeArtifact, String... contentLines) throws IOException {
+ ArchivedTreeArtifact artifact = ArchivedTreeArtifact.createForTree(treeArtifact);
+ writeFile(artifact.getPath(), contentLines);
+ return artifact;
+ }
+
+ private static ActionLookupData actionLookupData(int actionIndex) {
+ return ActionLookupData.create(ACTION_LOOKUP_KEY, actionIndex);
+ }
+
@Test
public void testEmpty() throws Exception {
FilesystemValueChecker checker =
@@ -532,7 +853,6 @@
@Test
public void getDirtyActionValues_touchedTreeDirectory_returnsEmptyDiff(
- @TestParameter BatchStatMode batchStatMode,
@TestParameter({"", "subdir"}) String touchedTreePath,
@TestParameter ModifiedSetReporting modifiedSet)
throws Exception {
@@ -550,7 +870,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
modifiedSet.getModifiedFileSet(tree.getExecPath()),
/*trustRemoteArtifacts=*/ false))
.isEmpty();
@@ -558,8 +878,7 @@
@Test
public void getDirtyActionValues_deleteEmptyTreeDirectory_returnsTreeKey(
- @TestParameter BatchStatMode batchStatMode, @TestParameter ModifiedSetReporting modifiedSet)
- throws Exception {
+ @TestParameter ModifiedSetReporting modifiedSet) throws Exception {
SpecialArtifact tree = createTreeArtifact("tree");
tree.getPath().createDirectoryAndParents();
SkyKey actionKey = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
@@ -573,15 +892,15 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
modifiedSet.getModifiedFileSet(tree.getExecPath()),
/*trustRemoteArtifacts=*/ false))
.containsExactly(actionKey);
}
@Test
- public void getDirtyActionValues_treeDirectoryReplacedWithSymlink_returnsTreeKey(
- @TestParameter BatchStatMode batchStatMode) throws Exception {
+ public void getDirtyActionValues_treeDirectoryReplacedWithSymlink_returnsTreeKey()
+ throws Exception {
SpecialArtifact tree = createTreeArtifact("tree");
tree.getPath().createDirectoryAndParents();
SkyKey actionKey = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
@@ -599,7 +918,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
ModifiedFileSet.EVERYTHING_MODIFIED,
/* trustRemoteArtifacts= */ false))
.containsExactly(actionKey); // Symbolic links should count as dirty
@@ -607,8 +926,7 @@
@Test
public void getDirtyActionValues_modifiedTreeFile_returnsTreeKey(
- @TestParameter BatchStatMode batchStatMode, @TestParameter ModifiedSetReporting modifiedSet)
- throws Exception {
+ @TestParameter ModifiedSetReporting modifiedSet) throws Exception {
SpecialArtifact tree = createTreeArtifact("tree");
TreeFileArtifact treeFile = TreeFileArtifact.createTreeOutput(tree, "file");
FileSystemUtils.writeIsoLatin1(treeFile.getPath(), "text");
@@ -623,7 +941,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
modifiedSet.getModifiedFileSet(treeFile.getExecPath()),
/*trustRemoteArtifacts=*/ false))
.containsExactly(actionKey);
@@ -631,8 +949,7 @@
@Test
public void getDirtyActionValues_addedTreeFile_returnsTreeKey(
- @TestParameter BatchStatMode batchStatMode, @TestParameter ModifiedSetReporting modifiedSet)
- throws Exception {
+ @TestParameter ModifiedSetReporting modifiedSet) throws Exception {
SpecialArtifact tree = createTreeArtifact("tree");
TreeFileArtifact treeFile = TreeFileArtifact.createTreeOutput(tree, "file1");
FileSystemUtils.writeIsoLatin1(treeFile.getPath());
@@ -648,7 +965,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
modifiedSet.getModifiedFileSet(newFile.getExecPath()),
/*trustRemoteArtifacts=*/ false))
.containsExactly(actionKey);
@@ -656,8 +973,7 @@
@Test
public void getDirtyActionValues_addedTreeFileToEmptyTree_returnsTreeKey(
- @TestParameter BatchStatMode batchStatMode, @TestParameter ModifiedSetReporting modifiedSet)
- throws Exception {
+ @TestParameter ModifiedSetReporting modifiedSet) throws Exception {
SpecialArtifact tree = createTreeArtifact("tree");
tree.getPath().createDirectoryAndParents();
SkyKey actionKey = ActionLookupData.create(ACTION_LOOKUP_KEY, 0);
@@ -672,7 +988,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
modifiedSet.getModifiedFileSet(newFile.getExecPath()),
/*trustRemoteArtifacts=*/ false))
.containsExactly(actionKey);
@@ -680,8 +996,7 @@
@Test
public void getDirtyActionValues_deletedTreeFile_returnsTreeKey(
- @TestParameter BatchStatMode batchStatMode, @TestParameter ModifiedSetReporting modifiedSet)
- throws Exception {
+ @TestParameter ModifiedSetReporting modifiedSet) throws Exception {
SpecialArtifact tree = createTreeArtifact("tree");
TreeFileArtifact treeFile = TreeFileArtifact.createTreeOutput(tree, "file");
FileSystemUtils.writeIsoLatin1(treeFile.getPath());
@@ -696,15 +1011,14 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
modifiedSet.getModifiedFileSet(treeFile.getExecPath()),
/*trustRemoteArtifacts=*/ false))
.containsExactly(actionKey);
}
@Test
- public void getDirtyActionValues_everythingModified_returnsAllKeys(
- @TestParameter BatchStatMode batchStatMode) throws Exception {
+ public void getDirtyActionValues_everythingModified_returnsAllKeys() throws Exception {
SpecialArtifact tree1 = createTreeArtifact("tree1");
TreeFileArtifact tree1File = TreeFileArtifact.createTreeOutput(tree1, "file");
FileSystemUtils.writeIsoLatin1(tree1File.getPath(), "text");
@@ -728,7 +1042,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
ModifiedFileSet.EVERYTHING_MODIFIED,
/*trustRemoteArtifacts=*/ false))
.containsExactly(actionKey1, actionKey2);
@@ -736,8 +1050,7 @@
@Test
public void getDirtyActionValues_changedFileNotInModifiedSet_returnsKeysFromSetOnly(
- @TestParameter BatchStatMode batchStatMode, @TestParameter boolean reportFirst)
- throws Exception {
+ @TestParameter boolean reportFirst) throws Exception {
SpecialArtifact tree1 = createTreeArtifact("tree1");
TreeFileArtifact tree1File = TreeFileArtifact.createTreeOutput(tree1, "file");
FileSystemUtils.writeIsoLatin1(tree1File.getPath(), "text");
@@ -761,7 +1074,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
ModifiedFileSet.builder()
.modify((reportFirst ? tree1File : tree2File).getExecPath())
.build(),
@@ -770,8 +1083,8 @@
}
@Test
- public void getDirtyActionValues_middleFileSkippedInModifiedFileSet_returnsKeysFromSetOnly(
- @TestParameter BatchStatMode batchStatMode) throws Exception {
+ public void getDirtyActionValues_middleFileSkippedInModifiedFileSet_returnsKeysFromSetOnly()
+ throws Exception {
SpecialArtifact treeA = createTreeArtifact("a_tree");
TreeFileArtifact treeAFile = TreeFileArtifact.createTreeOutput(treeA, "file");
FileSystemUtils.writeIsoLatin1(treeAFile.getPath());
@@ -802,7 +1115,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
ModifiedFileSet.builder()
.modify(treeAFile.getExecPath())
.modify(treeCFile.getExecPath())
@@ -812,8 +1125,7 @@
}
@Test
- public void getDirtyActionValues_nothingModified_returnsEmptyDiff(
- @TestParameter BatchStatMode batchStatMode) throws Exception {
+ public void getDirtyActionValues_nothingModified_returnsEmptyDiff() throws Exception {
SpecialArtifact tree = createTreeArtifact("tree");
TreeFileArtifact treeFile = TreeFileArtifact.createTreeOutput(tree, "file");
FileSystemUtils.writeIsoLatin1(treeFile.getPath());
@@ -828,7 +1140,7 @@
/*tsgm=*/ null, /*lastExecutionTimeRange=*/ null, FSVC_THREADS_FOR_TEST)
.getDirtyActionValues(
evaluator.getValues(),
- batchStatMode.getBatchStat(fs),
+ batchStat.getBatchStat(fs),
ModifiedFileSet.NOTHING_MODIFIED,
/*trustRemoteArtifacts=*/ false))
.isEmpty();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTestBase.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTestBase.java
deleted file mode 100644
index 8f60dac..0000000
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTestBase.java
+++ /dev/null
@@ -1,192 +0,0 @@
-// Copyright 2020 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.skyframe;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.actions.ActionLookupKey;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact;
-import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
-import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
-import com.google.devtools.build.lib.actions.ArtifactRoot;
-import com.google.devtools.build.lib.actions.ArtifactRoot.RootType;
-import com.google.devtools.build.lib.actions.FileArtifactValue;
-import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.testutil.ManualClock;
-import com.google.devtools.build.lib.vfs.BatchStat;
-import com.google.devtools.build.lib.vfs.DigestHashFunction;
-import com.google.devtools.build.lib.vfs.FileStatus;
-import com.google.devtools.build.lib.vfs.FileStatusWithDigest;
-import com.google.devtools.build.lib.vfs.FileStatusWithDigestAdapter;
-import com.google.devtools.build.lib.vfs.FileSystem;
-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.vfs.inmemoryfs.InMemoryFileSystem;
-import com.google.devtools.build.skyframe.SkyFunctionName;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import javax.annotation.Nullable;
-
-/**
- * Helper class to allow sharing test helpers between {@link FilesystemValueCheckerTest} and {@link
- * FilesystemValueCheckerParameterizedTest}.
- */
-public class FilesystemValueCheckerTestBase {
-
- static final int FSVC_THREADS_FOR_TEST = 200;
- static final ActionLookupKey ACTION_LOOKUP_KEY =
- new ActionLookupKey() {
- @Override
- public SkyFunctionName functionName() {
- return SkyFunctionName.FOR_TESTING;
- }
-
- @Nullable
- @Override
- public Label getLabel() {
- return null;
- }
-
- @Nullable
- @Override
- public BuildConfigurationKey getConfigurationKey() {
- return null;
- }
- };
-
- final MockFileSystem fs = new MockFileSystem();
-
- SpecialArtifact createTreeArtifact(String relPath) throws IOException {
- String outSegment = "bin";
- Path outputDir = fs.getPath("/" + outSegment);
- Path outputPath = outputDir.getRelative(relPath);
- outputDir.createDirectory();
- ArtifactRoot derivedRoot =
- ArtifactRoot.asDerivedRoot(fs.getPath("/"), RootType.Output, outSegment);
- return ActionsTestUtil.createTreeArtifactWithGeneratingAction(
- derivedRoot,
- derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)));
- }
-
- static ActionExecutionValue actionValueWithTreeArtifacts(List<TreeFileArtifact> contents)
- throws IOException {
- return actionValueWithTreeArtifacts(contents, ImmutableList.of());
- }
-
- static ActionExecutionValue actionValueWithTreeArtifacts(
- Iterable<TreeFileArtifact> contents, Iterable<ArchivedTreeArtifact> archivedTreeArtifacts)
- throws IOException {
- TreeArtifactValue.MultiBuilder treeArtifacts = TreeArtifactValue.newMultiBuilder();
-
- for (TreeFileArtifact output : contents) {
- treeArtifacts.putChild(output, createMetadataFromFileSystem(output));
- }
-
- for (ArchivedTreeArtifact archivedTreeArtifact : archivedTreeArtifacts) {
- treeArtifacts.setArchivedRepresentation(
- archivedTreeArtifact, createMetadataFromFileSystem(archivedTreeArtifact));
- }
-
- Map<Artifact, TreeArtifactValue> treeArtifactData = new HashMap<>();
- treeArtifacts.injectTo(treeArtifactData::put);
-
- return ActionExecutionValue.createForTesting(
- /*artifactData=*/ ImmutableMap.of(),
- ImmutableMap.copyOf(treeArtifactData),
- /*outputSymlinks=*/ null);
- }
-
- private static FileArtifactValue createMetadataFromFileSystem(Artifact artifact)
- throws IOException {
- Path path = artifact.getPath();
- FileArtifactValue noDigest =
- ActionMetadataHandler.fileArtifactValueFromArtifact(
- artifact, FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW)), null);
- return FileArtifactValue.createFromInjectedDigest(noDigest, path.getDigest());
- }
-
- void writeFile(Path path, String... lines) throws IOException {
- // Make sure we advance the clock to detect modifications which do not change the size, which
- // rely on ctime.
- fs.advanceClockMillis(1);
- FileSystemUtils.writeIsoLatin1(path, lines);
- }
-
- static final class MockFileSystem extends InMemoryFileSystem {
- boolean statThrowsRuntimeException;
- boolean readlinkThrowsIoException;
-
- MockFileSystem() {
- this(new ManualClock());
- }
-
- private MockFileSystem(ManualClock clock) {
- super(clock, DigestHashFunction.SHA256);
- }
-
- @Override
- public FileStatus statIfFound(PathFragment path, boolean followSymlinks) throws IOException {
- if (statThrowsRuntimeException) {
- throw new RuntimeException("bork");
- }
- return super.statIfFound(path, followSymlinks);
- }
-
- @Override
- protected PathFragment readSymbolicLink(PathFragment path) throws IOException {
- if (readlinkThrowsIoException) {
- throw new IOException("readlink failed");
- }
- return super.readSymbolicLink(path);
- }
-
- void advanceClockMillis(int millis) {
- ((ManualClock) clock).advanceMillis(millis);
- }
- }
-
- enum BatchStatMode {
- DISABLED {
- @Nullable
- @Override
- BatchStat getBatchStat(FileSystem fileSystem) {
- return null;
- }
- },
- ENABLED {
- @Override
- BatchStat getBatchStat(FileSystem fileSystem) {
- return (useDigest, includeLinks, paths) -> {
- List<FileStatusWithDigest> stats = new ArrayList<>();
- for (PathFragment pathFrag : paths) {
- stats.add(
- FileStatusWithDigestAdapter.adapt(
- fileSystem.getPath("/").getRelative(pathFrag).statIfFound(Symlinks.NOFOLLOW)));
- }
- return stats;
- };
- }
- };
-
- @Nullable
- abstract BatchStat getBatchStat(FileSystem fileSystem);
- }
-}
