Merge changes I338df11c,I3d1bf717,Id9c8c438

* changes:
  Mark getRemoteHead as @Nullable
  Push group operations down into leaf test classes.
  Use new test API for group creation in AbstractDaemonTest

java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -38,6 +38,7 @@ import com.google.common.jimfs.Jimfs;
 import com.google.common.primitives.Chars;
 import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
 import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.ContributorAgreement;
@@ -55,7 +56,6 @@ import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.RevisionApi;
 import com.google.gerrit.extensions.api.changes.SubmittedTogetherInfo;
 import com.google.gerrit.extensions.api.groups.GroupApi;
-import com.google.gerrit.extensions.api.groups.GroupInput;
 import com.google.gerrit.extensions.api.projects.BranchApi;
 import com.google.gerrit.extensions.api.projects.BranchInput;
 import com.google.gerrit.extensions.api.projects.ProjectInput;
@@ -264,6 +264,7 @@ public abstract class AbstractDaemonTest {
   @Inject protected ChangeNotes.Factory notesFactory;
   @Inject protected BatchAbandon batchAbandon;
   @Inject protected TestSshKeys sshKeys;
+  @Inject protected GroupOperations groupOperations;
 
   protected EventRecorder eventRecorder;
   protected GerritServer server;
@@ -1193,27 +1194,7 @@ public abstract class AbstractDaemonTest {
     return changeResourceFactory.create(notes.get(0), atrScope.get().getUser());
   }
 
-  protected String createGroup(String name) throws Exception {
+  @Nullable
-    return createGroup(name, "Administrators");
-  }
-
-  protected String createGroupWithRealName(String name) throws Exception {
-    GroupInput in = new GroupInput();
-    in.name = name;
-    in.ownerId = "Administrators";
-    gApi.groups().create(in);
-    return name;
-  }
-
-  protected String createGroup(String name, String owner) throws Exception {
-    name = name(name);
-    GroupInput in = new GroupInput();
-    in.name = name;
-    in.ownerId = owner;
-    gApi.groups().create(in);
-    return name;
-  }
-
   protected RevCommit getHead(Repository repo, String name) throws Exception {
     try (RevWalk rw = new RevWalk(repo)) {
       Ref r = repo.exactRef(name);
@@ -1225,16 +1206,19 @@ public abstract class AbstractDaemonTest {
     return getHead(repo, "HEAD");
   }
 
+  @Nullable
   protected RevCommit getRemoteHead(Project.NameKey project, String branch) throws Exception {
     try (Repository repo = repoManager.openRepository(project)) {
       return getHead(repo, branch.startsWith(Constants.R_REFS) ? branch : "refs/heads/" + branch);
     }
   }
 
+  @Nullable
   protected RevCommit getRemoteHead(String project, String branch) throws Exception {
     return getRemoteHead(new Project.NameKey(project), branch);
   }
 
+  @Nullable
   protected RevCommit getRemoteHead() throws Exception {
     return getRemoteHead(project, "master");
   }
@@ -1248,7 +1232,8 @@ public abstract class AbstractDaemonTest {
   protected ContributorAgreement configureContributorAgreement(boolean autoVerify)
       throws Exception {
     ContributorAgreement ca;
-    String g = createGroup(autoVerify ? "cla-test-group" : "cla-test-no-auto-verify-group");
+    String name = autoVerify ? "cla-test-group" : "cla-test-no-auto-verify-group";
+    String g = groupOperations.newGroup().name(name).create().get();
     GroupApi groupApi = gApi.groups().id(g);
     groupApi.description("CLA test group");
     InternalGroup caGroup = group(new AccountGroup.UUID(groupApi.detail().id));

javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -56,6 +56,7 @@ import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.acceptance.UseSsh;
 import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
 import com.google.gerrit.acceptance.testsuite.account.TestSshKeys;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.GlobalCapability;
@@ -97,6 +98,7 @@ import com.google.gerrit.gpg.PublicKeyStore;
 import com.google.gerrit.gpg.testing.TestKey;
 import com.google.gerrit.mail.Address;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
@@ -233,6 +235,8 @@ public class AccountIT extends AbstractDaemonTest {
 
   @Inject private AccountManager accountManager;
 
+  @Inject protected GroupOperations groupOperations;
+
   private AccountIndexedCounter accountIndexedCounter;
   private RegistrationHandle accountIndexEventCounterHandle;
   private RefUpdateCounter refUpdateCounter;
@@ -2208,7 +2212,9 @@ public class AccountIT extends AbstractDaemonTest {
   public void allGroupsForAUserAccountCanBeRetrieved() throws Exception {
     String username = name("user1");
     accountOperations.newAccount().username(username).create();
-    String group = createGroup("group");
+    AccountGroup.UUID groupID = groupOperations.newGroup().name("group").create();
+    String group = groupOperations.group(groupID).get().name();
+
     gApi.groups().id(group).addMembers(username);
 
     List<GroupInfo> allGroups = gApi.accounts().id(username).getGroups();

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -68,6 +68,7 @@ import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.data.LabelFunction;
 import com.google.gerrit.common.data.LabelType;
@@ -194,6 +195,8 @@ public class ChangeIT extends AbstractDaemonTest {
   @Inject private ChangeIndexCollection changeIndexCollection;
   @Inject private IndexConfig indexConfig;
 
+  @Inject protected GroupOperations groupOperations;
+
   private ChangeIndexedCounter changeIndexedCounter;
   private RegistrationHandle changeIndexedCounterHandle;
 
@@ -1823,7 +1826,7 @@ public class ChangeIT extends AbstractDaemonTest {
             .preferredEmail(email)
             .fullname(fullname)
             .create();
-    String testGroup = createGroupWithRealName("ab");
+    String testGroup = groupOperations.newGroup().name("ab").create().get();
     GroupApi groupApi = gApi.groups().id(testGroup);
     groupApi.description("test group");
     groupApi.addMembers(user.fullName);
@@ -1884,7 +1887,7 @@ public class ChangeIT extends AbstractDaemonTest {
             .fullname(myGroupUserFullname)
             .create();
 
-    String testGroup = createGroupWithRealName("kobe");
+    String testGroup = groupOperations.newGroup().name("kobe").create().get();
     GroupApi groupApi = gApi.groups().id(testGroup);
     groupApi.description("test group");
     groupApi.addMembers(myGroupUserFullname);

javatests/com/google/gerrit/acceptance/api/group/GroupsConsistencyIT.java

@@ -20,6 +20,7 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo;
 import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo.ConsistencyProblemInfo;
@@ -30,6 +31,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.group.db.GroupConfig;
 import com.google.gerrit.server.group.db.GroupNameNotes;
 import com.google.gerrit.server.group.db.testing.GroupTestUtil;
+import com.google.inject.Inject;
 import java.util.List;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.RefRename;
@@ -47,6 +49,8 @@ import org.junit.Test;
 @Sandboxed
 @NoHttpd
 public class GroupsConsistencyIT extends AbstractDaemonTest {
+
+  @Inject protected GroupOperations groupOperations;
   private GroupInfo gAdmin;
   private GroupInfo g1;
   private GroupInfo g2;
@@ -57,8 +61,8 @@ public class GroupsConsistencyIT extends AbstractDaemonTest {
   public void basicSetup() throws Exception {
     allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
 
-    String name1 = createGroup("g1");
+    String name1 = groupOperations.newGroup().name("g1").create().get();
-    String name2 = createGroup("g2");
+    String name2 = groupOperations.newGroup().name("g2").create().get();
 
     gApi.groups().id(name1).addMembers(user.fullName);
     gApi.groups().id(name2).addMembers(admin.fullName);
@@ -218,7 +222,7 @@ public class GroupsConsistencyIT extends AbstractDaemonTest {
   @Test
   public void cyclicSubgroup() throws Exception {
     updateGroupFile(RefNames.refsGroups(new AccountGroup.UUID(g1.id)), "subgroups", g1.id + "\n");
-    assertWarning("cyclic");
+    assertWarning("cycle");
   }
 
   private void assertError(String msg) throws Exception {

javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java

@@ -144,6 +144,23 @@ public class GroupsIT extends AbstractDaemonTest {
     }
   }
 
+  // Creates a group, but with uniquified name.
+  protected String createGroup(String name) throws Exception {
+    // TODO(hanwen): rewrite this test in terms of UUID. This requires redoing the assertion helpers
+    // too.
+    AccountGroup.UUID g = groupOperations.newGroup().ownerGroupUuid(adminGroupUuid()).create();
+    return groupRef(g).getName();
+  }
+
+  protected String createGroup(String name, String owner) throws Exception {
+    name = name(name);
+    GroupInput in = new GroupInput();
+    in.name = name;
+    in.ownerId = owner;
+    gApi.groups().create(in);
+    return name;
+  }
+
   @Override
   protected ProjectResetter.Config resetProjects() {
     // Don't reset All-Users since deleting users makes groups inconsistent (e.g. groups would

javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java

@@ -31,6 +31,7 @@ import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.Sandboxed;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerResult;
@@ -52,6 +53,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.change.ReviewerAdder;
 import com.google.gerrit.testing.FakeEmailSender.Message;
 import com.google.gson.stream.JsonReader;
+import com.google.inject.Inject;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -62,12 +64,15 @@ import java.util.Map;
 import org.junit.Test;
 
 public class ChangeReviewersIT extends AbstractDaemonTest {
+
+  @Inject protected GroupOperations groupOperations;
+
   @Test
   public void addGroupAsReviewer() throws Exception {
     // Set up two groups, one that is too large too add as reviewer, and one
     // that is too large to add without confirmation.
-    String largeGroup = createGroup("largeGroup");
+    String largeGroup = groupOperations.newGroup().name("largeGroup").create().get();
-    String mediumGroup = createGroup("mediumGroup");
+    String mediumGroup = groupOperations.newGroup().name("mediumGroup").create().get();
 
     int largeGroupSize = ReviewerAdder.DEFAULT_MAX_REVIEWERS + 1;
     int mediumGroupSize = ReviewerAdder.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
@@ -170,7 +175,7 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
     PushOneCommit.Result r = createChange();
     String changeId = r.getChangeId();
     AddReviewerInput in = new AddReviewerInput();
-    in.reviewer = createGroup("cc1");
+    in.reviewer = groupOperations.newGroup().name("cc1").create().get();
     in.state = CC;
     gApi.groups()
         .id(in.reviewer)
@@ -209,7 +214,7 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
     result = addReviewer(changeId, reviewer.username);
     assertThat(result.error).isNull();
     sender.clear();
-    in.reviewer = createGroup("cc2");
+    in.reviewer = groupOperations.newGroup().name("cc2").create().get();
     gApi.groups().id(in.reviewer).addMembers(usernames.toArray(new String[usernames.size()]));
     gApi.groups().id(in.reviewer).addMembers(reviewer.username);
     result = addReviewer(changeId, in);
@@ -479,8 +484,8 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
       usernames.add(u.username);
     }
 
-    String largeGroup = createGroup("largeGroup");
+    String largeGroup = groupOperations.newGroup().name("largeGroup").create().get();
-    String mediumGroup = createGroup("mediumGroup");
+    String mediumGroup = groupOperations.newGroup().name("mediumGroup").create().get();
     gApi.groups().id(largeGroup).addMembers(usernames.toArray(new String[largeGroupSize]));
     gApi.groups()
         .id(mediumGroup)
@@ -622,8 +627,8 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
         accountCreator.create(name("user2"), emailPrefix + "user2@example.com", "User2");
     TestAccount user3 =
         accountCreator.create(name("user3"), emailPrefix + "user3@example.com", "User3");
-    String group1 = createGroup("group1");
+    String group1 = groupOperations.newGroup().name("group1").create().get();
-    String group2 = createGroup("group2");
+    String group2 = groupOperations.newGroup().name("group2").create().get();
     gApi.groups().id(group1).addMembers(user1.username, user2.username);
     gApi.groups().id(group2).addMembers(user2.username, user3.username);
 

javatests/com/google/gerrit/acceptance/rest/change/IndexChangeIT.java

@@ -20,17 +20,22 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.project.testing.Util;
+import com.google.inject.Inject;
 import java.util.List;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
 import org.junit.Test;
 
 public class IndexChangeIT extends AbstractDaemonTest {
+
+  @Inject protected GroupOperations groupOperations;
+
   @Test
   public void indexChange() throws Exception {
     String changeId = createChange().getChangeId();
@@ -48,7 +53,8 @@ public class IndexChangeIT extends AbstractDaemonTest {
   public void indexChangeAfterOwnerLosesVisibility() throws Exception {
     // Create a test group with 2 users as members
     TestAccount user2 = accountCreator.user2();
-    String group = createGroup("test");
+    AccountGroup.UUID groupId = groupOperations.newGroup().name("test").create();
+    String group = groupOperations.group(groupId).get().name();
     gApi.groups().id(group).addMembers("admin", "user", user2.username);
 
     // Create a project and restrict its visibility to the group