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. ---