Change "Want-Code-Review" to require Code-Review>=1 rather than MAX
If Code-Review=MAX is required from all reviewers the change becomes
unsubmittable if a reviewer is added that cannot vote with
Code-Review=MAX. Hence only require Code-Review>=1 from all reviewers.
This is OK since we still have the "Code-Review" submit requirement that
requires Code-Review=MAX approvals.
Bug: Google b/368978825
Release-Notes: skip
Change-Id: I6957b753217254857e9f51250ec16f2d7f104ee3
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/Documentation/config-submit-requirements.txt b/Documentation/config-submit-requirements.txt
index 7d870a0..fe8da51 100644
--- a/Documentation/config-submit-requirements.txt
+++ b/Documentation/config-submit-requirements.txt
@@ -649,10 +649,11 @@
[[require-code-review-approvals-from-all-human-reviewers-example]]
=== Require Code-Review approvals from all human reviewers
-The following submit requirement requires a 'Code-Review+MAX' approval
-from all human reviewers of the change. Votes from service users
-(members of the link:access-control.html#service_users[Service Users]
-group) and the change owner are ignored.
+The following submit requirement requires a 'Code-Review' approval
+('Code-Review+1' or 'Code-Review+2') from all human reviewers of the
+change. Votes from service users (members of the
+link:access-control.html#service_users[Service Users] group) and the
+change owner are ignored.
The 'applicableIf' condition makes this submit requirement show up in
the UI only if it is not satisfied.
@@ -663,11 +664,11 @@
----
[submit-requirement "Want-Code-Review-From-All"]
- description = A maximum 'Code-Review' vote is required from all \
- human reviewers (service users that are reviewers \
- are ignored).
- applicableIf = -label:Code-Review=MAX,users=human_reviewers
- submittableIf = label:Code-Review=MAX,users=human_reviewers
+ description = A 'Code-Review' vote is required from all human \
+ reviewers (service users that are reviewers are \
+ ignored).
+ applicableIf = -label:Code-Review>=1,users=human_reviewers
+ submittableIf = label:Code-Review>=1,users=human_reviewers
----
It is possible to configure the 'Want-Code-Review-From-All' submit
@@ -681,11 +682,11 @@
----
[submit-requirement "Want-Code-Review-From-All"]
- description = A maximum 'Code-Review' vote is required from all \
- human reviewers (service users that are reviewers \
- are ignored).
- applicableIf = footer:\"Want-Code-Review: all\" -label:Code-Review=MAX,users=human_reviewers
- submittableIf = label:Code-Review=MAX,users=human_reviewers
+ description = A 'Code-Review' vote is required from all human \
+ reviewers (service users that are reviewers are \
+ ignored).
+ applicableIf = footer:\"Want-Code-Review: all\" -label:Code-Review>=1,users=human_reviewers
+ submittableIf = label:Code-Review>=1,users=human_reviewers
----
For more information about the "users=human_reviewers" arg see
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index 6f95fc1..d7d2f26 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -637,7 +637,7 @@
}
@Test
- public void submitRequirement_wantCodeReviewMaxHumanReviewers() throws Exception {
+ public void submitRequirement_wantCodeReviewFromHumanReviewers() throws Exception {
projectOperations
.project(project)
.forUpdate()
@@ -659,9 +659,9 @@
.setApplicabilityExpression(
Optional.of(
SubmitRequirementExpression.create(
- "-label:Code-Review=MAX,users=human_reviewers")))
+ "-label:Code-Review>=1,users=human_reviewers")))
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("label:Code-Review=MAX,users=human_reviewers"))
+ SubmitRequirementExpression.create("label:Code-Review>=1,users=human_reviewers"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -740,10 +740,34 @@
"Want-Code-Review-From-All",
Status.NOT_APPLICABLE,
/* isLegacy= */ false);
+
+ // Add another reviewer
+ TestAccount reviewer3 = accountCreator.create("reviewer3");
+ gApi.changes().id(changeId).addReviewer("reviewer3");
+
+ // Want-Code-Review-From-All is unsatisfied because reviewer3 didn't vote yet.
+ assertSubmitRequirementStatus(
+ gApi.changes().id(changeId).get().submitRequirements,
+ "Want-Code-Review-From-All",
+ Status.UNSATISFIED,
+ /* isLegacy= */ false);
+
+ // Vote with Code-Review+1 by reviewer3.
+ requestScopeOperations.setApiUser(reviewer3.id());
+ voteLabel(changeId, "Code-Review", 1);
+
+ // Want-Code-Review-From-All is not applicable because all reviewers voted with Code-Review >=
+ // 1.
+ assertSubmitRequirementStatus(
+ gApi.changes().id(changeId).get().submitRequirements,
+ "Want-Code-Review-From-All",
+ Status.NOT_APPLICABLE,
+ /* isLegacy= */ false);
}
@Test
- public void submitRequirement_wantCodeReviewMaxHumanReviewers_enabledByFooter() throws Exception {
+ public void submitRequirement_wantCodeReviewFromHumanReviewers_enabledByFooter()
+ throws Exception {
projectOperations
.project(project)
.forUpdate()
@@ -765,9 +789,9 @@
.setApplicabilityExpression(
Optional.of(
SubmitRequirementExpression.create(
- "footer:\"Want-Code-Review: all\" -label:Code-Review=MAX,users=human_reviewers")))
+ "footer:\"Want-Code-Review: all\" -label:Code-Review>=1,users=human_reviewers")))
.setSubmittabilityExpression(
- SubmitRequirementExpression.create("label:Code-Review=MAX,users=human_reviewers"))
+ SubmitRequirementExpression.create("label:Code-Review>=1,users=human_reviewers"))
.setAllowOverrideInChildProjects(false)
.build());
@@ -833,6 +857,29 @@
"Want-Code-Review-From-All",
Status.NOT_APPLICABLE,
/* isLegacy= */ false);
+
+ // Add another reviewer
+ TestAccount reviewer3 = accountCreator.create("reviewer3");
+ gApi.changes().id(changeId).addReviewer("reviewer3");
+
+ // Want-Code-Review-From-All is unsatisfied because reviewer3 didn't vote yet.
+ assertSubmitRequirementStatus(
+ gApi.changes().id(changeId).get().submitRequirements,
+ "Want-Code-Review-From-All",
+ Status.UNSATISFIED,
+ /* isLegacy= */ false);
+
+ // Vote with Code-Review+1 by reviewer3.
+ requestScopeOperations.setApiUser(reviewer3.id());
+ voteLabel(changeId, "Code-Review", 1);
+
+ // Want-Code-Review-From-All is not applicable because all reviewers voted with Code-Review >=
+ // 1.
+ assertSubmitRequirementStatus(
+ gApi.changes().id(changeId).get().submitRequirements,
+ "Want-Code-Review-From-All",
+ Status.NOT_APPLICABLE,
+ /* isLegacy= */ false);
}
@Test
