Fix bug in TargetPatternList.equals, which was ignoring the offset.
--
MOS_MIGRATED_REVID=114326701
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
index 4c864db..6b702e7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseValue.java
@@ -128,7 +128,7 @@
@ThreadSafe
public static SkyKey key(ImmutableList<String> targetPatterns, String offset,
boolean compileOneDependency, boolean buildTestsOnly, boolean determineTests,
- TestFilter testFilter) {
+ @Nullable TestFilter testFilter) {
return new SkyKey(SkyFunctions.TARGET_PATTERN_PHASE, new TargetPatternList(
targetPatterns, offset, compileOneDependency, buildTestsOnly, determineTests, testFilter));
}
@@ -144,17 +144,20 @@
private final boolean compileOneDependency;
private final boolean buildTestsOnly;
private final boolean determineTests;
- private final TestFilter testFilter;
+ @Nullable private final TestFilter testFilter;
public TargetPatternList(ImmutableList<String> targetPatterns, String offset,
boolean compileOneDependency, boolean buildTestsOnly, boolean determineTests,
- TestFilter testFilter) {
- this.targetPatterns = targetPatterns;
- this.offset = offset;
+ @Nullable TestFilter testFilter) {
+ this.targetPatterns = Preconditions.checkNotNull(targetPatterns);
+ this.offset = Preconditions.checkNotNull(offset);
this.compileOneDependency = compileOneDependency;
this.buildTestsOnly = buildTestsOnly;
this.determineTests = determineTests;
this.testFilter = testFilter;
+ if (buildTestsOnly || determineTests) {
+ Preconditions.checkNotNull(testFilter);
+ }
}
public ImmutableList<String> getTargetPatterns() {
@@ -183,13 +186,22 @@
@Override
public String toString() {
- return targetPatterns.toString();
+ StringBuilder result = new StringBuilder();
+ result.append(targetPatterns);
+ if (!offset.isEmpty()) {
+ result.append(" OFFSET=").append(offset);
+ }
+ result.append(compileOneDependency ? " COMPILE_ONE_DEPENDENCY" : "");
+ result.append(buildTestsOnly ? " BUILD_TESTS_ONLY" : "");
+ result.append(determineTests ? " DETERMINE_TESTS" : "");
+ result.append(testFilter != null ? testFilter : "");
+ return result.toString();
}
@Override
public int hashCode() {
- return Objects.hash(targetPatterns, compileOneDependency, buildTestsOnly, determineTests,
- testFilter);
+ return Objects.hash(targetPatterns, offset, compileOneDependency, buildTestsOnly,
+ determineTests, testFilter);
}
@Override
@@ -202,10 +214,11 @@
}
TargetPatternList other = (TargetPatternList) obj;
return other.targetPatterns.equals(this.targetPatterns)
+ && other.offset.equals(this.offset)
&& other.compileOneDependency == compileOneDependency
&& other.buildTestsOnly == buildTestsOnly
&& other.determineTests == determineTests
- && other.testFilter.equals(testFilter);
+ && Objects.equals(other.testFilter, testFilter);
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseKeyTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseKeyTest.java
new file mode 100644
index 0000000..e5d72aa
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseKeyTest.java
@@ -0,0 +1,83 @@
+// Copyright 2016 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.devtools.build.lib.skyframe.TargetPatternPhaseKeyTest.Flag.BUILD_TESTS_ONLY;
+import static com.google.devtools.build.lib.skyframe.TargetPatternPhaseKeyTest.Flag.COMPILE_ONE_DEPENDENCY;
+import static com.google.devtools.build.lib.skyframe.TargetPatternPhaseKeyTest.Flag.DETERMINE_TESTS;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.testing.EqualsTester;
+import com.google.common.testing.NullPointerTester;
+import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.pkgcache.LoadingOptions;
+import com.google.devtools.build.lib.pkgcache.TestFilter;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue.TargetPatternList;
+import com.google.devtools.common.options.Options;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import javax.annotation.Nullable;
+
+/** Tests for {@link TargetPatternList}. */
+@RunWith(JUnit4.class)
+public class TargetPatternPhaseKeyTest {
+ static enum Flag {
+ COMPILE_ONE_DEPENDENCY,
+ BUILD_TESTS_ONLY,
+ DETERMINE_TESTS
+ }
+
+ @Test
+ public void testEquality() throws Exception {
+ new EqualsTester()
+ .addEqualityGroup(of(ImmutableList.of("a"), "offset"))
+ .addEqualityGroup(of(ImmutableList.of("b"), "offset"))
+ .addEqualityGroup(of(ImmutableList.of("b"), ""))
+ .addEqualityGroup(of(ImmutableList.of("c"), ""))
+ .addEqualityGroup(of(ImmutableList.<String>of(), ""))
+ .addEqualityGroup(of(ImmutableList.<String>of(), "", null, COMPILE_ONE_DEPENDENCY))
+ .addEqualityGroup(of(ImmutableList.<String>of(), "", emptyTestFilter(), BUILD_TESTS_ONLY))
+ .addEqualityGroup(of(ImmutableList.<String>of(), "", emptyTestFilter(), DETERMINE_TESTS))
+ .testEquals();
+ }
+
+ private TargetPatternList of(ImmutableList<String> targetPatterns, String offset,
+ @Nullable TestFilter testFilter, Flag... flags) {
+ ImmutableSet<Flag> set = ImmutableSet.copyOf(flags);
+ boolean compileOneDependency = set.contains(Flag.COMPILE_ONE_DEPENDENCY);
+ boolean buildTestsOnly = set.contains(Flag.BUILD_TESTS_ONLY);
+ boolean determineTests = set.contains(Flag.DETERMINE_TESTS);
+ return new TargetPatternList(targetPatterns, offset, compileOneDependency, buildTestsOnly,
+ determineTests, testFilter);
+ }
+
+ private TargetPatternList of(ImmutableList<String> targetPatterns, String offset) {
+ return of(targetPatterns, offset, null);
+ }
+
+ private TestFilter emptyTestFilter() {
+ LoadingOptions options = Options.getDefaults(LoadingOptions.class);
+ return TestFilter.forOptions(options, NullEventHandler.INSTANCE, ImmutableSet.<String>of());
+ }
+
+ @Test
+ public void testNull() throws Exception {
+ new NullPointerTester()
+ .testAllPublicConstructors(TargetPatternList.class);
+ }
+}
