Merge changes Iebe401f1,I84338990,I6d073eb9
* changes:
Document how to measure the test coverage
Warn when a code owner config directly imports itself
Extend change message when code owner override is applied
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/BackendModule.java b/java/com/google/gerrit/plugins/codeowners/backend/BackendModule.java
index 50e5f68..3bdb518 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/BackendModule.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/BackendModule.java
@@ -47,7 +47,8 @@
install(new CodeOwnerSubmitRule.Module());
DynamicSet.bind(binder(), ExceptionHook.class).to(CodeOwnersExceptionHook.class);
- DynamicSet.bind(binder(), OnPostReview.class).to(CodeOwnersOnPostReview.class);
+ DynamicSet.bind(binder(), OnPostReview.class).to(OnCodeOwnerApproval.class);
+ DynamicSet.bind(binder(), OnPostReview.class).to(OnCodeOwnerOverride.class);
DynamicSet.bind(binder(), ReviewerAddedListener.class).to(CodeOwnersOnAddReviewer.class);
}
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
similarity index 97%
rename from java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java
rename to java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
index 1596e23..e818163 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnersOnPostReview.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerApproval.java
@@ -35,7 +35,8 @@
import java.util.stream.Stream;
/**
- * Callback that is invoked on post review.
+ * Callback that is invoked on post review and that extends the change message if a code owner
+ * approval was changed.
*
* <p>If a code owner approval was added, removed or changed, include in the change message that is
* being posted on vote, which of the files:
@@ -47,14 +48,14 @@
* </ul>
*/
@Singleton
-class CodeOwnersOnPostReview implements OnPostReview {
+class OnCodeOwnerApproval implements OnPostReview {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
private final CodeOwnerApprovalCheck codeOwnerApprovalCheck;
@Inject
- CodeOwnersOnPostReview(
+ OnCodeOwnerApproval(
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
CodeOwnerApprovalCheck codeOwnerApprovalCheck) {
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
new file mode 100644
index 0000000..df24fbb
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
@@ -0,0 +1,167 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners.backend;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableSet;
+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;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.restapi.change.OnPostReview;
+import com.google.gerrit.server.util.LabelVote;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * Callback that is invoked on post review and that extends the change message if a code owner
+ * override was changed.
+ *
+ * <p>If a code owner override was added, removed or changed, include in the change message that is
+ * being posted on vote, that the vote is a code owner override to let users know about its effect.
+ */
+@Singleton
+class OnCodeOwnerOverride implements OnPostReview {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
+
+ @Inject
+ OnCodeOwnerOverride(CodeOwnersPluginConfiguration codeOwnersPluginConfiguration) {
+ this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
+ }
+
+ @Override
+ public Optional<String> getChangeMessageAddOn(
+ IdentifiedUser user,
+ ChangeNotes changeNotes,
+ PatchSet patchSet,
+ Map<String, Short> oldApprovals,
+ Map<String, Short> approvals) {
+ if (codeOwnersPluginConfiguration.isDisabled(changeNotes.getChange().getDest())) {
+ return Optional.empty();
+ }
+
+ // code owner overrides are only relevant for the current patch set
+ if (!changeNotes.getChange().currentPatchSetId().equals(patchSet.id())) {
+ return Optional.empty();
+ }
+
+ ImmutableSet<RequiredApproval> overrideApprovals =
+ codeOwnersPluginConfiguration.getOverrideApproval(changeNotes.getProjectName());
+
+ List<String> messages = new ArrayList<>();
+ for (RequiredApproval overrideApproval : overrideApprovals) {
+ if (oldApprovals.get(overrideApproval.labelType().getName()) == null) {
+ // If oldApprovals doesn't contain the label or if the labels value in it is null, the label
+ // was not changed.
+ continue;
+ }
+
+ buildMessageForCodeOwnerOverride(
+ user, changeNotes, patchSet, oldApprovals, approvals, overrideApproval)
+ .ifPresent(messages::add);
+ }
+
+ if (messages.isEmpty()) {
+ return Optional.empty();
+ }
+ return Optional.of(Joiner.on("\n\n").join(messages));
+ }
+
+ private Optional<String> buildMessageForCodeOwnerOverride(
+ IdentifiedUser user,
+ ChangeNotes changeNotes,
+ PatchSet patchSet,
+ Map<String, Short> oldApprovals,
+ Map<String, Short> approvals,
+ RequiredApproval overrideApproval) {
+ LabelVote newVote = getNewVote(overrideApproval, approvals);
+
+ if (isCodeOwnerOverrideNewlyApplied(overrideApproval, oldApprovals, newVote)) {
+ return Optional.of(
+ String.format(
+ "By voting %s the code-owners submit requirement is overridden by %s",
+ newVote, user.getName()));
+ } else if (isCodeOwnerOverrideRemoved(overrideApproval, oldApprovals, newVote)) {
+ if (newVote.value() == 0) {
+ return Optional.of(
+ String.format(
+ "By removing the %s vote the code-owners submit requirement is no longer overridden"
+ + " by %s",
+ newVote.label(), user.getName()));
+ }
+ return Optional.of(
+ String.format(
+ "By voting %s the code-owners submit requirement is no longer overridden by %s",
+ newVote, user.getName()));
+ } else if (isCodeOwnerOverrideUpOrDowngraded(overrideApproval, oldApprovals, newVote)) {
+ return Optional.of(
+ String.format(
+ "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);
+ return Optional.empty();
+ }
+
+ private boolean isCodeOwnerOverrideNewlyApplied(
+ RequiredApproval requiredApproval, Map<String, Short> oldApprovals, LabelVote newVote) {
+ String labelName = requiredApproval.labelType().getName();
+ return oldApprovals.get(labelName) < requiredApproval.value()
+ && newVote.value() >= requiredApproval.value();
+ }
+
+ private boolean isCodeOwnerOverrideRemoved(
+ RequiredApproval requiredApproval, Map<String, Short> oldApprovals, LabelVote newVote) {
+ String labelName = requiredApproval.labelType().getName();
+ return oldApprovals.get(labelName) >= requiredApproval.value()
+ && newVote.value() < requiredApproval.value();
+ }
+
+ private boolean isCodeOwnerOverrideUpOrDowngraded(
+ RequiredApproval requiredApproval, Map<String, Short> oldApprovals, LabelVote newVote) {
+ String labelName = requiredApproval.labelType().getName();
+ return oldApprovals.get(labelName) >= requiredApproval.value()
+ && newVote.value() >= requiredApproval.value();
+ }
+
+ private LabelVote getNewVote(RequiredApproval requiredApproval, Map<String, Short> approvals) {
+ String labelName = requiredApproval.labelType().getName();
+ checkState(
+ approvals.containsKey(labelName),
+ "expected that approval on label %s exists (approvals = %s)",
+ labelName,
+ approvals);
+ return LabelVote.create(labelName, approvals.get(labelName));
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index e4714bd..cef6d52 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -855,6 +855,14 @@
PathCodeOwners.createKeyForImportedCodeOwnerConfig(
keyOfImportingCodeOwnerConfig, codeOwnerConfigReference);
+ if (isSelfImport(keyOfImportingCodeOwnerConfig, keyOfImportedCodeOwnerConfig)) {
+ return nonResolvableImport(
+ importType,
+ codeOwnerConfigFilePath,
+ "code owner config imports itself",
+ ValidationMessage.Type.WARNING);
+ }
+
Optional<ProjectState> projectState = projectCache.get(keyOfImportedCodeOwnerConfig.project());
if (!projectState.isPresent() || !isProjectReadable(keyOfImportedCodeOwnerConfig)) {
// we intentionally use the same error message for non-existing and non-readable projects so
@@ -944,6 +952,21 @@
return Optional.empty();
}
+ /** Whether the importing code owner config is the same as the imported code owner config. */
+ private boolean isSelfImport(
+ CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig,
+ CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig) {
+ return keyOfImportingCodeOwnerConfig.project().equals(keyOfImportedCodeOwnerConfig.project())
+ && keyOfImportingCodeOwnerConfig.ref().equals(keyOfImportedCodeOwnerConfig.ref())
+ && codeOwnersPluginConfiguration
+ .getBackend(keyOfImportingCodeOwnerConfig.branchNameKey())
+ .getFilePath(keyOfImportingCodeOwnerConfig)
+ .equals(
+ codeOwnersPluginConfiguration
+ .getBackend(keyOfImportedCodeOwnerConfig.branchNameKey())
+ .getFilePath(keyOfImportedCodeOwnerConfig));
+ }
+
private boolean isProjectReadable(CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig) {
try {
return permissionBackend
@@ -994,14 +1017,26 @@
CodeOwnerConfigImportType importType,
Path codeOwnerConfigFilePath,
String message) {
+ return nonResolvableImport(
+ importType,
+ codeOwnerConfigFilePath,
+ message,
+ codeOwnersPluginConfiguration.rejectNonResolvableImports(project)
+ ? ValidationMessage.Type.ERROR
+ : ValidationMessage.Type.WARNING);
+ }
+
+ private Optional<CommitValidationMessage> nonResolvableImport(
+ CodeOwnerConfigImportType importType,
+ Path codeOwnerConfigFilePath,
+ String message,
+ ValidationMessage.Type validationMessageType) {
return Optional.of(
new CommitValidationMessage(
String.format(
"invalid %s import in '%s': %s",
importType.getType(), codeOwnerConfigFilePath, message),
- codeOwnersPluginConfiguration.rejectNonResolvableImports(project)
- ? ValidationMessage.Type.ERROR
- : ValidationMessage.Type.WARNING));
+ validationMessageType));
}
private Optional<CommitValidationMessage> nonResolvableCodeOwner(
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
index 12b3360..3dfa9db 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
@@ -1023,6 +1023,102 @@
}
@Test
+ public void uploadConfigWithGlobalSelfImportReportsAWarning() throws Exception {
+ testUploadConfigWithSelfImport(CodeOwnerConfigImportType.GLOBAL);
+ }
+
+ @Test
+ public void uploadConfigWithPerFileSelfImportReportsAWarning() throws Exception {
+ testUploadConfigWithSelfImport(CodeOwnerConfigImportType.PER_FILE);
+ }
+
+ private void testUploadConfigWithSelfImport(CodeOwnerConfigImportType importType)
+ throws Exception {
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
+
+ // create a code owner config that imports itself
+ CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
+ CodeOwnerConfigReference codeOwnerConfigReference =
+ CodeOwnerConfigReference.builder(
+ CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY,
+ codeOwnerConfigOperations
+ .codeOwnerConfig(keyOfImportingCodeOwnerConfig)
+ .getFilePath())
+ .setProject(project)
+ .build();
+ CodeOwnerConfig codeOwnerConfig =
+ createCodeOwnerConfigWithImport(
+ keyOfImportingCodeOwnerConfig, importType, codeOwnerConfigReference);
+
+ PushOneCommit.Result r =
+ createChange(
+ "Add code owners",
+ codeOwnerConfigOperations
+ .codeOwnerConfig(keyOfImportingCodeOwnerConfig)
+ .getJGitFilePath(),
+ format(codeOwnerConfig));
+ assertOkWithWarnings(
+ r,
+ "invalid code owner config files",
+ String.format(
+ "invalid %s import in '%s': code owner config imports itself",
+ importType.getType(),
+ codeOwnerConfigOperations
+ .codeOwnerConfig(keyOfImportingCodeOwnerConfig)
+ .getFilePath()));
+ }
+
+ @Test
+ public void canUploadConfigWithGlobalImportOfFileWithExtensionFromSameFolder() throws Exception {
+ testUploadConfigWithImportOfFileWithExtensionFromSameFolder(CodeOwnerConfigImportType.GLOBAL);
+ }
+
+ @Test
+ public void canUploadConfigWithPerFileImportOfFileWithExtensionFromSameFolder() throws Exception {
+ testUploadConfigWithImportOfFileWithExtensionFromSameFolder(CodeOwnerConfigImportType.PER_FILE);
+ }
+
+ private void testUploadConfigWithImportOfFileWithExtensionFromSameFolder(
+ CodeOwnerConfigImportType importType) throws Exception {
+ skipTestIfImportsNotSupportedByCodeOwnersBackend();
+
+ // create a code owner config that imports a code owner config from the same folder but with an
+ // extension in the file name
+ CodeOwnerConfig.Key keyOfImportingCodeOwnerConfig = createCodeOwnerConfigKey("/");
+ CodeOwnerConfig.Key keyOfImportedCodeOwnerConfig =
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .fileName(getCodeOwnerConfigFileName() + "_extension")
+ .addCodeOwnerEmail(user.email())
+ .create();
+ GitUtil.fetch(testRepo, "refs/*:refs/*");
+ testRepo.reset(projectOperations.project(project).getHead("master"));
+ CodeOwnerConfigReference codeOwnerConfigReference =
+ CodeOwnerConfigReference.builder(
+ CodeOwnerConfigImportMode.GLOBAL_CODE_OWNER_SETS_ONLY,
+ codeOwnerConfigOperations
+ .codeOwnerConfig(keyOfImportedCodeOwnerConfig)
+ .getFilePath())
+ .setProject(project)
+ .build();
+ CodeOwnerConfig codeOwnerConfig =
+ createCodeOwnerConfigWithImport(
+ keyOfImportingCodeOwnerConfig, importType, codeOwnerConfigReference);
+
+ PushOneCommit.Result r =
+ createChange(
+ "Add code owners",
+ codeOwnerConfigOperations
+ .codeOwnerConfig(keyOfImportingCodeOwnerConfig)
+ .getJGitFilePath(),
+ format(codeOwnerConfig));
+ r.assertOkStatus();
+ }
+
+ @Test
public void cannotUploadConfigWithGlobalImportFromNonExistingProject() throws Exception {
testUploadConfigWithImportFromNonExistingProject(CodeOwnerConfigImportType.GLOBAL);
}
@@ -1764,6 +1860,16 @@
return ObjectIds.abbreviateName(id, testRepo.getRevWalk().getObjectReader());
}
+ private String getCodeOwnerConfigFileName() {
+ CodeOwnerBackend backend = backendConfig.getDefaultBackend();
+ if (backend instanceof FindOwnersBackend) {
+ return FindOwnersBackend.CODE_OWNER_CONFIG_FILE_NAME;
+ } else if (backend instanceof ProtoBackend) {
+ return ProtoBackend.CODE_OWNER_CONFIG_FILE_NAME;
+ }
+ throw new IllegalStateException("unknown code owner backend: " + backend.getClass().getName());
+ }
+
private static void assertOkWithoutMessages(PushOneCommit.Result pushResult) {
pushResult.assertOkStatus();
pushResult.assertNotMessage("fatal");
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
similarity index 99%
rename from javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
rename to javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
index 9bae548..97f426a 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersOnPostReviewIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerApprovalIT.java
@@ -38,9 +38,9 @@
import org.junit.Test;
/**
- * Acceptance test for {@code com.google.gerrit.plugins.codeowners.backend.CodeOwnersOnPostReview}.
+ * Acceptance test for {@code com.google.gerrit.plugins.codeowners.backend.OnCodeOwnerApproval}.
*/
-public class CodeOwnersOnPostReviewIT extends AbstractCodeOwnersIT {
+public class OnCodeOwnerApprovalIT extends AbstractCodeOwnersIT {
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private ProjectOperations projectOperations;
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java
new file mode 100644
index 0000000..f094f7b
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/OnCodeOwnerOverrrideIT.java
@@ -0,0 +1,437 @@
+// Copyright (C) 2021 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners.acceptance.api;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
+import com.google.gerrit.extensions.common.ChangeMessageInfo;
+import com.google.gerrit.extensions.common.LabelDefinitionInput;
+import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
+import com.google.inject.Inject;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.regex.Pattern;
+import org.junit.Test;
+
+/** Acceptance test for {@code com.google.gerrit.plugins.codeowners.backend.OnCodeOwnerOverride}. */
+public class OnCodeOwnerOverrrideIT extends AbstractCodeOwnersIT {
+ @Inject private RequestScopeOperations requestScopeOperations;
+ @Inject private ProjectOperations projectOperations;
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.disabled", value = "true")
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageNotExtendedIfCodeOwnersFuctionalityIsDisabled() throws Exception {
+ createOwnersOverrideLabel();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Owners-Override+1");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnersOverrideIsApplied() throws Exception {
+ createOwnersOverrideLabel();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "By voting Owners-Override+1 the code-owners submit requirement is overridden by %s\n",
+ admin.fullName()));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnersOverrideIsReApplied() throws Exception {
+ createOwnersOverrideLabel();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ int messageCount = gApi.changes().id(changeId).get().messages.size();
+
+ // Apply the Owners-Override+1 approval again
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ // Check that a new change message was added.
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(messages.size()).isEqualTo(messageCount + 1);
+
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "By voting Owners-Override+1 the code-owners submit requirement is still overridden by %s\n",
+ admin.fullName()));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnersOverrideIsAppliedByOtherUser() throws Exception {
+ createOwnersOverrideLabel();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "By voting Owners-Override+1 the code-owners submit requirement is overridden by %s\n",
+ admin.fullName()));
+
+ // Apply the Owners-Override+1 approval by another user
+ requestScopeOperations.setApiUser(user.id());
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "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 changeMessageExtendedIfCodeOwnersOverrideIsUpgraded() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+2", "Override", "+1", "Override", " 0", "No Override");
+ gApi.projects().name(project.get()).label("Owners-Override").create(input).get();
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(0, 2)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ // Upgrade the approval from Owners-Override+1 to Owners-Override+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(
+ String.format(
+ "Patch Set 1: Owners-Override+2\n\n"
+ + "By voting Owners-Override+2 the code-owners submit requirement is still"
+ + " overridden by %s\n",
+ admin.fullName()));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnersOverrideIsDowngraded() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+2", "Override", "+1", "Override", " 0", "No Override");
+ gApi.projects().name(project.get()).label("Owners-Override").create(input).get();
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(0, 2)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 2));
+
+ // Downgrade the approval from Owners-Override+2 to Owners-Override+1
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "By voting Owners-Override+1 the code-owners submit requirement is still"
+ + " overridden by %s\n",
+ admin.fullName()));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnersOverrideIsRemoved() throws Exception {
+ createOwnersOverrideLabel();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ // Remove the override approval
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 0));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: -Owners-Override\n\n"
+ + "By removing the Owners-Override vote the code-owners submit requirement is"
+ + " no longer overridden by %s\n",
+ admin.fullName()));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnersOverrideIsChangedToNegativeValue() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Override", " 0", "No Override", "-1", "No Override");
+ gApi.projects().name(project.get()).label("Owners-Override").create(input).get();
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Owners-Override")
+ .range(-1, 1)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ // Vote with Owners-Override-1
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", -1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override-1\n\n"
+ + "By voting Owners-Override-1 the code-owners submit requirement is no longer"
+ + " overridden by %s\n",
+ admin.fullName()));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageNotExtendedIfNonCodeOwnersOverrideIsApplied() throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+1", "Approval", " 0", "No Approval");
+ gApi.projects().name(project.get()).label("Other").create(input).get();
+
+ // Allow to vote on the Owners-Override label.
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ TestProjectUpdate.allowLabel("Other")
+ .range(0, 1)
+ .ref("refs/*")
+ .group(REGISTERED_USERS)
+ .build())
+ .update();
+
+ String changeId = createChange().getChangeId();
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Other", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Other+1");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnersOverrideIsAppliedTogetherWithComment()
+ throws Exception {
+ createOwnersOverrideLabel();
+
+ String path = "foo/bar.baz";
+ String changeId = createChange("Test Change", path, "file content").getChangeId();
+
+ ReviewInput.CommentInput commentInput = new ReviewInput.CommentInput();
+ commentInput.line = 1;
+ commentInput.message = "some comment";
+ commentInput.path = path;
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.labels = new HashMap<>();
+ reviewInput.labels.put("Owners-Override", (short) 1);
+ reviewInput.comments = new HashMap<>();
+ reviewInput.comments.put(commentInput.path, Lists.newArrayList(commentInput));
+ gApi.changes().id(changeId).current().review(reviewInput);
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "(1 comment)\n\n"
+ + "By voting Owners-Override+1 the code-owners submit requirement is"
+ + " overridden by %s\n",
+ admin.fullName()));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageNotExtendedIfCodeOwnersOverrideIsAppliedOnOldPatchSet()
+ throws Exception {
+ createOwnersOverrideLabel();
+
+ String changeId = createChange().getChangeId();
+
+ // create a second patch set
+ amendChange(changeId);
+
+ // vote on the first patch set
+ gApi.changes().id(changeId).revision(1).review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message).isEqualTo("Patch Set 1: Owners-Override+1");
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfDestinationBranchWasDeleted() throws Exception {
+ createOwnersOverrideLabel();
+
+ String branchName = "tempBranch";
+ createBranch(BranchNameKey.create(project, branchName));
+
+ String changeId = createChange("refs/for/" + branchName).getChangeId();
+
+ DeleteBranchesInput input = new DeleteBranchesInput();
+ input.branches = ImmutableList.of(branchName);
+ gApi.projects().name(project.get()).deleteBranches(input);
+
+ gApi.changes().id(changeId).current().review(new ReviewInput().label("Owners-Override", 1));
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .isEqualTo(
+ String.format(
+ "Patch Set 1: Owners-Override+1\n\n"
+ + "By voting Owners-Override+1 the code-owners submit requirement is overridden by %s\n",
+ admin.fullName()));
+ }
+
+ @Test
+ @GerritConfig(
+ name = "plugin.code-owners.overrideApproval",
+ values = {"Owners-Override+1", "Global-Override+1"})
+ public void changeMessageExtendedIfMultipleCodeOwnersOverridesAreAppliedTogether()
+ throws Exception {
+ createOwnersOverrideLabel();
+ createOwnersOverrideLabel("Global-Override");
+
+ String changeId = createChange().getChangeId();
+
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.labels = new HashMap<>();
+ reviewInput.labels.put("Owners-Override", (short) 1);
+ reviewInput.labels.put("Global-Override", (short) 1);
+ gApi.changes().id(changeId).current().review(reviewInput);
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message)
+ .matches(
+ Pattern.quote("Patch Set 1: ")
+ + "("
+ + Pattern.quote("Owners-Override+1 Global-Override+1")
+ + "|"
+ + Pattern.quote("Global-Override+1 Owners-Override+1")
+ + ")"
+ + Pattern.quote(
+ String.format(
+ "\n\nBy voting Owners-Override+1 the code-owners submit requirement is"
+ + " overridden by %s\n\n"
+ + "By voting Global-Override+1 the code-owners submit requirement is"
+ + " overridden by %s\n",
+ admin.fullName(), admin.fullName())));
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void changeMessageExtendedIfCodeOwnersOverrideAndCodeOwnerApprovalAreAppliedTogether()
+ throws Exception {
+ createOwnersOverrideLabel();
+
+ String path = "foo/bar.baz";
+ String changeId = createChange("Test Change", path, "file content").getChangeId();
+
+ ReviewInput reviewInput = new ReviewInput();
+ reviewInput.labels = new HashMap<>();
+ reviewInput.labels.put("Owners-Override", (short) 1);
+ reviewInput.labels.put("Code-Review", (short) 1);
+ gApi.changes().id(changeId).current().review(reviewInput);
+
+ Collection<ChangeMessageInfo> messages = gApi.changes().id(changeId).get().messages;
+ assertThat(Iterables.getLast(messages).message).startsWith("Patch Set 1: ");
+ assertThat(Iterables.getLast(messages).message)
+ .containsMatch(
+ "("
+ + Pattern.quote("Owners-Override+1 Code-Review+1")
+ + "|"
+ + Pattern.quote("Code-Review+1 Owners-Override+1")
+ + ")");
+ assertThat(Iterables.getLast(messages).message)
+ .contains(
+ String.format(
+ "By voting Owners-Override+1 the code-owners submit requirement is"
+ + " overridden by %s\n",
+ admin.fullName()));
+ assertThat(Iterables.getLast(messages).message)
+ .contains(
+ String.format(
+ "By voting Code-Review+1 the following files are now code-owner approved by"
+ + " %s:\n"
+ + "* %s\n",
+ admin.fullName(), path));
+ }
+}
diff --git a/resources/Documentation/build.md b/resources/Documentation/build.md
index 6c2db04..38d44ed 100644
--- a/resources/Documentation/build.md
+++ b/resources/Documentation/build.md
@@ -22,6 +22,14 @@
```
bazel test //plugins/@PLUGIN@/...
```
+\
+To measure the test coverage run:
+
+```
+ bazel coverage --test_output=all plugins/code-owners/... --coverage_report_generator=@bazel_tools//tools/test:coverage_report_generator --combined_report=lcov --instrumentation_filter="^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/acceptance[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/api[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/api/impl[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/backend[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/common[/:],,^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/metrics[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/restapi[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/testing[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/util[/:],^//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/validation[/:]" && genhtml -o . --ignore-errors source bazel-out/_coverage/_coverage_report.dat
+```
+\
+and then open the generated `index.html` in a browser.
---
