Do not inject tree artifact values using `MetadataConsumer`. Action file system expects `TreeArtifactValue`s to be injected to the output store directly. Restrict injection to files only. PiperOrigin-RevId: 332343506
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 251dcc2..1b3f6c2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -54,7 +54,6 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; import com.google.devtools.build.lib.actions.Artifact.OwnerlessArtifactWrapper; -import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.CachedActionEvent; import com.google.devtools.build.lib.actions.EnvironmentalExecException; @@ -72,7 +71,6 @@ import com.google.devtools.build.lib.actions.StoppedScanningActionEvent; import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.actions.cache.MetadataHandler; -import com.google.devtools.build.lib.actions.cache.MetadataInjector; import com.google.devtools.build.lib.analysis.config.CoreOptions; import com.google.devtools.build.lib.buildtool.BuildRequestOptions; import com.google.devtools.build.lib.cmdline.Label; @@ -406,13 +404,21 @@ ActionPostprocessing postprocessing, boolean hasDiscoveredInputs) throws ActionExecutionException, InterruptedException { - MetadataAggregator metadataAggregator; if (actionFileSystem != null) { - metadataAggregator = new MetadataAggregator(metadataHandler); updateActionFileSystemContext( - action, actionFileSystem, env, metadataAggregator, expandedFilesets); - } else { - metadataAggregator = null; + action, + actionFileSystem, + env, + (artifact, metadata) -> { + // Inject metadata for files, since they can be created by actions using manual writes + // (as opposed to within spawns), in which case, we would otherwise not inject them to + // the output store. Action file system needs them to be present in the output store in + // order for the files to be readable. + if (!artifact.isChildOfDeclaredDirectory()) { + metadataHandler.injectFile(artifact, metadata); + } + }, + expandedFilesets); } ActionExecutionContext actionExecutionContext = @@ -452,8 +458,7 @@ actionStartTime, actionExecutionContext, actionLookupData, - postprocessing, - metadataAggregator))); + postprocessing))); SharedActionCallback callback = getSharedActionCallback(env.getListener(), hasDiscoveredInputs, action, actionLookupData); @@ -865,7 +870,6 @@ private final ActionLookupData actionLookupData; private final ActionExecutionStatusReporter statusReporter; private final ActionPostprocessing postprocessing; - @Nullable private final MetadataAggregator metadataAggregator; ActionRunner( Action action, @@ -873,8 +877,7 @@ long actionStartTime, ActionExecutionContext actionExecutionContext, ActionLookupData actionLookupData, - ActionPostprocessing postprocessing, - @Nullable MetadataAggregator metadataAggregator) { + ActionPostprocessing postprocessing) { this.action = action; this.metadataHandler = metadataHandler; this.actionStartTime = actionStartTime; @@ -882,7 +885,6 @@ this.actionLookupData = actionLookupData; this.statusReporter = statusReporterRef.get(); this.postprocessing = postprocessing; - this.metadataAggregator = metadataAggregator; } @SuppressWarnings("LogAndThrow") // Thrown exception shown in user output, not info logs. @@ -1104,10 +1106,6 @@ Preconditions.checkState(action.inputsDiscovered(), "Action %s successfully executed, but inputs still not known", action); - if (metadataAggregator != null) { - metadataAggregator.finish(); - } - if (!checkOutputs(action, metadataHandler)) { throw toActionExecutionException( "not all outputs were created or valid", @@ -1760,31 +1758,4 @@ return input != null ? input : perBuildFileCache.getInput(execPath); } } - - /** - * Assists with aggregation of tree artifacts for an action file system which is only aware of - * individual outputs. - */ - private static final class MetadataAggregator implements MetadataConsumer { - private final TreeArtifactValue.MultiBuilder treeArtifacts = - TreeArtifactValue.newConcurrentMultiBuilder(); - private final MetadataInjector metadataInjector; - - MetadataAggregator(MetadataInjector metadataInjector) { - this.metadataInjector = metadataInjector; - } - - @Override - public void accept(Artifact output, FileArtifactValue metadata) { - if (output.isChildOfDeclaredDirectory()) { - treeArtifacts.putChild((TreeFileArtifact) output, metadata); - } else { - metadataInjector.injectFile(output, metadata); - } - } - - void finish() { - treeArtifacts.injectTo(metadataInjector); - } - } }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java index 81a05fe..093b840 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -43,10 +43,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import javax.annotation.Nullable; -import javax.annotation.concurrent.ThreadSafe; /** * Value for TreeArtifacts, which contains a digest and the {@link FileArtifactValue}s of its child @@ -70,14 +67,22 @@ } /** Builder for constructing multiple instances of {@link TreeArtifactValue} at once. */ - public interface MultiBuilder { + public static final class MultiBuilder { + + private final Map<SpecialArtifact, Builder> map = new HashMap<>(); + + private MultiBuilder() {} + /** * Puts a child tree file into this builder under its {@linkplain TreeFileArtifact#getParent * parent}. * * @return {@code this} for convenience */ - MultiBuilder putChild(TreeFileArtifact child, FileArtifactValue metadata); + public MultiBuilder putChild(TreeFileArtifact child, FileArtifactValue metadata) { + map.computeIfAbsent(child.getParent(), Builder::new).putChild(child, metadata); + return this; + } /** * Sets the archived representation and its metadata for the {@linkplain @@ -86,24 +91,25 @@ * <p>Setting an archived representation is only allowed once per {@linkplain SpecialArtifact * tree artifact}. */ - MultiBuilder setArchivedRepresentation( - ArchivedTreeArtifact archivedArtifact, FileArtifactValue metadata); + public MultiBuilder setArchivedRepresentation( + ArchivedTreeArtifact archivedArtifact, FileArtifactValue metadata) { + map.computeIfAbsent(archivedArtifact.getParent(), Builder::new) + .setArchivedRepresentation(ArchivedRepresentation.create(archivedArtifact, metadata)); + return this; + } /** * For each unique parent seen by this builder, passes the aggregated metadata to {@link * TreeArtifactInjector#injectTree}. */ - void injectTo(TreeArtifactInjector treeInjector); + public void injectTo(TreeArtifactInjector treeInjector) { + map.forEach((parent, builder) -> treeInjector.injectTree(parent, builder.build())); + } } /** Returns a new {@link MultiBuilder}. */ public static MultiBuilder newMultiBuilder() { - return new BasicMultiBuilder(); - } - - /** Returns a new thread-safe {@link MultiBuilder}. */ - public static MultiBuilder newConcurrentMultiBuilder() { - return new ConcurrentMultiBuilder(); + return new MultiBuilder(); } /** @@ -138,6 +144,7 @@ private final byte[] digest; private final ImmutableSortedMap<TreeFileArtifact, FileArtifactValue> childData; + /** * Optional archived representation of the entire tree artifact which can be sent instead of all * the items in the directory. @@ -471,74 +478,4 @@ fingerprint.digestAndReset(), finalChildData, archivedRepresentation, entirelyRemote); } } - - private static final class BasicMultiBuilder implements MultiBuilder { - private final Map<SpecialArtifact, Builder> map = new HashMap<>(); - - @Override - public MultiBuilder putChild(TreeFileArtifact child, FileArtifactValue metadata) { - map.computeIfAbsent(child.getParent(), Builder::new).putChild(child, metadata); - return this; - } - - @Override - public MultiBuilder setArchivedRepresentation( - ArchivedTreeArtifact archivedArtifact, FileArtifactValue metadata) { - map.computeIfAbsent(archivedArtifact.getParent(), Builder::new) - .setArchivedRepresentation(ArchivedRepresentation.create(archivedArtifact, metadata)); - return this; - } - - @Override - public void injectTo(TreeArtifactInjector treeInjector) { - map.forEach((parent, builder) -> treeInjector.injectTree(parent, builder.build())); - } - } - - @ThreadSafe - private static final class ConcurrentMultiBuilder implements MultiBuilder { - private final ConcurrentMap<SpecialArtifact, ConcurrentMap<TreeFileArtifact, FileArtifactValue>> - children = new ConcurrentHashMap<>(); - private final ConcurrentMap<SpecialArtifact, ArchivedRepresentation> archivedRepresentations = - new ConcurrentHashMap<>(); - - @Override - public MultiBuilder putChild(TreeFileArtifact child, FileArtifactValue metadata) { - children - .computeIfAbsent(child.getParent(), parent -> new ConcurrentHashMap<>()) - .put(child, metadata); - return this; - } - - @Override - public MultiBuilder setArchivedRepresentation( - ArchivedTreeArtifact archivedArtifact, FileArtifactValue metadata) { - Object oldValue = - archivedRepresentations.putIfAbsent( - archivedArtifact.getParent(), - ArchivedRepresentation.create(archivedArtifact, metadata)); - checkArgument( - oldValue == null, - "Tried to add 2 archived representations for %s", - archivedArtifact.getParent()); - // We inject entries based on keys in the children map. Make sure a placeholder exists in case - // the tree artifact is otherwise empty. - children.computeIfAbsent(archivedArtifact.getParent(), ignored -> new ConcurrentHashMap<>()); - return this; - } - - @Override - public void injectTo(TreeArtifactInjector treeInjector) { - children.forEach( - (parent, children) -> { - Builder builder = new Builder(parent); - children.forEach(builder::putChild); - ArchivedRepresentation archivedRepresentation = archivedRepresentations.get(parent); - if (archivedRepresentation != null) { - builder.setArchivedRepresentation(archivedRepresentation); - } - treeInjector.injectTree(parent, builder.build()); - }); - } - } }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java index eff3f41..2e6cbb6 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java
@@ -44,6 +44,7 @@ import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.EvaluationProgressReceiver; import com.google.devtools.build.skyframe.EvaluationProgressReceiver.EvaluationState; +import com.google.devtools.build.skyframe.GraphInconsistencyReceiver; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.ValueOrException; @@ -72,7 +73,13 @@ @Before public final void createBuilder() throws Exception { progressReceiver = new TrackingEvaluationProgressReceiver(); - builder = createBuilder(inMemoryCache, 1, /*keepGoing=*/ false, progressReceiver); + builder = + createBuilder( + inMemoryCache, + 1, + /*keepGoing=*/ false, + progressReceiver, + GraphInconsistencyReceiver.THROWING); } @Before
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java index 28c409f..afa7009 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skyframe; import static com.google.devtools.build.lib.actions.util.ActionCacheTestHelper.AMNESIAC_CACHE; +import static com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator.DEFAULT_STORED_EVENT_FILTER; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; @@ -102,7 +103,9 @@ import com.google.devtools.build.skyframe.EvaluationContext; import com.google.devtools.build.skyframe.EvaluationProgressReceiver; import com.google.devtools.build.skyframe.EvaluationResult; +import com.google.devtools.build.skyframe.GraphInconsistencyReceiver; import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator; +import com.google.devtools.build.skyframe.MemoizingEvaluator.EmittedEventState; import com.google.devtools.build.skyframe.RecordingDifferencer; import com.google.devtools.build.skyframe.SequencedRecordingDifferencer; import com.google.devtools.build.skyframe.SequentialBuildDriver; @@ -184,17 +187,34 @@ return createBuilder(actionCache, 1, /*keepGoing=*/ false); } + protected BuilderWithResult createBuilder( + ActionCache actionCache, GraphInconsistencyReceiver graphInconsistencyReceiver) + throws Exception { + return createBuilder( + actionCache, + 1, + /*keepGoing=*/ false, + /*evaluationProgressReceiver=*/ null, + graphInconsistencyReceiver); + } + /** Create a ParallelBuilder with a DatabaseDependencyChecker using the specified ActionCache. */ protected BuilderWithResult createBuilder( ActionCache actionCache, final int threadCount, final boolean keepGoing) throws Exception { - return createBuilder(actionCache, threadCount, keepGoing, null); + return createBuilder( + actionCache, + threadCount, + keepGoing, + /*evaluationProgressReceiver=*/ null, + GraphInconsistencyReceiver.THROWING); } protected BuilderWithResult createBuilder( final ActionCache actionCache, final int threadCount, final boolean keepGoing, - @Nullable EvaluationProgressReceiver evaluationProgressReceiver) + @Nullable EvaluationProgressReceiver evaluationProgressReceiver, + GraphInconsistencyReceiver graphInconsistencyReceiver) throws Exception { AtomicReference<PathPackageLocator> pkgLocator = new AtomicReference<>( @@ -277,7 +297,11 @@ .put(SkyFunctions.ACTION_SKETCH, new ActionSketchFunction(actionKeyContext)) .build(), differencer, - evaluationProgressReceiver); + evaluationProgressReceiver, + graphInconsistencyReceiver, + DEFAULT_STORED_EVENT_FILTER, + new EmittedEventState(), + /*keepEdges=*/ true); final SequentialBuildDriver driver = new SequentialBuildDriver(evaluator); PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID()); PrecomputedValue.ACTION_ENV.set(differencer, ImmutableMap.<String, String>of()); @@ -378,6 +402,9 @@ /** A non-persistent cache. */ protected InMemoryActionCache inMemoryCache; + protected GraphInconsistencyReceiver graphInconsistencyReceiver = + GraphInconsistencyReceiver.THROWING; + protected SkyFunction actionTemplateExpansionFunction; /** A class that records an event. */ @@ -426,9 +453,12 @@ return createBuilder(AMNESIAC_CACHE); } - /** Creates and returns a new caching builder based on the inMemoryCache. */ + /** + * Creates and returns a new caching builder based on the {@link #inMemoryCache} and {@link + * #graphInconsistencyReceiver}. + */ protected BuilderWithResult cachingBuilder() throws Exception { - return createBuilder(inMemoryCache); + return createBuilder(inMemoryCache, graphInconsistencyReceiver); } /** {@link Builder} that saves its most recent {@link EvaluationResult}. */
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java index 2de577f..5634913 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactValueTest.java
@@ -46,9 +46,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; -import org.junit.runners.Parameterized.Parameters; /** Tests for {@link TreeArtifactValue}. */ @RunWith(JUnit4.class) @@ -443,171 +440,138 @@ assertThat(e).hasMessageThat().contains("/tree/link pointing to ../tree/file"); } - /** Parameterized tests for {@link TreeArtifactValue.MultiBuilder}. */ - @RunWith(Parameterized.class) - public static final class MultiBuilderTest { + @Test + public void multiBuilder_empty_injectsNothing() { + Map<SpecialArtifact, TreeArtifactValue> results = new HashMap<>(); - private static final ArtifactRoot ROOT = - ArtifactRoot.asDerivedRoot( - new InMemoryFileSystem(DigestHashFunction.SHA256).getPath("/root"), BIN_PATH); + TreeArtifactValue.newMultiBuilder().injectTo(results::put); - private enum MultiBuilderType { - BASIC { - @Override - TreeArtifactValue.MultiBuilder newMultiBuilder() { - return TreeArtifactValue.newMultiBuilder(); - } - }, - CONCURRENT { - @Override - TreeArtifactValue.MultiBuilder newMultiBuilder() { - return TreeArtifactValue.newConcurrentMultiBuilder(); - } - }; + assertThat(results).isEmpty(); + } - abstract TreeArtifactValue.MultiBuilder newMultiBuilder(); - } + @Test + public void multiBuilder_injectsSingleTreeArtifact() { + TreeArtifactValue.MultiBuilder treeArtifacts = TreeArtifactValue.newMultiBuilder(); + SpecialArtifact parent = createTreeArtifact("bin/tree"); + TreeFileArtifact child1 = TreeFileArtifact.createTreeOutput(parent, "child1"); + TreeFileArtifact child2 = TreeFileArtifact.createTreeOutput(parent, "child2"); + Map<SpecialArtifact, TreeArtifactValue> results = new HashMap<>(); - @Parameter public MultiBuilderType multiBuilderType; - private final Map<SpecialArtifact, TreeArtifactValue> results = new HashMap<>(); + treeArtifacts + .putChild(child1, metadataWithId(1)) + .putChild(child2, metadataWithId(2)) + .injectTo(results::put); - @Parameters(name = "{0}") - public static MultiBuilderType[] params() { - return MultiBuilderType.values(); - } + assertThat(results) + .containsExactly( + parent, + TreeArtifactValue.newBuilder(parent) + .putChild(child1, metadataWithId(1)) + .putChild(child2, metadataWithId(2)) + .build()); + } - @Test - public void emptyBuilder_injectsNothing() { - TreeArtifactValue.MultiBuilder treeArtifacts = multiBuilderType.newMultiBuilder(); + @Test + public void multiBuilder_injectsMultipleTreeArtifacts() { + TreeArtifactValue.MultiBuilder treeArtifacts = TreeArtifactValue.newMultiBuilder(); + SpecialArtifact parent1 = createTreeArtifact("bin/tree1"); + TreeFileArtifact parent1Child1 = TreeFileArtifact.createTreeOutput(parent1, "child1"); + TreeFileArtifact parent1Child2 = TreeFileArtifact.createTreeOutput(parent1, "child2"); + SpecialArtifact parent2 = createTreeArtifact("bin/tree2"); + TreeFileArtifact parent2Child = TreeFileArtifact.createTreeOutput(parent2, "child"); + Map<SpecialArtifact, TreeArtifactValue> results = new HashMap<>(); - treeArtifacts.injectTo(results::put); + treeArtifacts + .putChild(parent1Child1, metadataWithId(1)) + .putChild(parent2Child, metadataWithId(3)) + .putChild(parent1Child2, metadataWithId(2)) + .injectTo(results::put); - assertThat(results).isEmpty(); - } + assertThat(results) + .containsExactly( + parent1, + TreeArtifactValue.newBuilder(parent1) + .putChild(parent1Child1, metadataWithId(1)) + .putChild(parent1Child2, metadataWithId(2)) + .build(), + parent2, + TreeArtifactValue.newBuilder(parent2) + .putChild(parent2Child, metadataWithId(3)) + .build()); + } - @Test - public void injectsSingleTreeArtifact() { - TreeArtifactValue.MultiBuilder treeArtifacts = multiBuilderType.newMultiBuilder(); - SpecialArtifact parent = createTreeArtifact("bin/tree"); - TreeFileArtifact child1 = TreeFileArtifact.createTreeOutput(parent, "child1"); - TreeFileArtifact child2 = TreeFileArtifact.createTreeOutput(parent, "child2"); + @Test + public void multiBuilder_injectsTreeArtifactWithArchivedRepresentation() { + TreeArtifactValue.MultiBuilder builder = TreeArtifactValue.newMultiBuilder(); + SpecialArtifact parent = createTreeArtifact("bin/tree"); + TreeFileArtifact child = TreeFileArtifact.createTreeOutput(parent, "child"); + FileArtifactValue childMetadata = metadataWithId(1); + ArchivedTreeArtifact archivedTreeArtifact = createArchivedTreeArtifact(parent); + FileArtifactValue archivedTreeArtifactMetadata = metadataWithId(2); + Map<SpecialArtifact, TreeArtifactValue> results = new HashMap<>(); - treeArtifacts - .putChild(child1, metadataWithId(1)) - .putChild(child2, metadataWithId(2)) - .injectTo(results::put); + builder + .putChild(child, childMetadata) + .setArchivedRepresentation(archivedTreeArtifact, archivedTreeArtifactMetadata) + .injectTo(results::put); - assertThat(results) - .containsExactly( - parent, - TreeArtifactValue.newBuilder(parent) - .putChild(child1, metadataWithId(1)) - .putChild(child2, metadataWithId(2)) - .build()); - } + assertThat(results) + .containsExactly( + parent, + TreeArtifactValue.newBuilder(parent) + .putChild(child, childMetadata) + .setArchivedRepresentation(archivedTreeArtifact, archivedTreeArtifactMetadata) + .build()); + } - @Test - public void injectsMultipleTreeArtifacts() { - TreeArtifactValue.MultiBuilder treeArtifacts = multiBuilderType.newMultiBuilder(); - SpecialArtifact parent1 = createTreeArtifact("bin/tree1"); - TreeFileArtifact parent1Child1 = TreeFileArtifact.createTreeOutput(parent1, "child1"); - TreeFileArtifact parent1Child2 = TreeFileArtifact.createTreeOutput(parent1, "child2"); - SpecialArtifact parent2 = createTreeArtifact("bin/tree2"); - TreeFileArtifact parent2Child = TreeFileArtifact.createTreeOutput(parent2, "child"); + @Test + public void multiBuilder_injectsEmptyTreeArtifactWithArchivedRepresentation() { + TreeArtifactValue.MultiBuilder builder = TreeArtifactValue.newMultiBuilder(); + SpecialArtifact parent = createTreeArtifact("bin/tree"); + ArchivedTreeArtifact archivedTreeArtifact = createArchivedTreeArtifact(parent); + FileArtifactValue metadata = metadataWithId(1); + Map<SpecialArtifact, TreeArtifactValue> results = new HashMap<>(); - treeArtifacts - .putChild(parent1Child1, metadataWithId(1)) - .putChild(parent2Child, metadataWithId(3)) - .putChild(parent1Child2, metadataWithId(2)) - .injectTo(results::put); + builder.setArchivedRepresentation(archivedTreeArtifact, metadata).injectTo(results::put); - assertThat(results) - .containsExactly( - parent1, - TreeArtifactValue.newBuilder(parent1) - .putChild(parent1Child1, metadataWithId(1)) - .putChild(parent1Child2, metadataWithId(2)) - .build(), - parent2, - TreeArtifactValue.newBuilder(parent2) - .putChild(parent2Child, metadataWithId(3)) - .build()); - } + assertThat(results) + .containsExactly( + parent, + TreeArtifactValue.newBuilder(parent) + .setArchivedRepresentation(archivedTreeArtifact, metadata) + .build()); + } - @Test - public void injectsTreeArtifactWithArchivedRepresentation() { - TreeArtifactValue.MultiBuilder builder = multiBuilderType.newMultiBuilder(); - SpecialArtifact parent = createTreeArtifact("bin/tree"); - TreeFileArtifact child = TreeFileArtifact.createTreeOutput(parent, "child"); - FileArtifactValue childMetadata = metadataWithId(1); - ArchivedTreeArtifact archivedTreeArtifact = createArchivedTreeArtifact(parent); - FileArtifactValue archivedTreeArtifactMetadata = metadataWithId(2); + @Test + public void multiBuilder_injectsTreeArtifactsWithAndWithoutArchivedRepresentation() { + TreeArtifactValue.MultiBuilder builder = TreeArtifactValue.newMultiBuilder(); + SpecialArtifact parent1 = createTreeArtifact("bin/tree1"); + ArchivedTreeArtifact archivedArtifact1 = createArchivedTreeArtifact(parent1); + FileArtifactValue archivedArtifact1Metadata = metadataWithId(1); + TreeFileArtifact parent1Child = TreeFileArtifact.createTreeOutput(parent1, "child"); + FileArtifactValue parent1ChildMetadata = metadataWithId(2); + SpecialArtifact parent2 = createTreeArtifact("bin/tree2"); + TreeFileArtifact parent2Child = TreeFileArtifact.createTreeOutput(parent2, "child"); + FileArtifactValue parent2ChildMetadata = metadataWithId(3); + Map<SpecialArtifact, TreeArtifactValue> results = new HashMap<>(); - builder - .putChild(child, childMetadata) - .setArchivedRepresentation(archivedTreeArtifact, archivedTreeArtifactMetadata) - .injectTo(results::put); + builder + .setArchivedRepresentation(archivedArtifact1, archivedArtifact1Metadata) + .putChild(parent1Child, parent1ChildMetadata) + .putChild(parent2Child, parent2ChildMetadata) + .injectTo(results::put); - assertThat(results) - .containsExactly( - parent, - TreeArtifactValue.newBuilder(parent) - .putChild(child, childMetadata) - .setArchivedRepresentation(archivedTreeArtifact, archivedTreeArtifactMetadata) - .build()); - } - - @Test - public void injectsEmptyTreeArtifactWithArchivedRepresentation() { - TreeArtifactValue.MultiBuilder builder = multiBuilderType.newMultiBuilder(); - SpecialArtifact parent = createTreeArtifact("bin/tree"); - ArchivedTreeArtifact archivedTreeArtifact = createArchivedTreeArtifact(parent); - FileArtifactValue metadata = metadataWithId(1); - - builder.setArchivedRepresentation(archivedTreeArtifact, metadata).injectTo(results::put); - - assertThat(results) - .containsExactly( - parent, - TreeArtifactValue.newBuilder(parent) - .setArchivedRepresentation(archivedTreeArtifact, metadata) - .build()); - } - - @Test - public void injectsTreeArtifactsWithAndWithoutArchivedRepresentation() { - TreeArtifactValue.MultiBuilder builder = multiBuilderType.newMultiBuilder(); - SpecialArtifact parent1 = createTreeArtifact("bin/tree1"); - ArchivedTreeArtifact archivedArtifact1 = createArchivedTreeArtifact(parent1); - FileArtifactValue archivedArtifact1Metadata = metadataWithId(1); - TreeFileArtifact parent1Child = TreeFileArtifact.createTreeOutput(parent1, "child"); - FileArtifactValue parent1ChildMetadata = metadataWithId(2); - SpecialArtifact parent2 = createTreeArtifact("bin/tree2"); - TreeFileArtifact parent2Child = TreeFileArtifact.createTreeOutput(parent2, "child"); - FileArtifactValue parent2ChildMetadata = metadataWithId(3); - - builder - .setArchivedRepresentation(archivedArtifact1, archivedArtifact1Metadata) - .putChild(parent1Child, parent1ChildMetadata) - .putChild(parent2Child, parent2ChildMetadata) - .injectTo(results::put); - - assertThat(results) - .containsExactly( - parent1, - TreeArtifactValue.newBuilder(parent1) - .putChild(parent1Child, parent1ChildMetadata) - .setArchivedRepresentation(archivedArtifact1, metadataWithId(1)) - .build(), - parent2, - TreeArtifactValue.newBuilder(parent2) - .putChild(parent2Child, parent2ChildMetadata) - .build()); - } - - private static SpecialArtifact createTreeArtifact(String execPath) { - return TreeArtifactValueTest.createTreeArtifact(execPath, ROOT); - } + assertThat(results) + .containsExactly( + parent1, + TreeArtifactValue.newBuilder(parent1) + .putChild(parent1Child, parent1ChildMetadata) + .setArchivedRepresentation(archivedArtifact1, metadataWithId(1)) + .build(), + parent2, + TreeArtifactValue.newBuilder(parent2) + .putChild(parent2Child, parent2ChildMetadata) + .build()); } private static ArchivedTreeArtifact createArchivedTreeArtifact(SpecialArtifact specialArtifact) {