commit b6d59cca8098c61f01135305361dfa6661f3c91b
author Edwin Kempin <ekempin@google.com> Thu Oct 08 11:21:06 2020 +0200
committer Edwin Kempin <ekempin@google.com> Thu Oct 08 11:24:29 2020 +0200
tree e5b9994617893d544101db9da523c0c7f873fed5
parent 54e5deaf52e99da547875869ac70a06d95a407df

CodeOwnerApprovalCheckTest: Make the approvedByGlobalCodeOwner more readable

Looking at the test one could think that the file was already implictly
approved by the admin user who creates the change and is a code owner.
This is not the case since implicit approvals are disabled by default.
However the test becomes more readable if we use a non-code-owner user
to create the change. Add a verification that the file is not approved
before the global code owner adds their approval. Also add some more
comments to make the code easier to read.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ib626315a1ca5efdd9b2e5a066f878507606d1ad8

javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java

diff --git a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
index 941a861..c0516a9 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/backend/CodeOwnerApprovalCheckTest.java
@@ -894,8 +894,10 @@
   @Test
   @GerritConfig(name = "plugin.code-owners.globalCodeOwner", value = "bot@example.com")
   public void approvedByGlobalCodeOwner() throws Exception {
+    // Create a bot user that is a global code owner.
     TestAccount bot = accountCreator.create("bot", "bot@example.com", "Bot", null);
 
+    // Create a code owner config file so that we are not in the bootstrapping mode.
     codeOwnerConfigOperations
         .newCodeOwnerConfig()
         .project(project)
@@ -904,11 +906,25 @@
         .addCodeOwnerEmail(admin.email())
         .create();
 
+    // Create a change as a user that is not a code owner.
     Path path = Paths.get("/foo/bar.baz");
     String changeId =
-        createChange("Change Adding A File", JgitPath.of(path).get(), "file content").getChangeId();
+        createChange(user, "Change Adding A File", JgitPath.of(path).get(), "file content")
+            .getChangeId();
 
-    // let the bot approve the change
+    // Verify that the file is not approved yet.
+    Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
+        codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+    FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
+        assertThatStream(fileCodeOwnerStatuses).onlyElement();
+    fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
+    fileCodeOwnerStatusSubject
+        .hasNewPathStatus()
+        .value()
+        .hasStatusThat()
+        .isEqualTo(CodeOwnerStatus.INSUFFICIENT_REVIEWERS);
+
+    // Let the bot approve the change.
     projectOperations
         .project(project)
         .forUpdate()
@@ -917,11 +933,10 @@
     requestScopeOperations.setApiUser(bot.id());
     approve(changeId);
 
+    // Check that the file is approved now.
     requestScopeOperations.setApiUser(admin.id());
-    Stream<FileCodeOwnerStatus> fileCodeOwnerStatuses =
-        codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
-    FileCodeOwnerStatusSubject fileCodeOwnerStatusSubject =
-        assertThatStream(fileCodeOwnerStatuses).onlyElement();
+    fileCodeOwnerStatuses = codeOwnerApprovalCheck.getFileStatuses(getChangeNotes(changeId));
+    fileCodeOwnerStatusSubject = assertThatStream(fileCodeOwnerStatuses).onlyElement();
     fileCodeOwnerStatusSubject.hasNewPathStatus().value().hasPathThat().isEqualTo(path);
     fileCodeOwnerStatusSubject
         .hasNewPathStatus()