Checks test API: Allow easier specification of required/optional
Using blocking conditions instead of a required flag is a mechanism
which will allow us quite some flexibility in the future. However,
most tests don't need that flexibility. They only need the notion
of a required or optional checker - even if we extend the blocking
conditions in the future. Hence, introduce those much easier concepts
to the test API. In addition, adjust tests where reasonable to this
different notion. This should also keep those tests more stable in the
future.
Change-Id: Ic443771994931aa4596f11e7839d3a1a33ad27b3
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerCreation.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerCreation.java
index 298d0c9..044bf30 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerCreation.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerCreation.java
@@ -14,9 +14,6 @@
package com.google.gerrit.plugins.checks.acceptance.testsuite;
-import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
-import static java.util.Comparator.naturalOrder;
-
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSortedSet;
import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
@@ -24,9 +21,7 @@
import com.google.gerrit.plugins.checks.api.BlockingCondition;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
import com.google.gerrit.reviewdb.client.Project;
-import java.util.Arrays;
import java.util.Optional;
-import java.util.stream.Stream;
@AutoValue
public abstract class TestCheckerCreation {
@@ -82,19 +77,17 @@
abstract Builder status(CheckerStatus status);
- public abstract Builder blockingConditions(
- ImmutableSortedSet<BlockingCondition> blockingConditions);
-
- public Builder blockingConditions(BlockingCondition first, BlockingCondition... rest) {
- return blockingConditions(
- Stream.concat(Stream.of(first), Arrays.stream(rest))
- .collect(toImmutableSortedSet(naturalOrder())));
- }
-
- public Builder clearBlockingConditions() {
+ public Builder optional() {
return blockingConditions(ImmutableSortedSet.of());
}
+ public Builder required() {
+ return blockingConditions(ImmutableSortedSet.of(BlockingCondition.STATE_NOT_PASSING));
+ }
+
+ public abstract Builder blockingConditions(
+ ImmutableSortedSet<BlockingCondition> blockingConditions);
+
public abstract Builder query(String query);
public Builder clearQuery() {
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java
index 063911b..80c247c 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java
@@ -14,18 +14,13 @@
package com.google.gerrit.plugins.checks.acceptance.testsuite;
-import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
-import static java.util.Comparator.naturalOrder;
-
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSortedSet;
import com.google.gerrit.acceptance.testsuite.ThrowingConsumer;
import com.google.gerrit.plugins.checks.api.BlockingCondition;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
import com.google.gerrit.reviewdb.client.Project;
-import java.util.Arrays;
import java.util.Optional;
-import java.util.stream.Stream;
@AutoValue
public abstract class TestCheckerUpdate {
@@ -78,19 +73,17 @@
abstract Builder status(CheckerStatus status);
- public abstract Builder blockingConditions(
- ImmutableSortedSet<BlockingCondition> blockingConditions);
-
- public Builder blockingConditions(BlockingCondition first, BlockingCondition... rest) {
- return blockingConditions(
- Stream.concat(Stream.of(first), Arrays.stream(rest))
- .collect(toImmutableSortedSet(naturalOrder())));
- }
-
- public Builder clearBlockingConditions() {
+ public Builder optional() {
return blockingConditions(ImmutableSortedSet.of());
}
+ public Builder required() {
+ return blockingConditions(ImmutableSortedSet.of(BlockingCondition.STATE_NOT_PASSING));
+ }
+
+ public abstract Builder blockingConditions(
+ ImmutableSortedSet<BlockingCondition> blockingConditions);
+
public abstract Builder query(String query);
public Builder clearQuery() {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChangeCheckInfoIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChangeCheckInfoIT.java
index d153f51..8f84423 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChangeCheckInfoIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ChangeCheckInfoIT.java
@@ -17,7 +17,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
-import static com.google.gerrit.plugins.checks.api.BlockingCondition.STATE_NOT_PASSING;
import static com.google.gerrit.truth.CacheStatsSubject.assertThat;
import static com.google.gerrit.truth.CacheStatsSubject.cloneStats;
@@ -94,11 +93,7 @@
public void combinedCheckStateViaGet() throws Exception {
CheckerUuid optionalCheckerUuid = checkerOperations.newChecker().repository(project).create();
CheckerUuid requiredCheckerUuid =
- checkerOperations
- .newChecker()
- .repository(project)
- .blockingConditions(STATE_NOT_PASSING)
- .create();
+ checkerOperations.newChecker().repository(project).required().create();
assertThat(getChangeCheckInfo(changeId))
.hasValue(new ChangeCheckInfo("checks", CombinedCheckState.IN_PROGRESS));
checkOperations
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
index d9e5ff7..a19359c 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
@@ -35,7 +35,6 @@
import com.google.gerrit.plugins.checks.ListChecksOption;
import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerTestData;
-import com.google.gerrit.plugins.checks.api.BlockingCondition;
import com.google.gerrit.plugins.checks.api.CheckInfo;
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.plugins.checks.api.CheckerStatus;
@@ -265,18 +264,14 @@
@Test
public void getCheckReturnsBlockingConditionsOnlyForCheckerOption() throws Exception {
CheckerUuid checkerUuid =
- checkerOperations
- .newChecker()
- .repository(project)
- .blockingConditions(BlockingCondition.STATE_NOT_PASSING)
- .create();
+ checkerOperations.newChecker().repository(project).required().create();
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).upsert();
assertThat(getCheckInfo(patchSetId, checkerUuid).blocking).isNull();
assertThat(getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER).blocking)
- .containsExactly(BlockingCondition.STATE_NOT_PASSING);
+ .isNotEmpty();
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
index 4a1b052..7a9c811 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
@@ -127,16 +127,15 @@
CheckerUuid checkerUuid =
checkerOperations
.newChecker()
- .blockingConditions(BlockingCondition.STATE_NOT_PASSING)
+ .blockingConditions(ImmutableSortedSet.of(BlockingCondition.STATE_NOT_PASSING))
.create();
assertThat(getCheckerInfo(checkerUuid).blocking)
.containsExactly(BlockingCondition.STATE_NOT_PASSING);
}
@Test
- public void getCheckerWithoutBlockingCondition() throws Exception {
- CheckerUuid checkerUuid =
- checkerOperations.newChecker().blockingConditions(ImmutableSortedSet.of()).create();
+ public void getOptionalChecker() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().optional().create();
assertThat(getCheckerInfo(checkerUuid).blocking).isEmpty();
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/db/GetCombinedCheckStateIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/db/GetCombinedCheckStateIT.java
index b0cb2a2..c09b870 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/db/GetCombinedCheckStateIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/db/GetCombinedCheckStateIT.java
@@ -22,7 +22,6 @@
import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
import com.google.gerrit.plugins.checks.acceptance.testsuite.CheckerTestData;
import com.google.gerrit.plugins.checks.acceptance.testsuite.TestCheckerCreation;
-import com.google.gerrit.plugins.checks.api.BlockingCondition;
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.plugins.checks.api.CombinedCheckState;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -113,11 +112,7 @@
}
private TestCheckerCreation.Builder newRequiredChecker() {
- return checkerOperations
- .newChecker()
- .repository(project)
- .enable()
- .blockingConditions(BlockingCondition.STATE_NOT_PASSING);
+ return checkerOperations.newChecker().repository(project).enable().required();
}
private void setCheckSuccessful(CheckerUuid checkerUuid) {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/rules/ChecksSubmitRuleIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/rules/ChecksSubmitRuleIT.java
index fadd0f9..7d26c1f 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/rules/ChecksSubmitRuleIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/rules/ChecksSubmitRuleIT.java
@@ -24,7 +24,6 @@
import com.google.gerrit.plugins.checks.CheckerUuid;
import com.google.gerrit.plugins.checks.acceptance.AbstractCheckersTest;
import com.google.gerrit.plugins.checks.acceptance.testsuite.TestCheckerCreation;
-import com.google.gerrit.plugins.checks.api.BlockingCondition;
import com.google.gerrit.plugins.checks.api.CheckState;
import com.google.gerrit.reviewdb.client.PatchSet;
import org.junit.Before;
@@ -144,11 +143,7 @@
// }
private TestCheckerCreation.Builder newRequiredChecker() {
- return checkerOperations
- .newChecker()
- .repository(project)
- .enable()
- .blockingConditions(BlockingCondition.STATE_NOT_PASSING);
+ return checkerOperations.newChecker().repository(project).enable().required();
}
private void postCheckResult(CheckerUuid checkerUuid, CheckState checkState) {
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
index a9e237d..65b4fb8 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
@@ -15,6 +15,7 @@
package com.google.gerrit.plugins.checks.acceptance.testsuite;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.common.truth.Truth.assert_;
import static com.google.common.truth.Truth8.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -182,20 +183,16 @@
}
@Test
- public void specifiedBlockingConditionsAreRespectedForCheckerCreation() throws Exception {
- CheckerUuid checkerUuid =
- checkerOperations
- .newChecker()
- .blockingConditions(BlockingCondition.STATE_NOT_PASSING)
- .create();
+ public void requiredCheckerCanBeCreated() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().required().create();
CheckerInfo checker = getCheckerFromServer(checkerUuid);
- assertThat(checker.blocking).containsExactly(BlockingCondition.STATE_NOT_PASSING);
+ assertThat(checker.blocking).isNotEmpty();
}
@Test
- public void requestingNoBlockingConditionsIsPossibleForCheckerCreation() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().clearBlockingConditions().create();
+ public void optionalCheckerCanBeCreated() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().optional().create();
CheckerInfo checker = getCheckerFromServer(checkerUuid);
assertThat(checker.blocking).isEmpty();
@@ -350,11 +347,11 @@
}
@Test
- public void blockConditionsOfExistingCheckerCanBeRetrieved() throws Exception {
+ public void blockingConditionsOfExistingCheckerCanBeRetrieved() throws Exception {
CheckerUuid checkerUuid =
checkerOperations
.newChecker()
- .blockingConditions(BlockingCondition.STATE_NOT_PASSING)
+ .blockingConditions(ImmutableSortedSet.of(BlockingCondition.STATE_NOT_PASSING))
.create();
ImmutableSortedSet<BlockingCondition> blockingConditions =
@@ -364,6 +361,24 @@
}
@Test
+ public void requiredStateOfExistingCheckerCanBeRetrieved() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().required().create();
+
+ boolean required = checkerOperations.checker(checkerUuid).get().isRequired();
+
+ assertWithMessage("checker.required()").that(required).isTrue();
+ }
+
+ @Test
+ public void optionalStateOfExistingCheckerCanBeRetrieved() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().optional().create();
+
+ boolean required = checkerOperations.checker(checkerUuid).get().isRequired();
+
+ assertWithMessage("checker.required()").that(required).isFalse();
+ }
+
+ @Test
public void emptyQueryOfExistingCheckerCanBeRetrieved() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().clearQuery().create();
@@ -464,19 +479,24 @@
}
@Test
- public void blockingConditionsCanBeUpdated() throws Exception {
- CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- assertThat(checkerOperations.checker(checkerUuid).asInfo().blocking).isEmpty();
+ public void requiredCheckerCanBeMadeOptional() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().required().create();
- checkerOperations
- .checker(checkerUuid)
- .forUpdate()
- .blockingConditions(BlockingCondition.STATE_NOT_PASSING)
- .update();
- assertThat(checkerOperations.checker(checkerUuid).asInfo().blocking)
- .containsExactly(BlockingCondition.STATE_NOT_PASSING);
- checkerOperations.checker(checkerUuid).forUpdate().clearBlockingConditions().update();
- assertThat(checkerOperations.checker(checkerUuid).asInfo().blocking).isEmpty();
+ checkerOperations.checker(checkerUuid).forUpdate().optional().update();
+
+ assertWithMessage("checker.required()")
+ .that(checkerOperations.checker(checkerUuid).get().isRequired())
+ .isFalse();
+ }
+
+ @Test
+ public void optionalCheckerCanBeMadeRequired() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().optional().create();
+
+ checkerOperations.checker(checkerUuid).forUpdate().required().update();
+ assertWithMessage("checker.required()")
+ .that(checkerOperations.checker(checkerUuid).get().isRequired())
+ .isTrue();
}
@Test
