diff options
-rw-r--r-- | java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java | 10 | ||||
-rw-r--r-- | javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java | 104 |
2 files changed, 108 insertions, 6 deletions
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index df729cb63c..6f278ab712 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -570,7 +570,15 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil } if ("submittable".equalsIgnoreCase(value)) { - return new SubmittablePredicate(SubmitRecord.Status.OK); + // SubmittablePredicate will match if *any* of the submit records are OK, + // but we need to check that they're *all* OK, so check that none of the + // submit records match any of the negative cases. To avoid checking yet + // more negative cases for CLOSED and FORCED, instead make sure at least + // one submit record is OK. + return Predicate.and( + new SubmittablePredicate(SubmitRecord.Status.OK), + Predicate.not(new SubmittablePredicate(SubmitRecord.Status.NOT_READY)), + Predicate.not(new SubmittablePredicate(SubmitRecord.Status.RULE_ERROR))); } if ("ignored".equalsIgnoreCase(value)) { diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java index a704f0cd09..fcc33c9e82 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeSubmitRequirementIT.java @@ -23,6 +23,8 @@ import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitRequirement; import com.google.gerrit.extensions.annotations.Exports; +import com.google.gerrit.extensions.api.changes.ChangeApi; +import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.SubmitRequirementInfo; import com.google.gerrit.extensions.config.FactoryModule; @@ -34,7 +36,6 @@ import com.google.inject.Singleton; import java.util.ArrayList; import java.util.List; import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; import org.junit.Test; public class ChangeSubmitRequirementIT extends AbstractDaemonTest { @@ -99,20 +100,113 @@ public class ChangeSubmitRequirementIT extends AbstractDaemonTest { assertThat(result.get(0).requirements).containsExactly(reqInfo); } + @Test + public void submittableQueryRuleNotReady() throws Exception { + ChangeApi change = newChangeApi(); + + // Satisfy the default rule. + approveChange(change); + + // The custom rule is NOT_READY. + rule.block(true); + change.index(); + + assertThat(queryIsSubmittable()).isEmpty(); + } + + @Test + public void submittableQueryRuleError() throws Exception { + ChangeApi change = newChangeApi(); + + // Satisfy the default rule. + approveChange(change); + + rule.status(Optional.of(SubmitRecord.Status.RULE_ERROR)); + change.index(); + + assertThat(queryIsSubmittable()).isEmpty(); + } + + @Test + public void submittableQueryDefaultRejected() throws Exception { + ChangeApi change = newChangeApi(); + + // CodeReview:-2 the change, causing the default rule to fail. + rejectChange(change); + + rule.status(Optional.of(SubmitRecord.Status.OK)); + change.index(); + + assertThat(queryIsSubmittable()).isEmpty(); + } + + @Test + public void submittableQueryRuleOk() throws Exception { + ChangeApi change = newChangeApi(); + + // Satisfy the default rule. + approveChange(change); + + rule.status(Optional.of(SubmitRecord.Status.OK)); + change.index(); + + List<ChangeInfo> result = queryIsSubmittable(); + assertThat(result).hasSize(1); + assertThat(result.get(0).changeId).isEqualTo(change.info().changeId); + } + + @Test + public void submittableQueryRuleNoRecord() throws Exception { + ChangeApi change = newChangeApi(); + + // Satisfy the default rule. + approveChange(change); + + // Our custom rule isn't providing any submit records. + rule.status(Optional.empty()); + change.index(); + + // is:submittable should return the change, since it was approved and the custom rule is not + // blocking it. + List<ChangeInfo> result = queryIsSubmittable(); + assertThat(result).hasSize(1); + assertThat(result.get(0).changeId).isEqualTo(change.info().changeId); + } + + private List<ChangeInfo> queryIsSubmittable() throws Exception { + return gApi.changes().query("is:submittable project:" + project.get()).get(); + } + + private ChangeApi newChangeApi() throws Exception { + return gApi.changes().id(createChange().getChangeId()); + } + + private void approveChange(ChangeApi changeApi) throws Exception { + changeApi.current().review(ReviewInput.approve()); + } + + private void rejectChange(ChangeApi changeApi) throws Exception { + changeApi.current().review(ReviewInput.reject()); + } + @Singleton private static class CustomSubmitRule implements SubmitRule { - private final AtomicBoolean block = new AtomicBoolean(true); + private Optional<SubmitRecord.Status> recordStatus = Optional.empty(); public void block(boolean block) { - this.block.set(block); + this.status(block ? Optional.of(SubmitRecord.Status.NOT_READY) : Optional.empty()); + } + + public void status(Optional<SubmitRecord.Status> status) { + this.recordStatus = status; } @Override public Optional<SubmitRecord> evaluate(ChangeData changeData) { - if (block.get()) { + if (this.recordStatus.isPresent()) { SubmitRecord record = new SubmitRecord(); record.labels = new ArrayList<>(); - record.status = SubmitRecord.Status.NOT_READY; + record.status = this.recordStatus.get(); record.requirements = ImmutableList.of(req); return Optional.of(record); } |