Add tests for checkers that have an invalid checker UUID in the config

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ib44498d0804f5a2acd8dc47a14234913cfdfaf7b
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 9c5eed6..50106e6 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImpl.java
@@ -17,6 +17,7 @@
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.stream.Collectors.toList;
 import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
 
 import com.google.common.base.Splitter;
@@ -42,11 +43,11 @@
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
+import java.util.stream.Stream;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.BlobBasedConfig;
@@ -251,36 +252,16 @@
       Optional<Checker> checker = getChecker(checkerUuid);
       checkState(checker.isPresent(), "Tried to invalidate a non-existing test checker");
 
+      if (testCheckerInvalidation.invalidUuid()) {
+        setValueInCheckerConfig("uuid", "invalid");
+      }
+
       if (testCheckerInvalidation.invalidBlockingCondition()) {
-        try (Repository repo = repoManager.openRepository(allProjectsName)) {
-          TestRepository<Repository> testRepo = new TestRepository<>(repo);
-          Config checkerConfig =
-              readConfig(testRepo, checkerUuid.toRefName(), CheckerConfig.CHECKER_CONFIG_FILE);
-          List<String> blocking =
-              new ArrayList<>(
-                  Arrays.asList(checkerConfig.getStringList("checker", null, "blocking")));
-          blocking.add("invalid");
-          checkerConfig.setStringList("checker", null, "blocking", blocking);
-          testRepo
-              .branch(checkerUuid.toRefName())
-              .commit()
-              .add(CheckerConfig.CHECKER_CONFIG_FILE, checkerConfig.toText())
-              .create();
-        }
+        addValueInCheckerConfig("blocking", "invalid");
       }
 
       if (testCheckerInvalidation.invalidStatus()) {
-        try (Repository repo = repoManager.openRepository(allProjectsName)) {
-          TestRepository<Repository> testRepo = new TestRepository<>(repo);
-          Config checkerConfig =
-              readConfig(testRepo, checkerUuid.toRefName(), CheckerConfig.CHECKER_CONFIG_FILE);
-          checkerConfig.setString("checker", null, "status", "invalid");
-          testRepo
-              .branch(checkerUuid.toRefName())
-              .commit()
-              .add(CheckerConfig.CHECKER_CONFIG_FILE, checkerConfig.toText())
-              .create();
-        }
+        setValueInCheckerConfig("status", "invalid");
       }
 
       if (testCheckerInvalidation.nonParseableConfig()) {
@@ -303,6 +284,36 @@
       }
     }
 
+    private void setValueInCheckerConfig(String key, String value) throws Exception {
+      updateCheckerConfig(cfg -> cfg.setString("checker", null, key, value));
+    }
+
+    private void addValueInCheckerConfig(String key, String value) throws Exception {
+      updateCheckerConfig(
+          cfg ->
+              cfg.setStringList(
+                  "checker",
+                  null,
+                  key,
+                  Streams.concat(
+                          Arrays.stream(cfg.getStringList("checker", null, key)), Stream.of(value))
+                      .collect(toList())));
+    }
+
+    private void updateCheckerConfig(Consumer<Config> configUpdater) throws Exception {
+      try (Repository repo = repoManager.openRepository(allProjectsName)) {
+        TestRepository<Repository> testRepo = new TestRepository<>(repo);
+        Config checkerConfig =
+            readConfig(testRepo, checkerUuid.toRefName(), CheckerConfig.CHECKER_CONFIG_FILE);
+        configUpdater.accept(checkerConfig);
+        testRepo
+            .branch(checkerUuid.toRefName())
+            .commit()
+            .add(CheckerConfig.CHECKER_CONFIG_FILE, checkerConfig.toText())
+            .create();
+      }
+    }
+
     private CheckerUpdate toCheckerUpdate(TestCheckerUpdate checkerUpdate) {
       CheckerUpdate.Builder builder = CheckerUpdate.builder();
       checkerUpdate.name().ifPresent(builder::setName);
diff --git a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java
index b08bbd0..b5a9f02 100644
--- a/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java
+++ b/java/com/google/gerrit/plugins/checks/acceptance/testsuite/TestCheckerInvalidation.java
@@ -21,6 +21,8 @@
 public abstract class TestCheckerInvalidation {
   public abstract boolean nonParseableConfig();
 
+  public abstract boolean invalidUuid();
+
   public abstract boolean invalidBlockingCondition();
 
   public abstract boolean invalidStatus();
@@ -33,6 +35,7 @@
     return new AutoValue_TestCheckerInvalidation.Builder()
         .checkerInvalidator(checkerInvalidator)
         .nonParseableConfig(false)
+        .invalidUuid(false)
         .invalidBlockingCondition(false)
         .invalidStatus(false)
         .deleteRef(false);
@@ -46,6 +49,12 @@
 
     abstract Builder nonParseableConfig(boolean nonParseableConfig);
 
+    public Builder invalidUuid() {
+      return invalidUuid(true);
+    }
+
+    abstract Builder invalidUuid(boolean invalidUuid);
+
     public Builder invalidBlockingCondition() {
       return invalidBlockingCondition(true);
     }
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 226c2aa..c4afb78 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/GetCheckerIT.java
@@ -212,6 +212,16 @@
   }
 
   @Test
+  public void getCheckerWithInvalidUuidFails() throws Exception {
+    CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+    checkerOperations.checker(checkerUuid).forInvalidation().invalidUuid().invalidate();
+
+    exception.expect(RestApiException.class);
+    exception.expectMessage("Cannot retrieve checker " + checkerUuid);
+    getCheckerInfo(checkerUuid);
+  }
+
+  @Test
   public void getCheckerWithInvalidBlockingConditionFails() throws Exception {
     CheckerUuid checkerUuid = checkerOperations.newChecker().create();
     checkerOperations
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 516f35a..64e9c17 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/ListCheckersIT.java
@@ -91,17 +91,19 @@
     CheckerUuid invalidCheckerUuid1 = checkerOperations.newChecker().create();
     CheckerUuid invalidCheckerUuid2 = checkerOperations.newChecker().create();
     CheckerUuid invalidCheckerUuid3 = checkerOperations.newChecker().create();
+    CheckerUuid invalidCheckerUuid4 = checkerOperations.newChecker().create();
     checkerOperations
         .checker(invalidCheckerUuid1)
         .forInvalidation()
         .nonParseableConfig()
         .invalidate();
+    checkerOperations.checker(invalidCheckerUuid2).forInvalidation().invalidUuid().invalidate();
     checkerOperations
-        .checker(invalidCheckerUuid2)
+        .checker(invalidCheckerUuid3)
         .forInvalidation()
         .invalidBlockingCondition()
         .invalidate();
-    checkerOperations.checker(invalidCheckerUuid3).forInvalidation().invalidStatus().invalidate();
+    checkerOperations.checker(invalidCheckerUuid4).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/testsuite/CheckerOperationsImplTest.java b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
index 4a9584f..9023ef6 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/testsuite/CheckerOperationsImplTest.java
@@ -513,6 +513,22 @@
   }
 
   @Test
+  public void uuidCanBeMadeInvalid() throws Exception {
+    CheckerUuid checkerUuid = checkerOperations.newChecker().create();
+
+    checkerOperations.checker(checkerUuid).forInvalidation().invalidUuid().invalidate();
+
+    try {
+      checkers.getChecker(checkerUuid);
+      assert_().fail("expected ConfigInvalidException");
+    } catch (ConfigInvalidException e) {
+      // expected
+      assertThat(e.getMessage()).contains("value of checker.uuid");
+      assertThat(e.getMessage()).contains("does not match expected checker UUID");
+    }
+  }
+
+  @Test
   public void blockingConditionsCanBeMadeInvalid() throws Exception {
     CheckerUuid checkerUuid = checkerOperations.newChecker().create();