Merge "Always show owners below file name when "Show all owners" is checked"
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
index b3a921e..af7d9b7 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheck.java
@@ -287,7 +287,7 @@
RequiredApproval requiredApproval = codeOwnersConfig.getRequiredApproval();
logger.atFine().log("requiredApproval = %s", requiredApproval);
- ImmutableSet<RequiredApproval> overrideApprovals = codeOwnersConfig.getOverrideApproval();
+ ImmutableSet<RequiredApproval> overrideApprovals = codeOwnersConfig.getOverrideApprovals();
boolean hasOverride =
hasOverride(currentPatchSetApprovals, overrideApprovals, patchSetUploader);
logger.atFine().log(
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
index db3f936..138623f 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/OnCodeOwnerOverride.java
@@ -76,7 +76,7 @@
}
ImmutableList<RequiredApproval> appliedOverrideApprovals =
- codeOwnersConfig.getOverrideApproval().stream()
+ codeOwnersConfig.getOverrideApprovals().stream()
.sorted(comparing(RequiredApproval::toString))
// If oldApprovals doesn't contain the label or if the labels value in it is null, the
// label was not changed.
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshot.java b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshot.java
index 37b286b..669b92f 100644
--- a/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshot.java
+++ b/java/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshot.java
@@ -439,7 +439,7 @@
* @return the override approvals that should be used, an empty set if no override approval is
* configured, in this case the override functionality is disabled
*/
- public ImmutableSet<RequiredApproval> getOverrideApproval() {
+ public ImmutableSet<RequiredApproval> getOverrideApprovals() {
try {
return filterOutDuplicateRequiredApprovals(
getConfiguredRequiredApproval(overrideApprovalConfig));
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
index e233344..8140fac 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
@@ -162,7 +162,7 @@
@Nullable
ImmutableList<RequiredApprovalInfo> formatOverrideApprovalInfo(Project.NameKey projectName) {
ImmutableList<RequiredApprovalInfo> overrideApprovalInfos =
- codeOwnersPluginConfiguration.getProjectConfig(projectName).getOverrideApproval().stream()
+ codeOwnersPluginConfiguration.getProjectConfig(projectName).getOverrideApprovals().stream()
.sorted(comparing(requiredApproval -> requiredApproval.toString()))
.map(CodeOwnerProjectConfigJson::formatRequiredApproval)
.collect(toImmutableList());
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersPluginConfigValidatorIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersPluginConfigValidatorIT.java
index 90f950a..aa269c7 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersPluginConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnersPluginConfigValidatorIT.java
@@ -326,11 +326,11 @@
PushResult r = pushRefsMetaConfig();
assertThat(r.getRemoteUpdate(RefNames.REFS_CONFIG).getStatus()).isEqualTo(Status.OK);
- ImmutableSet<RequiredApproval> overrideApproval =
- codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApproval();
- assertThat(overrideApproval).hasSize(1);
- assertThat(overrideApproval).element(0).hasLabelNameThat().isEqualTo("Code-Review");
- assertThat(overrideApproval).element(0).hasValueThat().isEqualTo(2);
+ ImmutableSet<RequiredApproval> overrideApprovals =
+ codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApprovals();
+ assertThat(overrideApprovals).hasSize(1);
+ assertThat(overrideApprovals).element(0).hasLabelNameThat().isEqualTo("Code-Review");
+ assertThat(overrideApprovals).element(0).hasValueThat().isEqualTo(2);
}
@Test
@@ -423,11 +423,11 @@
PushResult r = pushRefsMetaConfig();
assertThat(r.getRemoteUpdate(RefNames.REFS_CONFIG).getStatus()).isEqualTo(Status.OK);
- ImmutableSet<RequiredApproval> overrideApproval =
- codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApproval();
- assertThat(overrideApproval).hasSize(1);
- assertThat(overrideApproval).element(0).hasLabelNameThat().isEqualTo("Owners-Override");
- assertThat(overrideApproval).element(0).hasValueThat().isEqualTo(1);
+ ImmutableSet<RequiredApproval> overrideApprovals =
+ codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApprovals();
+ assertThat(overrideApprovals).hasSize(1);
+ assertThat(overrideApprovals).element(0).hasLabelNameThat().isEqualTo("Owners-Override");
+ assertThat(overrideApprovals).element(0).hasValueThat().isEqualTo(1);
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java
index 9e6fb23..f92d566 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java
@@ -254,7 +254,7 @@
@Test
public void setOverrideApproval() throws Exception {
- assertThat(codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApproval())
+ assertThat(codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApprovals())
.isEmpty();
String overrideLabel1 = "Bypass-Owners";
@@ -271,13 +271,13 @@
assertThat(updatedConfig.overrideApproval.get(0).value).isEqualTo(1);
assertThat(updatedConfig.overrideApproval.get(1).label).isEqualTo(overrideLabel2);
assertThat(updatedConfig.overrideApproval.get(1).value).isEqualTo(1);
- assertThat(codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApproval())
+ assertThat(codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApprovals())
.hasSize(2);
input.overrideApprovals = ImmutableList.of();
updatedConfig = projectCodeOwnersApiFactory.project(project).updateConfig(input);
assertThat(updatedConfig.overrideApproval).isNull();
- assertThat(codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApproval())
+ assertThat(codeOwnersPluginConfiguration.getProjectConfig(project).getOverrideApprovals())
.isEmpty();
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshotTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshotTest.java
index 9cc865d..d1a4adb 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshotTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/config/CodeOwnersPluginConfigSnapshotTest.java
@@ -16,11 +16,13 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.plugins.codeowners.testing.CodeOwnerSetSubject.hasEmail;
import static com.google.gerrit.plugins.codeowners.testing.RequiredApprovalSubject.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.OptionalSubject.assertThat;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.config.GerritConfig;
@@ -172,6 +174,152 @@
}
@Test
+ public void getGlobalCodeOwnersIfNoneIsConfigured() throws Exception {
+ assertThat(cfgSnapshot().getGlobalCodeOwners()).isEmpty();
+ }
+
+ @Test
+ @GerritConfig(
+ name = "plugin.code-owners.globalCodeOwner",
+ values = {"global-code-owner-1@example.com", "global-code-owner-2@example.com"})
+ public void getGlobalCodeOwnerssIfNoneIsConfiguredOnProjectLevel() throws Exception {
+ TestAccount globalCodeOwner1 =
+ accountCreator.create(
+ "globalCodeOwner1",
+ "global-code-owner-1@example.com",
+ "Global Code Owner 1",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner2 =
+ accountCreator.create(
+ "globalCodeOwner2",
+ "global-code-owner-2@example.com",
+ "Global Code Owner 2",
+ /* displayName= */ null);
+ assertThat(cfgSnapshot().getGlobalCodeOwners())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(globalCodeOwner1.email(), globalCodeOwner2.email());
+ }
+
+ @Test
+ @GerritConfig(
+ name = "plugin.code-owners.globalCodeOwner",
+ values = {"global-code-owner-1@example.com", "global-code-owner-2@example.com"})
+ public void globalCodeOwnersOnProjectLevelOverrideGloballyConfiguredGlobalCodeOwners()
+ throws Exception {
+ accountCreator.create(
+ "globalCodeOwner1",
+ "global-code-owner-1@example.com",
+ "Global Code Owner 1",
+ /* displayName= */ null);
+ accountCreator.create(
+ "globalCodeOwner2",
+ "global-code-owner-2@example.com",
+ "Global Code Owner 2",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner3 =
+ accountCreator.create(
+ "globalCodeOwner3",
+ "global-code-owner-3@example.com",
+ "Global Code Owner 3",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner4 =
+ accountCreator.create(
+ "globalCodeOwner4",
+ "global-code-owner-4@example.com",
+ "Global Code Owner 4",
+ /* displayName= */ null);
+ configureGlobalCodeOwners(allProjects, globalCodeOwner3.email(), globalCodeOwner4.email());
+ assertThat(cfgSnapshot().getGlobalCodeOwners())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(globalCodeOwner3.email(), globalCodeOwner4.email());
+ }
+
+ @Test
+ @GerritConfig(
+ name = "plugin.code-owners.globalCodeOwner",
+ values = {"global-code-owner-1@example.com", "global-code-owner-2@example.com"})
+ public void globalCodeOwnersAreInheritedFromParentProject() throws Exception {
+ accountCreator.create(
+ "globalCodeOwner1",
+ "global-code-owner-1@example.com",
+ "Global Code Owner 1",
+ /* displayName= */ null);
+ accountCreator.create(
+ "globalCodeOwner2",
+ "global-code-owner-2@example.com",
+ "Global Code Owner 2",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner3 =
+ accountCreator.create(
+ "globalCodeOwner3",
+ "global-code-owner-3@example.com",
+ "Global Code Owner 3",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner4 =
+ accountCreator.create(
+ "globalCodeOwner4",
+ "global-code-owner-4@example.com",
+ "Global Code Owner 4",
+ /* displayName= */ null);
+ configureGlobalCodeOwners(allProjects, globalCodeOwner3.email(), globalCodeOwner4.email());
+ assertThat(cfgSnapshot().getGlobalCodeOwners())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(globalCodeOwner3.email(), globalCodeOwner4.email());
+ }
+
+ @Test
+ public void inheritedGlobalCodeOwnersCanBeOverridden() throws Exception {
+ TestAccount globalCodeOwner1 =
+ accountCreator.create(
+ "globalCodeOwner1",
+ "global-code-owner-1@example.com",
+ "Global Code Owner 1",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner2 =
+ accountCreator.create(
+ "globalCodeOwner2",
+ "global-code-owner-2@example.com",
+ "Global Code Owner 2",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner3 =
+ accountCreator.create(
+ "globalCodeOwner3",
+ "global-code-owner-3@example.com",
+ "Global Code Owner 3",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner4 =
+ accountCreator.create(
+ "globalCodeOwner4",
+ "global-code-owner-4@example.com",
+ "Global Code Owner 4",
+ /* displayName= */ null);
+ configureGlobalCodeOwners(allProjects, globalCodeOwner1.email(), globalCodeOwner2.email());
+ configureGlobalCodeOwners(project, globalCodeOwner3.email(), globalCodeOwner4.email());
+ assertThat(cfgSnapshot().getGlobalCodeOwners())
+ .comparingElementsUsing(hasEmail())
+ .containsExactly(globalCodeOwner3.email(), globalCodeOwner4.email());
+ }
+
+ @Test
+ public void inheritedGlobalCodeOwnersCanBeRemoved() throws Exception {
+ TestAccount globalCodeOwner1 =
+ accountCreator.create(
+ "globalCodeOwner1",
+ "global-code-owner-1@example.com",
+ "Global Code Owner 1",
+ /* displayName= */ null);
+ TestAccount globalCodeOwner2 =
+ accountCreator.create(
+ "globalCodeOwner2",
+ "global-code-owner-2@example.com",
+ "Global Code Owner 2",
+ /* displayName= */ null);
+ configureGlobalCodeOwners(allProjects, globalCodeOwner1.email(), globalCodeOwner2.email());
+ configureGlobalCodeOwners(project, "");
+ assertThat(cfgSnapshot().getGlobalCodeOwners()).isEmpty();
+ }
+
+ @Test
public void getExemptedAccountsIfNoneIsConfigured() throws Exception {
assertThat(cfgSnapshot().getExemptedAccounts()).isEmpty();
}
@@ -201,7 +349,8 @@
@GerritConfig(
name = "plugin.code-owners.exemptedUser",
values = {"exempted-user-1@example.com", "exempted-user-2@example.com"})
- public void exemptedAccountsOnProjectLevelOverrideGlobalExemptedAcounts() throws Exception {
+ public void exemptedAccountsOnProjectLevelOverrideGloballyConfiguredExemptedAcounts()
+ throws Exception {
accountCreator.create(
"exemptedUser1", "exempted-user-1@example.com", "Exempted User 1", /* displayName= */ null);
accountCreator.create(
@@ -482,7 +631,18 @@
@Test
public void inheritedDisabledBranchCanBeOverridden() throws Exception {
configureDisabledBranch(allProjects, "refs/heads/master");
- enableCodeOwnersForAllBranches(project);
+ configureDisabledBranch(project, "refs/heads/test");
+ assertThat(cfgSnapshot().isDisabled("master")).isFalse();
+ assertThat(cfgSnapshot().isDisabled("test")).isTrue();
+ }
+
+ @Test
+ public void inheritedDisabledBranchCanBeRemoved() throws Exception {
+ configureDisabledBranch(allProjects, "refs/heads/master");
+
+ // override the inherited config with an empty value to enable code owners for all branches
+ configureDisabledBranch(project, "");
+
assertThat(cfgSnapshot().isDisabled("master")).isFalse();
}
@@ -654,7 +814,7 @@
@Test
@GerritConfig(name = "plugin.code-owners.requiredApproval", value = "Code-Review+2")
- public void getConfiguredDefaultRequireApproval() throws Exception {
+ public void getGloballyConfiguredRequiredApproval() throws Exception {
RequiredApproval requiredApproval = cfgSnapshot().getRequiredApproval();
assertThat(requiredApproval).hasLabelNameThat().isEqualTo("Code-Review");
assertThat(requiredApproval).hasValueThat().isEqualTo(2);
@@ -734,7 +894,7 @@
@Test
@GerritConfig(name = "plugin.code-owners.requiredApproval", value = "Code-Review+1")
- public void requiredApprovalConfiguredOnProjectLevelOverridesDefaultRequiredApproval()
+ public void requiredApprovalConfiguredOnProjectLevelOverridesGloballyConfiguredRequiredApproval()
throws Exception {
configureRequiredApproval(project, "Code-Review+2");
RequiredApproval requiredApproval = cfgSnapshot().getRequiredApproval();
@@ -751,8 +911,9 @@
}
@Test
- @GerritConfig(name = "plugin.code-owners.backend", value = FindOwnersBackend.ID)
- public void inheritedRequiredApprovalOverridesDefaultRequiredApproval() throws Exception {
+ @GerritConfig(name = "plugin.code-owners.requiredApproval", value = "Code-Review+1")
+ public void inheritedRequiredApprovalOverridesGloballyConfiguredRequiredApproval()
+ throws Exception {
configureRequiredApproval(allProjects, "Code-Review+2");
RequiredApproval requiredApproval = cfgSnapshot().getRequiredApproval();
assertThat(requiredApproval).hasLabelNameThat().isEqualTo("Code-Review");
@@ -832,25 +993,47 @@
@Test
public void getOverrideApprovalWhenNoRequiredApprovalIsConfigured() throws Exception {
- assertThat(cfgSnapshot().getOverrideApproval()).isEmpty();
+ assertThat(cfgSnapshot().getOverrideApprovals()).isEmpty();
}
@Test
- @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Code-Review+2")
- public void getConfiguredDefaultOverrideApproval() throws Exception {
- ImmutableSet<RequiredApproval> requiredApproval = cfgSnapshot().getOverrideApproval();
- assertThat(requiredApproval).hasSize(1);
- assertThat(requiredApproval).element(0).hasLabelNameThat().isEqualTo("Code-Review");
- assertThat(requiredApproval).element(0).hasValueThat().isEqualTo(2);
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void getConfiguredOverrideApproval() throws Exception {
+ createOwnersOverrideLabel();
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
+ assertThat(requiredApprovals).hasSize(1);
+ assertThat(requiredApprovals).element(0).hasLabelNameThat().isEqualTo("Owners-Override");
+ assertThat(requiredApprovals).element(0).hasValueThat().isEqualTo(1);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Foo-Bar+1")
+ public void getOverrideApprovalIfNonExistingLabelIsConfiguredAsOverrideApproval()
+ throws Exception {
+ assertThat(cfgSnapshot().getOverrideApprovals()).isEmpty();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Code-Review+3")
+ public void getOverrideApprovalIfNonExistingLabelValueIsConfiguredAsOverrideApproval()
+ throws Exception {
+ assertThat(cfgSnapshot().getOverrideApprovals()).isEmpty();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "INVALID")
+ public void getOverrideApprovalIfInvalidOverrideApprovalIsConfigured() throws Exception {
+ assertThat(cfgSnapshot().getOverrideApprovals()).isEmpty();
}
@Test
public void getOverrideApprovalConfiguredOnProjectLevel() throws Exception {
- configureOverrideApproval(project, "Code-Review+2");
- ImmutableSet<RequiredApproval> requiredApproval = cfgSnapshot().getOverrideApproval();
- assertThat(requiredApproval).hasSize(1);
- assertThat(requiredApproval).element(0).hasLabelNameThat().isEqualTo("Code-Review");
- assertThat(requiredApproval).element(0).hasValueThat().isEqualTo(2);
+ createOwnersOverrideLabel();
+ configureOverrideApproval(project, "Owners-Override+1");
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
+ assertThat(requiredApprovals).hasSize(1);
+ assertThat(requiredApprovals).element(0).hasLabelNameThat().isEqualTo("Owners-Override");
+ assertThat(requiredApprovals).element(0).hasValueThat().isEqualTo(1);
}
@Test
@@ -864,7 +1047,7 @@
OverrideApprovalConfig.KEY_OVERRIDE_APPROVAL,
ImmutableList.of("Owners-Override+1", "Other-Override+1"));
- ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApproval();
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
assertThat(
requiredApprovals.stream()
.map(requiredApproval -> requiredApproval.toString())
@@ -873,16 +1056,102 @@
}
@Test
- @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "INVALID")
- public void getOverrideApprovalIfInvalidOverrideApprovalIsConfigured() throws Exception {
- assertThat(cfgSnapshot().getOverrideApproval()).isEmpty();
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void overrideApprovalConfiguredOnProjectLevelOverridesGloballyConfiguredOverrideApproval()
+ throws Exception {
+ createOwnersOverrideLabel();
+ createOwnersOverrideLabel("Other-Override");
+
+ configureOverrideApproval(project, "Other-Override+1");
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
+ assertThat(requiredApprovals).hasSize(1);
+ assertThat(requiredApprovals).element(0).hasLabelNameThat().isEqualTo("Other-Override");
+ assertThat(requiredApprovals).element(0).hasValueThat().isEqualTo(1);
+ }
+
+ @Test
+ public void overrideApprovalIsInheritedFromParentProject() throws Exception {
+ createOwnersOverrideLabel();
+
+ configureOverrideApproval(allProjects, "Owners-Override+1");
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
+ assertThat(requiredApprovals).hasSize(1);
+ assertThat(requiredApprovals).element(0).hasLabelNameThat().isEqualTo("Owners-Override");
+ assertThat(requiredApprovals).element(0).hasValueThat().isEqualTo(1);
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.overrideApproval", value = "Owners-Override+1")
+ public void inheritedOverrideApprovalOverridesGloballyConfiguredOverrideApproval()
+ throws Exception {
+ createOwnersOverrideLabel();
+ createOwnersOverrideLabel("Other-Override");
+
+ configureOverrideApproval(allProjects, "Other-Override+1");
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
+ assertThat(requiredApprovals).hasSize(1);
+ assertThat(requiredApprovals).element(0).hasLabelNameThat().isEqualTo("Other-Override");
+ assertThat(requiredApprovals).element(0).hasValueThat().isEqualTo(1);
+ }
+
+ @Test
+ public void projectLevelOverrideApprovalOverridesInheritedOverrideApproval() throws Exception {
+ createOwnersOverrideLabel();
+ createOwnersOverrideLabel("Other-Override");
+
+ configureOverrideApproval(allProjects, "Owners-Override+1");
+ configureOverrideApproval(project, "Other-Override+1");
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
+ assertThat(requiredApprovals).hasSize(1);
+ assertThat(requiredApprovals).element(0).hasLabelNameThat().isEqualTo("Other-Override");
+ assertThat(requiredApprovals).element(0).hasValueThat().isEqualTo(1);
+ }
+
+ @Test
+ public void
+ projectLevelOverrideApprovalOverridesInheritedOverrideApprovalWithDifferentLabelValue()
+ throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.values = ImmutableMap.of("+2", "Super-Override", "+1", "Override", " 0", "No Override");
+ gApi.projects().name(project.get()).label("Owners-Override").create(input).get();
+
+ configureOverrideApproval(allProjects, "Owners-Override+1");
+ configureOverrideApproval(project, "Owners-Override+2");
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
+ assertThat(requiredApprovals).hasSize(1);
+ assertThat(requiredApprovals).element(0).hasLabelNameThat().isEqualTo("Owners-Override");
+ assertThat(requiredApprovals).element(0).hasValueThat().isEqualTo(2);
+ }
+
+ @Test
+ public void getOverrideApprovalIfNonExistingLabelIsConfiguredAsOverrideApprovalOnProjectLevel()
+ throws Exception {
+ configureOverrideApproval(project, "Foo-Bar+1");
+ assertThat(cfgSnapshot().getOverrideApprovals()).isEmpty();
+ }
+
+ @Test
+ public void
+ getOverrideApprovalIfNonExistingLabelValueIsConfiguredAsOverrideApprovalOnProjectLevel()
+ throws Exception {
+ createOwnersOverrideLabel();
+ configureOverrideApproval(project, "Owners-Override+2");
+ assertThat(cfgSnapshot().getOverrideApprovals()).isEmpty();
}
@Test
public void getOverrideApprovalIfInvalidOverrideApprovalIsConfiguredOnProjectLevel()
throws Exception {
configureOverrideApproval(project, "INVALID");
- assertThat(cfgSnapshot().getOverrideApproval()).isEmpty();
+ assertThat(cfgSnapshot().getOverrideApprovals()).isEmpty();
+ }
+
+ @Test
+ public void projectLevelOverrideApprovalForOtherProjectHasNoEffect() throws Exception {
+ createOwnersOverrideLabel();
+ Project.NameKey otherProject = projectOperations.newProject().create();
+ configureOverrideApproval(otherProject, "Owners-Override+1");
+ assertThat(cfgSnapshot().getOverrideApprovals()).isEmpty();
}
@Test
@@ -894,10 +1163,10 @@
ImmutableList.of("Code-Review+2", "Code-Review+1", "Code-Review+2"));
// If multiple values are set for a key, the last value wins.
- ImmutableSet<RequiredApproval> requiredApproval = cfgSnapshot().getOverrideApproval();
- assertThat(requiredApproval).hasSize(1);
- assertThat(requiredApproval).element(0).hasLabelNameThat().isEqualTo("Code-Review");
- assertThat(requiredApproval).element(0).hasValueThat().isEqualTo(1);
+ ImmutableSet<RequiredApproval> requiredApprovals = cfgSnapshot().getOverrideApprovals();
+ assertThat(requiredApprovals).hasSize(1);
+ assertThat(requiredApprovals).element(0).hasLabelNameThat().isEqualTo("Code-Review");
+ assertThat(requiredApprovals).element(0).hasValueThat().isEqualTo(1);
}
@Test
@@ -1266,6 +1535,15 @@
fallbackCodeOwners.name());
}
+ private void configureGlobalCodeOwners(Project.NameKey project, String... globalCodeOwners)
+ throws Exception {
+ setCodeOwnersConfig(
+ project,
+ /* subsection= */ null,
+ GeneralConfig.KEY_GLOBAL_CODE_OWNER,
+ ImmutableList.copyOf(globalCodeOwners));
+ }
+
private void configureExemptedUsers(Project.NameKey project, String... exemptedUsers)
throws Exception {
setCodeOwnersConfig(
@@ -1294,10 +1572,6 @@
project, /* subsection= */ null, StatusConfig.KEY_DISABLED_BRANCH, disabledBranch);
}
- private void enableCodeOwnersForAllBranches(Project.NameKey project) throws Exception {
- setCodeOwnersConfig(project, /* subsection= */ null, StatusConfig.KEY_DISABLED_BRANCH, "");
- }
-
private void configureBackend(Project.NameKey project, String backendName) throws Exception {
configureBackend(project, /* branch= */ null, backendName);
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/config/GeneralConfigTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/config/GeneralConfigTest.java
index be65c1f..aed1651 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/config/GeneralConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/config/GeneralConfigTest.java
@@ -1411,7 +1411,7 @@
@Test
@GerritConfig(
- name = "plugin.code-owners.exemptedUsers",
+ name = "plugin.code-owners.exemptedUser",
values = {"bot1@example.com", "bot2@example.com"})
public void inheritedExemptedUsersCanBeRemovedOnProjectLevel() throws Exception {
Config cfg = new Config();
diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/config/StatusConfigTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/config/StatusConfigTest.java
index 2d892e4..4e82aeb 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/config/StatusConfigTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/config/StatusConfigTest.java
@@ -238,6 +238,19 @@
disabledBranchConfigurationInPluginConfigOverridesDisabledBranchConfigurationInGerritConfig()
throws Exception {
Config cfg = new Config();
+ cfg.setString(SECTION_CODE_OWNERS, null, KEY_DISABLED_BRANCH, "refs/heads/test");
+ assertThat(statusConfig.isDisabledForBranch(cfg, BranchNameKey.create(project, "master")))
+ .isFalse();
+ assertThat(statusConfig.isDisabledForBranch(cfg, BranchNameKey.create(project, "test")))
+ .isTrue();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.disabledBranch", value = "refs/heads/master")
+ public void
+ disabledBranchConfigurationInPluginConfigCanRemoveDisabledBranchConfigurationInGerritConfig()
+ throws Exception {
+ Config cfg = new Config();
cfg.setString(SECTION_CODE_OWNERS, null, KEY_DISABLED_BRANCH, "");
assertThat(statusConfig.isDisabledForBranch(cfg, BranchNameKey.create(project, "master")))
.isFalse();
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
index 313bef4..ffecde7 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
@@ -169,7 +169,7 @@
when(codeOwnersPluginConfigSnapshot.areImplicitApprovalsEnabled()).thenReturn(true);
when(codeOwnersPluginConfigSnapshot.getRequiredApproval())
.thenReturn(RequiredApproval.create(getDefaultCodeReviewLabel(), (short) 2));
- when(codeOwnersPluginConfigSnapshot.getOverrideApproval())
+ when(codeOwnersPluginConfigSnapshot.getOverrideApprovals())
.thenReturn(
ImmutableSet.of(
RequiredApproval.create(
@@ -242,7 +242,7 @@
public void withMultipleOverrides() throws Exception {
createOwnersOverrideLabel();
- when(codeOwnersPluginConfigSnapshot.getOverrideApproval())
+ when(codeOwnersPluginConfigSnapshot.getOverrideApprovals())
.thenReturn(
ImmutableSet.of(
RequiredApproval.create(LabelType.withDefaultValues("Owners-Override"), (short) 1),
@@ -284,7 +284,7 @@
when(codeOwnersPluginConfigSnapshot.areImplicitApprovalsEnabled()).thenReturn(true);
when(codeOwnersPluginConfigSnapshot.getRequiredApproval())
.thenReturn(RequiredApproval.create(getDefaultCodeReviewLabel(), (short) 2));
- when(codeOwnersPluginConfigSnapshot.getOverrideApproval())
+ when(codeOwnersPluginConfigSnapshot.getOverrideApprovals())
.thenReturn(
ImmutableSet.of(
RequiredApproval.create(
diff --git a/resources/Documentation/config.md b/resources/Documentation/config.md
index 6250677..dd7cd35 100644
--- a/resources/Documentation/config.md
+++ b/resources/Documentation/config.md
@@ -1,21 +1,24 @@
# Configuration
-The global configuration of the @PLUGIN@ plugin is stored in the `gerrit.config`
-file in the `plugin.@PLUGIN@` subsection.
+The @PLUGIN@ plugin can be configured on host and on project-level:
+
+* host-level:\
+ in the `gerrit.config` file in the `plugin.@PLUGIN@` subsection
+* project-level:\
+ in `@PLUGIN@.config` files that are stored in the `refs/meta/config` branches
+ of the projects
This page describes all available configuration parameters. For configuration
recommendations please consult the [config guide](config-guide.html).
-## <a id="projectLevelConfigFile">
-In addition some configuration can be done on the project level in
-`@PLUGIN@.config` files that are stored in the `refs/meta/config` branches of
-the projects.
+## <a id="inheritance">Inheritance</a>
-Parameters that are not set for a project are inherited from the parent project
-or the global configuration in `gerrit.config`.
+Projects inherit the configuration of their parent projects, following the chain
+of parent projects until the `All-Projects` root project is reached which
+inherits the configuration from `gerrit.config`.
-A config setting on project level overrides the corresponsing setting that is
-inherited from parent projects and the global configuration in `gerrit.config`.
+Setting a configuration parameter for a project overrides any inherited value
+for this configuration parameter.
**NOTE:** Some configuration parameters have a list of values and can be
specified multiple times (e.g. `disabledBranch`). If such a value is set on
