Sort override approvals that are returned in CodeOwnerProjectConfigInfo
Otherwise the order is random and the
getConfigWithMultipleConfiguredOverrideApproval and
withMultipleOverrides tests are flaky.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I4021bfaf33277fe63f825596b5a839deecbc07b2
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
index 6e53e33..bf3a8ad 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
@@ -16,6 +16,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static java.util.Comparator.comparing;
import static java.util.Objects.requireNonNull;
import com.google.common.annotations.VisibleForTesting;
@@ -164,6 +165,7 @@
ImmutableList<RequiredApprovalInfo> formatOverrideApprovalInfo(Project.NameKey projectName) {
ImmutableList<RequiredApprovalInfo> overrideApprovalInfos =
codeOwnersPluginConfiguration.getOverrideApproval(projectName).stream()
+ .sorted(comparing(requiredApproval -> requiredApproval.toString()))
.map(CodeOwnerProjectConfigJson::formatRequiredApproval)
.collect(toImmutableList());
return overrideApprovalInfos.isEmpty() ? null : overrideApprovalInfos;
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
index 9cc77ea..d6bf9ab 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
@@ -238,11 +238,11 @@
CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
assertThat(codeOwnerBranchConfigInfo.overrideApproval).hasSize(2);
- assertThat(codeOwnerBranchConfigInfo.overrideApproval.get(0).label)
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval.get(0).label).isEqualTo("Code-Review");
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval.get(0).value).isEqualTo(2);
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval.get(1).label)
.isEqualTo("Owners-Override");
- assertThat(codeOwnerBranchConfigInfo.overrideApproval.get(0).value).isEqualTo(1);
- assertThat(codeOwnerBranchConfigInfo.overrideApproval.get(1).label).isEqualTo("Code-Review");
- assertThat(codeOwnerBranchConfigInfo.overrideApproval.get(1).value).isEqualTo(2);
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval.get(1).value).isEqualTo(1);
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerProjectConfigIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerProjectConfigIT.java
index a6de7df..0a14b8e 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerProjectConfigIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerProjectConfigIT.java
@@ -265,11 +265,11 @@
CodeOwnerProjectConfigInfo codeOwnerProjectConfigInfo =
projectCodeOwnersApiFactory.project(project).getConfig();
assertThat(codeOwnerProjectConfigInfo.overrideApproval).hasSize(2);
- assertThat(codeOwnerProjectConfigInfo.overrideApproval.get(0).label)
+ assertThat(codeOwnerProjectConfigInfo.overrideApproval.get(0).label).isEqualTo("Code-Review");
+ assertThat(codeOwnerProjectConfigInfo.overrideApproval.get(0).value).isEqualTo(2);
+ assertThat(codeOwnerProjectConfigInfo.overrideApproval.get(1).label)
.isEqualTo("Owners-Override");
- assertThat(codeOwnerProjectConfigInfo.overrideApproval.get(0).value).isEqualTo(1);
- assertThat(codeOwnerProjectConfigInfo.overrideApproval.get(1).label).isEqualTo("Code-Review");
- assertThat(codeOwnerProjectConfigInfo.overrideApproval.get(1).value).isEqualTo(2);
+ assertThat(codeOwnerProjectConfigInfo.overrideApproval.get(1).value).isEqualTo(1);
}
@Test
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
index bd632c4..51b6ea5 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
@@ -248,10 +248,10 @@
ImmutableList<RequiredApprovalInfo> requiredApprovalInfos =
codeOwnerProjectConfigJson.formatOverrideApprovalInfo(project);
assertThat(requiredApprovalInfos).hasSize(2);
- assertThat(requiredApprovalInfos.get(0).label).isEqualTo("Owners-Override");
- assertThat(requiredApprovalInfos.get(0).value).isEqualTo(1);
- assertThat(requiredApprovalInfos.get(1).label).isEqualTo("Code-Review");
- assertThat(requiredApprovalInfos.get(1).value).isEqualTo(2);
+ assertThat(requiredApprovalInfos.get(0).label).isEqualTo("Code-Review");
+ assertThat(requiredApprovalInfos.get(0).value).isEqualTo(2);
+ assertThat(requiredApprovalInfos.get(1).label).isEqualTo("Owners-Override");
+ assertThat(requiredApprovalInfos.get(1).value).isEqualTo(1);
}
@Test
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 64a077e..da00ae4 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -617,7 +617,7 @@
| `disabled` | optional | Whether the code owners functionality is disabled for the branch. If `true` the code owners API is disabled and submitting changes doesn't require code owner approvals. Not set if `false`.
| `backend_id`| optional | ID of the code owner backend that is configured for the branch. Not set if `disabled` is `true`.
| `required_approval` | optional | The approval that is required from code owners to approve the files in a change as [RequiredApprovalInfo](#required-approval-info) entity. The required approval defines which approval counts as code owner approval. Any approval on this label with a value >= the given value is considered as code owner approval. Not set if `disabled` is `true`.
-| `override_approval` | optional | Approvals that count as override for the code owners submit check as a list of [RequiredApprovalInfo](#required-approval-info) entities. If multiple approvals are returned, any of them is sufficient to override the code owners submit check. All returned override approvals are guarenteed to have distinct label names. Any approval on these labels with a value >= the given values is considered as code owner override. If unset, overriding the code owners submit check is disabled. Not set if `disabled` is `true`.
+| `override_approval` | optional | Approvals that count as override for the code owners submit check as a list of [RequiredApprovalInfo](#required-approval-info) entities (sorted alphabetically). If multiple approvals are returned, any of them is sufficient to override the code owners submit check. All returned override approvals are guarenteed to have distinct label names. Any approval on these labels with a value >= the given values is considered as code owner override. If unset, overriding the code owners submit check is disabled. Not set if `disabled` is `true`.
| `no_code_owners_defined` | optional | Whether the branch doesn't contain any code owner config file yet. If a branch doesn't contain any code owner config file yet, the projects owners are considered as code owners. Once a first code owner config file is added to the branch, the project owners are no longer code owners (unless code ownership is granted to them via the code owner config file). Not set if `false` or if `disabled` is `true`.
---
