OnCodeOwnerApproval/OnCodeOwnerOverride: Remove wrong error log
We wrongly throught that this code was unreachable and that reaching it
would be an error. Add a test case that hits this code and drop the
wrong error log.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I3adba5cb1ba2e7e50a7d3313b40e5745075da155
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
index dff2876..1cf59ce 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
@@ -199,16 +199,7 @@
newVote, user.getName()));
}
} else {
- logger.atSevere().log(
- "code owner approval was neither newly applied, removed, upgraded nor downgraded:"
- + " project = %s, change = %s, patch set = %d, oldApprvals = %s, approvals = %s,"
- + " requiredApproval = %s",
- changeNotes.getProjectName(),
- changeNotes.getChangeId(),
- patchSet.id().get(),
- oldApprovals,
- approvals,
- requiredApproval);
+ // non-approval was downgraded (e.g. -1 to -2)
return Optional.empty();
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
index 52ca037..8a8c702 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
@@ -20,7 +20,6 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval;
@@ -44,8 +43,6 @@
*/
@Singleton
class OnCodeOwnerOverride implements OnPostReview {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
@Inject
@@ -82,8 +79,7 @@
continue;
}
- buildMessageForCodeOwnerOverride(
- user, changeNotes, patchSet, oldApprovals, approvals, overrideApproval)
+ buildMessageForCodeOwnerOverride(user, patchSet, oldApprovals, approvals, overrideApproval)
.ifPresent(messages::add);
}
@@ -95,7 +91,6 @@
private Optional<String> buildMessageForCodeOwnerOverride(
IdentifiedUser user,
- ChangeNotes changeNotes,
PatchSet patchSet,
Map<String, Short> oldApprovals,
Map<String, Short> approvals,
@@ -137,16 +132,7 @@
"By voting %s the code-owners submit requirement is still overridden by %s",
newVote, user.getName()));
}
- logger.atSevere().log(
- "code owner override was neither newly applied, removed, upgraded nor downgraded:"
- + " project = %s, change = %s, patch set = %d, oldApprvals = %s, approvals = %s,"
- + " requiredApproval = %s",
- changeNotes.getProjectName(),
- changeNotes.getChangeId(),
- patchSet.id().get(),
- oldApprovals,
- approvals,
- overrideApproval);
+ // non-approval was downgraded (e.g. -1 to -2)
return Optional.empty();
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
index 79b3924..971f8e9 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
@@ -1061,4 +1061,28 @@
+ "* %s\n",
user.fullName(), path));
}
+
+ @Test
+ public void changeMessageNotExtendedIfNonApprovalIsDowngraded() throws Exception {
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ String path = "foo/bar.baz";
+ String changeId = createChange("Test Change", path, "file content").getChangeId();
+
+ // Apply the Code-Review-1.
+ gApi.changes().id(changeId).current().review(ReviewInput.dislike());
+
+ // Change Code-Review-1 to Code-Review-2
+ gApi.changes().id(changeId).current().review(ReviewInput.reject());
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Code-Review-2");
+ }
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java
index 5f17972..1265466 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java
@@ -599,4 +599,45 @@
+ "By voting Owners-Override+1 the code-owners submit requirement is overridden by %s\n",
user.fullName()));
}
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageNotExtendedIfNonApprovalIsDowngraded() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values =
+ ImmutableMap.of("+1", "Override", " 0", "No Override", "-1", "Dislike", "-2", "Blocked");
+ gApi.projects().name(project.get()).label("Owners-Override").create(input);
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(-2, 1)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/foo/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
+ String path = "foo/bar.baz";
+ String changeId = createChange("Test Change", path, "file content").getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", -1));
+
+ // Downgrade the vote from -1 to -2.
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", -2));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Owners-Override-2");
+ }
}
