From 614fe0dfb9e6bed90c361e4b6bfff37c11a4775f Mon Sep 17 00:00:00 2001 From: jcater Date: Wed, 28 Feb 2018 12:44:39 -0800 Subject: Refactor the AbstractAction computeKey method to be easier to add invariants for every type of action. PiperOrigin-RevId: 187368369 --- .../build/lib/actions/ActionCacheCheckerTest.java | 9 +++++---- .../build/lib/actions/util/ActionsTestUtil.java | 5 +++-- .../devtools/build/lib/actions/util/TestAction.java | 8 +++----- .../analysis/actions/TemplateExpansionActionTest.java | 19 +++++++++++++++---- .../build/lib/analysis/util/AnalysisTestUtil.java | 5 ++--- .../lib/rules/cpp/CreateIncSymlinkActionTest.java | 19 +++++++++++++------ .../build/lib/skyframe/SkyframeAwareActionTest.java | 9 +++++---- 7 files changed, 46 insertions(+), 28 deletions(-) (limited to 'src/test/java/com/google/devtools/build') diff --git a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java index 15f6bd78b1..87578eb195 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/actions/ActionCacheCheckerTest.java @@ -35,6 +35,7 @@ import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.skyframe.FileArtifactValue; import com.google.devtools.build.lib.testutil.ManualClock; import com.google.devtools.build.lib.testutil.Scratch; +import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -173,16 +174,16 @@ public class ActionCacheCheckerTest { Action action = new NullAction() { @Override - protected String computeKey(ActionKeyContext actionKeyContext) { - return "key1"; + protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) { + fp.addString("key1"); } }; runAction(action); action = new NullAction() { @Override - protected String computeKey(ActionKeyContext actionKeyContext) { - return "key2"; + protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) { + fp.addString("key2"); } }; runAction(action); diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java index e3bd837086..a9cd9f8c67 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java @@ -66,6 +66,7 @@ import com.google.devtools.build.lib.exec.SingleBuildFileCache; import com.google.devtools.build.lib.packages.AspectDescriptor; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.util.FileType; +import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.ResourceUsage; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileStatus; @@ -311,8 +312,8 @@ public final class ActionsTestUtil { } @Override - protected String computeKey(ActionKeyContext actionKeyContext) { - return "action"; + protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) { + fp.addString("action"); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java b/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java index 82867ba905..b709c6852f 100644 --- a/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java +++ b/src/test/java/com/google/devtools/build/lib/actions/util/TestAction.java @@ -127,11 +127,9 @@ public class TestAction extends AbstractAction { } @Override - protected String computeKey(ActionKeyContext actionKeyContext) { - Fingerprint f = new Fingerprint(); - f.addPaths(Artifact.asSortedPathFragments(getOutputs())); - f.addPaths(Artifact.asSortedPathFragments(getMandatoryInputs())); - return f.hexDigestAndReset(); + protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) { + fp.addPaths(Artifact.asSortedPathFragments(getOutputs())); + fp.addPaths(Artifact.asSortedPathFragments(getMandatoryInputs())); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java index 470909c5b5..b2e3006f30 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction.Te import com.google.devtools.build.lib.exec.BinTools; import com.google.devtools.build.lib.exec.util.TestExecutorBuilder; import com.google.devtools.build.lib.testutil.FoundationTestCase; +import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -125,7 +126,8 @@ public class TemplateExpansionActionTest extends FoundationTestCase { TemplateExpansionAction b = new TemplateExpansionAction(NULL_ACTION_OWNER, outputArtifact2, Template.forString(TEMPLATE), ImmutableList.of(Substitution.of("%key%", "foo")), false); - assertThat(b.computeKey(actionKeyContext)).isEqualTo(a.computeKey(actionKeyContext)); + + assertThat(computeKey(a)).isEqualTo(computeKey(b)); } @Test @@ -138,7 +140,8 @@ public class TemplateExpansionActionTest extends FoundationTestCase { TemplateExpansionAction b = new TemplateExpansionAction(NULL_ACTION_OWNER, outputArtifact2, Template.forString(TEMPLATE), ImmutableList.of(Substitution.of("%key%", "foo2")), false); - assertThat(a.computeKey(actionKeyContext).equals(b.computeKey(actionKeyContext))).isFalse(); + + assertThat(computeKey(a)).isNotEqualTo(computeKey(b)); } @Test @@ -151,7 +154,8 @@ public class TemplateExpansionActionTest extends FoundationTestCase { TemplateExpansionAction b = new TemplateExpansionAction(NULL_ACTION_OWNER, outputArtifact2, Template.forString(TEMPLATE), ImmutableList.of(Substitution.of("%key%", "foo")), true); - assertThat(a.computeKey(actionKeyContext).equals(b.computeKey(actionKeyContext))).isFalse(); + + assertThat(computeKey(a)).isNotEqualTo(computeKey(b)); } @Test @@ -164,7 +168,8 @@ public class TemplateExpansionActionTest extends FoundationTestCase { TemplateExpansionAction b = new TemplateExpansionAction(NULL_ACTION_OWNER, outputArtifact2, Template.forString(TEMPLATE + " "), ImmutableList.of(Substitution.of("%key%", "foo")), false); - assertThat(a.computeKey(actionKeyContext).equals(b.computeKey(actionKeyContext))).isFalse(); + + assertThat(computeKey(a)).isNotEqualTo(computeKey(b)); } private TemplateExpansionAction createWithArtifact() { @@ -228,4 +233,10 @@ public class TemplateExpansionActionTest extends FoundationTestCase { executeTemplateExpansion(expected, ImmutableList.of(Substitution.of("%key%", SPECIAL_CHARS))); } + + private String computeKey(TemplateExpansionAction action) { + Fingerprint fp = new Fingerprint(); + action.computeKey(actionKeyContext, fp); + return fp.hexDigestAndReset(); + } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java index dc15b84842..a86066d1a5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java @@ -51,6 +51,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.syntax.SkylarkSemantics; import com.google.devtools.build.lib.testutil.TestConstants; +import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; @@ -234,9 +235,7 @@ public final class AnalysisTestUtil { } @Override - public String computeKey(ActionKeyContext actionKeyContext) { - return ""; - } + public void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) {} @Override public Artifact getVolatileStatus() { diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java index 6c4a74f289..f24f200509 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.testutil.FoundationTestCase; +import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; @@ -54,8 +55,8 @@ public class CreateIncSymlinkActionTest extends FoundationTestCase { d = new Artifact(PathFragment.create("d"), root); CreateIncSymlinkAction action2 = new CreateIncSymlinkAction(NULL_ACTION_OWNER, ImmutableMap.of(c, d, a, b), includePath); - assertThat(action2.computeKey(actionKeyContext)) - .isEqualTo(action1.computeKey(actionKeyContext)); + + assertThat(computeKey(action2)).isEqualTo(computeKey(action1)); } @Test @@ -71,8 +72,8 @@ public class CreateIncSymlinkActionTest extends FoundationTestCase { b = new Artifact(PathFragment.create("c"), root); CreateIncSymlinkAction action2 = new CreateIncSymlinkAction(NULL_ACTION_OWNER, ImmutableMap.of(a, b), includePath); - assertThat(action2.computeKey(actionKeyContext)) - .isNotEqualTo(action1.computeKey(actionKeyContext)); + + assertThat(computeKey(action2)).isNotEqualTo(computeKey(action1)); } @Test @@ -88,8 +89,8 @@ public class CreateIncSymlinkActionTest extends FoundationTestCase { b = new Artifact(PathFragment.create("b"), root); CreateIncSymlinkAction action2 = new CreateIncSymlinkAction(NULL_ACTION_OWNER, ImmutableMap.of(a, b), includePath); - assertThat(action2.computeKey(actionKeyContext)) - .isNotEqualTo(action1.computeKey(actionKeyContext)); + + assertThat(computeKey(action2)).isNotEqualTo(computeKey(action1)); } @Test @@ -125,4 +126,10 @@ public class CreateIncSymlinkActionTest extends FoundationTestCase { action.prepare(fileSystem, rootDirectory); assertThat(extra.exists()).isFalse(); } + + private String computeKey(CreateIncSymlinkAction action) { + Fingerprint fp = new Fingerprint(); + action.computeKey(actionKeyContext, fp); + return fp.hexDigestAndReset(); + } } 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 d221a772b3..bc206657e6 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 @@ -209,8 +209,9 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase { } @Override - protected String computeKey(ActionKeyContext actionKeyContext) { - return getPrimaryOutput().getExecPathString() + executionCounter.get(); + protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) { + fp.addString(getPrimaryOutput().getExecPathString()); + fp.addInt(executionCounter.get()); } } @@ -655,8 +656,8 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase { } @Override - protected String computeKey(ActionKeyContext actionKeyContext) { - return new Fingerprint().addInt(42).hexDigestAndReset(); + protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) { + fp.addInt(42); } } -- cgit v1.2.3