Merge "CheckSubmitRule: Return Optional#empty() if all checkers are optional"
diff --git a/java/com/google/gerrit/plugins/checks/Checks.java b/java/com/google/gerrit/plugins/checks/Checks.java
index 9ae249c..ab3caad 100644
--- a/java/com/google/gerrit/plugins/checks/Checks.java
+++ b/java/com/google/gerrit/plugins/checks/Checks.java
@@ -92,6 +92,20 @@
throws IOException, StorageException;
/**
+ * Returns true if all checkers for the project are optional for submitting the change identified
+ * by the {@code patchSetId} parameter, that is all checkers are either disabled or not required
+ * for submission.
+ *
+ * @param projectName the name of the project.
+ * @param patchSetId the ID of the patch set.
+ * @return
+ * @throws IOException if failed to check if all required checks have passed.
+ * @throws StorageException if failed to check if all required checks have passed.
+ */
+ boolean areAllCheckersOptionalForSubmit(Project.NameKey projectName, PatchSet.Id patchSetId)
+ throws IOException, StorageException;
+
+ /**
* Computes an ETag for the checks of the given change.
*
* @param projectName the name of the project that contains the change
diff --git a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
index 44f3d70..0c69c26 100644
--- a/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
+++ b/java/com/google/gerrit/plugins/checks/db/NoteDbChecks.java
@@ -157,6 +157,13 @@
}
@Override
+ public boolean areAllCheckersOptionalForSubmit(
+ Project.NameKey projectName, PatchSet.Id patchSetId) throws IOException, StorageException {
+ return checkers.checkersOf(projectName).stream()
+ .allMatch(checker -> !isRequiredForSubmit(checker, patchSetId.changeId()));
+ }
+
+ @Override
public String getETag(Project.NameKey projectName, Change.Id changeId) throws IOException {
try (Repository repo = repoManager.openRepository(projectName);
RevWalk rw = new RevWalk(repo)) {
diff --git a/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java b/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
index bc1db28..de6bf49 100644
--- a/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
+++ b/java/com/google/gerrit/plugins/checks/rules/ChecksSubmitRule.java
@@ -75,6 +75,9 @@
boolean areAllRequiredCheckersPassing;
try {
+ if (checks.areAllCheckersOptionalForSubmit(project, currentPatchSetId)) {
+ return Optional.empty();
+ }
areAllRequiredCheckersPassing =
checks.areAllRequiredCheckersPassing(project, currentPatchSetId);
} catch (IOException e) {
diff --git a/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java b/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
index d96c7b6..affb4c6 100644
--- a/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
+++ b/javatests/com/google/gerrit/plugins/checks/rules/ChecksSubmitRuleTest.java
@@ -52,6 +52,7 @@
Checks checks = mock(Checks.class);
when(checks.areAllRequiredCheckersPassing(any(), any()))
.thenThrow(new IOException("Fail for test"));
+ when(checks.areAllCheckersOptionalForSubmit(any(), any())).thenReturn(false);
ChecksSubmitRule checksSubmitRule = new ChecksSubmitRule(checks);
@@ -72,6 +73,30 @@
assertErrorRecord(submitRecords, "failed to evaluate check states for change 1");
}
+ @Test
+ public void returnsEmptyOptionalIfAllCheckersAreOptional() throws Exception {
+ Checks checks = mock(Checks.class);
+ when(checks.areAllCheckersOptionalForSubmit(any(), any())).thenReturn(true);
+
+ ChecksSubmitRule checksSubmitRule = new ChecksSubmitRule(checks);
+
+ ChangeData cd = mock(ChangeData.class);
+ Change.Id changeId = Change.id(1);
+ when(cd.project()).thenReturn(Project.nameKey("My-Project"));
+ when(cd.getId()).thenReturn(Change.id(1));
+ when(cd.currentPatchSet())
+ .thenReturn(
+ PatchSet.builder()
+ .id(PatchSet.id(changeId, 1))
+ .commitId(ObjectId.zeroId())
+ .uploader(Account.id(1000))
+ .createdOn(TimeUtil.nowTs())
+ .build());
+
+ Optional<SubmitRecord> submitRecord = checksSubmitRule.evaluate(cd);
+ assertThat(submitRecord).isEmpty();
+ }
+
private static void assertErrorRecord(
Optional<SubmitRecord> submitRecord, String expectedErrorMessage) {
assertThat(submitRecord).isPresent();
