Have separate APIs for updating and invalidating test checkers
There are many ways how a checker can be invalid (non-parseable config,
missing checker ref, invalid blocking condition, invalid status etc.).
Adding all these possibilities to the TestCheckerUpdate API pollutes
this API. Instead add a separate API which is dedicated to making
checkers invalid. Follow-up changes will add futher conditions that make
a checker invalid.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ice51f8c6a81195639f676c34766bbb94c90e5a50
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperations.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperations.java
index 2cbc9ab..849d9ff 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperations.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperations.java
@@ -164,5 +164,23 @@
* @return a builder to update the checker
*/
TestCheckerUpdate.Builder forUpdate();
+
+ /**
+ * Starts the fluent chain to invalidate a checker. The returned builder can be used to specify
+ * how the checker should be made invalid. To invalidate the checker for real, {@link
+ * TestCheckerInvalidation.Builder#invalidate()} must be called.
+ *
+ * <p>Example:
+ *
+ * <pre>
+ * checkerOperations.forInvalidation().invalidConfig().invalidate();
+ * </pre>
+ *
+ * <p><strong>Note:</strong> The invalidation will fail with an exception if the checker to
+ * invalidate doesn't exist.
+ *
+ * @return a builder to invalidate the checker
+ */
+ TestCheckerInvalidation.Builder forInvalidation();
}
}
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
index e3ff24e..9c5eed6 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
@@ -239,18 +239,19 @@
private void updateChecker(TestCheckerUpdate testCheckerUpdate) throws Exception {
CheckerUpdate checkerUpdate = toCheckerUpdate(testCheckerUpdate);
checkersUpdate.get().updateChecker(checkerUuid, checkerUpdate);
+ }
- if (testCheckerUpdate.forceInvalidConfig()) {
- try (Repository repo = repoManager.openRepository(allProjectsName)) {
- new TestRepository<>(repo)
- .branch(checkerUuid.toRefName())
- .commit()
- .add(CheckerConfig.CHECKER_CONFIG_FILE, "invalid-config")
- .create();
- }
- }
+ @Override
+ public TestCheckerInvalidation.Builder forInvalidation() {
+ return TestCheckerInvalidation.builder(this::invalidateChecker);
+ }
- if (testCheckerUpdate.forceInvalidBlockingCondition()) {
+ private void invalidateChecker(TestCheckerInvalidation testCheckerInvalidation)
+ throws Exception {
+ Optional<Checker> checker = getChecker(checkerUuid);
+ checkState(checker.isPresent(), "Tried to invalidate a non-existing test checker");
+
+ if (testCheckerInvalidation.invalidBlockingCondition()) {
try (Repository repo = repoManager.openRepository(allProjectsName)) {
TestRepository<Repository> testRepo = new TestRepository<>(repo);
Config checkerConfig =
@@ -268,7 +269,7 @@
}
}
- if (testCheckerUpdate.forceInvalidStatus()) {
+ if (testCheckerInvalidation.invalidStatus()) {
try (Repository repo = repoManager.openRepository(allProjectsName)) {
TestRepository<Repository> testRepo = new TestRepository<>(repo);
Config checkerConfig =
@@ -282,7 +283,17 @@
}
}
- if (testCheckerUpdate.deleteRef()) {
+ if (testCheckerInvalidation.nonParseableConfig()) {
+ try (Repository repo = repoManager.openRepository(allProjectsName)) {
+ new TestRepository<>(repo)
+ .branch(checkerUuid.toRefName())
+ .commit()
+ .add(CheckerConfig.CHECKER_CONFIG_FILE, "non-parseable-config")
+ .create();
+ }
+ }
+
+ if (testCheckerInvalidation.deleteRef()) {
try (Repository repo = repoManager.openRepository(allProjectsName)) {
RefUpdate ru =
new TestRepository<>(repo).getRepository().updateRef(checkerUuid.toRefName(), true);
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java
new file mode 100644
index 0000000..b08bbd0
--- /dev/null
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java
@@ -0,0 +1,78 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// 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.gerrit.plugins.checks.acceptance.testsuite;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.acceptance.testsuite.ThrowingConsumer;
+
+@AutoValue
+public abstract class TestCheckerInvalidation {
+ public abstract boolean nonParseableConfig();
+
+ public abstract boolean invalidBlockingCondition();
+
+ public abstract boolean invalidStatus();
+
+ public abstract boolean deleteRef();
+
+ abstract ThrowingConsumer<TestCheckerInvalidation> checkerInvalidator();
+
+ public static Builder builder(ThrowingConsumer<TestCheckerInvalidation> checkerInvalidator) {
+ return new AutoValue_TestCheckerInvalidation.Builder()
+ .checkerInvalidator(checkerInvalidator)
+ .nonParseableConfig(false)
+ .invalidBlockingCondition(false)
+ .invalidStatus(false)
+ .deleteRef(false);
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public Builder nonParseableConfig() {
+ return nonParseableConfig(true);
+ }
+
+ abstract Builder nonParseableConfig(boolean nonParseableConfig);
+
+ public Builder invalidBlockingCondition() {
+ return invalidBlockingCondition(true);
+ }
+
+ abstract Builder invalidBlockingCondition(boolean invalidBlockingCondition);
+
+ public Builder invalidStatus() {
+ return invalidStatus(true);
+ }
+
+ abstract Builder invalidStatus(boolean invalidStatus);
+
+ public Builder deleteRef() {
+ return deleteRef(true);
+ }
+
+ abstract Builder deleteRef(boolean deleteRef);
+
+ abstract Builder checkerInvalidator(
+ ThrowingConsumer<TestCheckerInvalidation> checkerInvalidator);
+
+ abstract TestCheckerInvalidation autoBuild();
+
+ /** Executes the checker invalidation as specified. */
+ public void invalidate() {
+ TestCheckerInvalidation checkerInvalidation = autoBuild();
+ checkerInvalidation.checkerInvalidator().acceptAndThrowSilently(checkerInvalidation);
+ }
+ }
+}
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 99f6f92..1fd5409 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerUpdate.java
@@ -43,23 +43,10 @@
public abstract Optional<String> query();
- public abstract boolean forceInvalidConfig();
-
- public abstract boolean forceInvalidBlockingCondition();
-
- public abstract boolean forceInvalidStatus();
-
- public abstract boolean deleteRef();
-
abstract ThrowingConsumer<TestCheckerUpdate> checkerUpdater();
public static Builder builder(ThrowingConsumer<TestCheckerUpdate> checkerUpdater) {
- return new AutoValue_TestCheckerUpdate.Builder()
- .checkerUpdater(checkerUpdater)
- .forceInvalidConfig(false)
- .forceInvalidBlockingCondition(false)
- .forceInvalidStatus(false)
- .deleteRef(false);
+ return new AutoValue_TestCheckerUpdate.Builder().checkerUpdater(checkerUpdater);
}
@AutoValue.Builder
@@ -91,30 +78,6 @@
return status(CheckerStatus.DISABLED);
}
- public Builder forceInvalidConfig() {
- return forceInvalidConfig(true);
- }
-
- abstract Builder forceInvalidConfig(boolean forceInvalidConfig);
-
- public Builder forceInvalidBlockingCondition() {
- return forceInvalidBlockingCondition(true);
- }
-
- abstract Builder forceInvalidBlockingCondition(boolean forceInvalidBlockingCondition);
-
- public Builder forceInvalidStatus() {
- return forceInvalidStatus(true);
- }
-
- abstract Builder forceInvalidStatus(boolean forceInvalidStatus);
-
- public Builder deleteRef() {
- return deleteRef(true);
- }
-
- abstract Builder deleteRef(boolean deleteRef);
-
public abstract Builder blockingConditions(
ImmutableSortedSet<BlockingCondition> blockingConditions);
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java
index d351813..51c2ca4 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/CheckersIT.java
@@ -39,7 +39,7 @@
public void checkersOfOmitsInvalidCheckers() throws Exception {
CheckerUuid checkerUuid1 = checkerOperations.newChecker().repository(project).create();
CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
- checkerOperations.checker(checkerUuid1).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid1).forInvalidation().nonParseableConfig().invalidate();
assertThat(getCheckerUuidsOf(project)).containsExactly(checkerUuid2);
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
index 9c68e13..1fc7c8a 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/CreateCheckIT.java
@@ -206,7 +206,7 @@
@Test
public void cannotCreateCheckForInvalidChecker() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
CheckInput input = new CheckInput();
input.checkerUuid = checkerUuid.get();
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 cb0da5e..6145e3c 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckIT.java
@@ -288,7 +288,7 @@
CheckKey checkKey = CheckKey.create(project, patchSetId, checkerUuid);
checkOperations.newCheck(checkKey).setState(CheckState.RUNNING).upsert();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
CheckInfo check = getCheckInfo(patchSetId, checkerUuid, ListChecksOption.CHECKER);
assertThat(check).isNotNull();
@@ -364,7 +364,7 @@
public void getCheckForInvalidChecker() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
@@ -374,7 +374,7 @@
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
assertCheckNotFound(patchSetId, checkerUuid);
}
@@ -416,7 +416,7 @@
public void getCheckForNonExistingChecker() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- checkerOperations.checker(checkerUuid).forUpdate().deleteRef().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().deleteRef().invalidate();
assertThat(getCheckInfo(patchSetId, checkerUuid)).isNotNull();
}
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 0d7baf9..226c2aa 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
@@ -204,7 +204,7 @@
@Test
public void getInvalidCheckerFails() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
exception.expect(RestApiException.class);
exception.expectMessage("Cannot retrieve checker " + checkerUuid);
@@ -214,7 +214,11 @@
@Test
public void getCheckerWithInvalidBlockingConditionFails() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidBlockingCondition().update();
+ checkerOperations
+ .checker(checkerUuid)
+ .forInvalidation()
+ .invalidBlockingCondition()
+ .invalidate();
exception.expect(RestApiException.class);
exception.expectMessage("Cannot retrieve checker " + checkerUuid);
@@ -224,7 +228,7 @@
@Test
public void getCheckerWithInvalidStatusFails() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidStatus().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().invalidStatus().invalidate();
exception.expect(RestApiException.class);
exception.expectMessage("Cannot retrieve checker " + checkerUuid);
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
index f52257e..516f35a 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
@@ -91,13 +91,17 @@
CheckerUuid invalidCheckerUuid1 = checkerOperations.newChecker().create();
CheckerUuid invalidCheckerUuid2 = checkerOperations.newChecker().create();
CheckerUuid invalidCheckerUuid3 = checkerOperations.newChecker().create();
- checkerOperations.checker(invalidCheckerUuid1).forUpdate().forceInvalidConfig().update();
+ checkerOperations
+ .checker(invalidCheckerUuid1)
+ .forInvalidation()
+ .nonParseableConfig()
+ .invalidate();
checkerOperations
.checker(invalidCheckerUuid2)
- .forUpdate()
- .forceInvalidBlockingCondition()
- .update();
- checkerOperations.checker(invalidCheckerUuid3).forUpdate().forceInvalidStatus().update();
+ .forInvalidation()
+ .invalidBlockingCondition()
+ .invalidate();
+ checkerOperations.checker(invalidCheckerUuid3).forInvalidation().invalidStatus().invalidate();
List<CheckerInfo> allCheckers = checkersApi.all();
assertThat(allCheckers).containsExactly(checkerOperations.checker(checkerUuid).asInfo());
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
index 807a499..b320ada 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListChecksIT.java
@@ -153,7 +153,7 @@
CheckerUuid checkerUuid2 = checkerOperations.newChecker().repository(project).create();
checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid1)).upsert();
checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid2)).upsert();
- checkerOperations.checker(checkerUuid2).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid2).forInvalidation().nonParseableConfig().invalidate();
List<CheckInfo> checks = checksApiFactory.revision(patchSetId).list(ListChecksOption.CHECKER);
@@ -211,7 +211,7 @@
public void listIncludesCheckFromInvalidChecker() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
@@ -220,7 +220,7 @@
public void listIncludesCheckFromNonExistingChecker() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
checkOperations.newCheck(CheckKey.create(project, patchSetId, checkerUuid)).upsert();
- checkerOperations.checker(checkerUuid).forUpdate().deleteRef().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().deleteRef().invalidate();
assertThat(checksApiFactory.revision(patchSetId).list()).hasSize(1);
}
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
index 0a58a79..3116ee6 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
@@ -513,7 +513,7 @@
@Test
public void queryPendingChecksForInvalidCheckerFails() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
exception.expect(RestApiException.class);
exception.expectMessage("Cannot query pending checks");
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
index 6e4dd2e..2d97826 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/UpdateCheckIT.java
@@ -228,7 +228,7 @@
@Test
public void canUpdateCheckForNonExistingChecker() throws Exception {
- checkerOperations.checker(checkKey.checkerUuid()).forUpdate().deleteRef().update();
+ checkerOperations.checker(checkKey.checkerUuid()).forInvalidation().deleteRef().invalidate();
CheckInput input = new CheckInput();
input.state = CheckState.SUCCESSFUL;
@@ -239,7 +239,11 @@
@Test
public void canUpdateCheckForInvalidChecker() throws Exception {
- checkerOperations.checker(checkKey.checkerUuid()).forUpdate().forceInvalidConfig().update();
+ checkerOperations
+ .checker(checkKey.checkerUuid())
+ .forInvalidation()
+ .nonParseableConfig()
+ .invalidate();
CheckInput input = new CheckInput();
input.state = CheckState.SUCCESSFUL;
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 e256e35..4a9584f 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
@@ -502,7 +502,7 @@
public void configCanBeMadeInvalid() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidConfig().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().nonParseableConfig().invalidate();
try {
checkers.getChecker(checkerUuid);
@@ -516,7 +516,11 @@
public void blockingConditionsCanBeMadeInvalid() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidBlockingCondition().update();
+ checkerOperations
+ .checker(checkerUuid)
+ .forInvalidation()
+ .invalidBlockingCondition()
+ .invalidate();
try {
checkers.getChecker(checkerUuid);
@@ -531,7 +535,7 @@
public void statusCanBeMadeInvalid() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- checkerOperations.checker(checkerUuid).forUpdate().forceInvalidStatus().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().invalidStatus().invalidate();
try {
checkers.getChecker(checkerUuid);
@@ -546,7 +550,7 @@
public void refCanBeDeleted() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().create();
- checkerOperations.checker(checkerUuid).forUpdate().deleteRef().update();
+ checkerOperations.checker(checkerUuid).forInvalidation().deleteRef().invalidate();
try (Repository allProjectsRepo = repoManager.openRepository(allProjects)) {
assertThat(allProjectsRepo.exactRef(checkerUuid.toRefName())).isNull();
