Show that the overrideIf of a configured Code-Owners SR has no effect
Whether a change is code-owner approved is checked by the (hard-coded)
code owners submit rule. This submit rule respects overrides that comply
with the label that is configured as override in the code-owners plugin
configuration.
If a Code-Owners submit requirement is manually configured in
project.config it conflicts with the (hard-coded) code owners submit
rule. If submit requirements conflict the change only gets submittable
when both submit requirements pass.
This means if the Code-Owners submit requirement has a different
overrideIf condition than what's configured in the code-owners plugin
configuration it has no effect:
1. If the overrideIf is satisfied it doesn't count as an override for
the (hard-coded) code owners submit rule so this submit rule is still
not passing and hence the change doesn't get submittable.
2. If the override that is configured in the code-owners plugin
configuration is satisfied the (hard-coded) code owners submit rule
is passing. If the Code-Owners submit requirement uses
"has:approval_code-owners" as submittableIf condition it is passing
now too since "has:approval_code-owners" just means to check if the
(hard-coded) code owners submit rule passes.
This means the overrideIf condition of a configured Code-Owners submit
requirement can neither replace nor add to the override condition that
is configured in the code-owners plugin configuration.
This also means defining a Code-Owners submit requirement that has an
overrideIf condition that is different from the override condition that
is configured in the code-owners plugin configuration has no effect and
is only confusing as the overrideIf condition from the Code-Owners
submit requirement is shown in the UI but doesn't work.
Change-Id: I5bfe4a45ad4e2ec67fae5ec3681012537e7b6a63
Signed-off-by: Edwin Kempin <ekempin@google.com>
Reviewed-on: https://gerrit-review.googlesource.com/c/plugins/code-owners/+/412560
Reviewed-by: Patrick Hiesel <hiesel@google.com>
Tested-by: Zuul <zuul-63@gerritcodereview-ci.iam.gserviceaccount.com>
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerSubmitRuleIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerSubmitRuleIT.java
index 618f5df..e078738 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerSubmitRuleIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerSubmitRuleIT.java
@@ -724,6 +724,55 @@
}
@Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void cannotDefineOverrideWithCodeOwnersSubmitRequirement() throws Exception {
+ // Create the Owners-Override label that is configured as override for the Code-Owners submit
+ // requirement.
+ createOwnersOverrideLabel();
+
+ // Configure a Code-Owners submit requirement with Other-Override+1 as override (note this is a
+ // different label than the one that is configured as override in the code-owners plugin
+ // configuration).
+ createOwnersOverrideLabel("Other-Override");
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "Code-Owners";
+ input.submittabilityExpression = "has:approval_code-owners";
+ input.overrideExpression = "label:Other-Override+1";
+ gApi.projects().name(project.get()).submitRequirement("Code-Owners").create(input);
+
+ // Create a change and approve the Code-Review submit requirement.
+ PushOneCommit.Result r = createChange("Some Change", "foo.txt", "some content");
+ String changeId = r.getChangeId();
+ approve(changeId);
+
+ // Check that the change is not submittable since the Code-Owners submit rule is not satisfied
+ // yet.
+ assertThat(gApi.changes().id(changeId).get().submittable).isFalse();
+
+ // Apply the override that is configured in the Code-Owners submit requirement and see that it
+ // has no effect (since the hard-coded code-owners submit rule is not overridden by it).
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Other-Override", 1));
+ assertThat(gApi.changes().id(changeId).get().submittable).isFalse();
+
+ // Apply the override that is configured in the code-owners plugin configuration and see that it
+ // makes the change submittable.
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+ assertThat(gApi.changes().id(changeId).get().submittable).isTrue();
+
+ // Remove the override that is configured in the Code-Owners submit requirement and see that it
+ // has no effect and the change is still submittable (this is because the code-owners submit
+ // rule is fulfilled now and hence the submittableIf condition of the Code-Owners submit
+ // requirement passes).
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Other-Override", 0));
+ assertThat(gApi.changes().id(changeId).get().submittable).isTrue();
+
+ // Removing the override that is configured in the code-owners plugin configuration makes the
+ // change again not submittable.
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 0));
+ assertThat(gApi.changes().id(changeId).get().submittable).isFalse();
+ }
+
+ @Test
public void submitRuleIsNotInvokedWhenQueryingChange() throws Exception {
PushOneCommit.Result r = createChange("Some Change", "foo.txt", "some content");
String changeId = r.getChangeId();
