Fix flaky test for group renaming in ProjectConfig
The groups API supports renaming a group. This will start a background
thread that goes over all projects that have this group name in their
configs and perform a local rename.
The test was flaky because of that asynchronous behavior. With this
commit, we test the rename on the project config instead wich allows
us to skip the asynchronous behavior that has caused the flakyness.
The new test would also have caught the original bug.
Change-Id: I2f26ff5d01176240e1368acd8389bfac6ee2b494
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index e4bb73a..0af116a 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -113,7 +113,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
import java.util.stream.Stream;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
@@ -705,36 +704,6 @@
}
@Test
- public void renamingGroupChangesProjectConfigs() throws Exception {
- String name = name("Name1");
- GroupInfo group = gApi.groups().create(name).get();
-
- // Use group in a permission
- projectOperations
- .project(project)
- .forUpdate()
- .add(allow(Permission.READ).ref(RefNames.REFS_CONFIG).group(AccountGroup.uuid(group.id)))
- .update();
- Optional<String> beforeRename =
- projectCache.get(project).get().getLocalGroups().stream()
- .filter(g -> g.getUUID().get().equals(group.id))
- .map(GroupReference::getName)
- .findAny();
- // Groups created with ProjectOperations always have their UUID as local name
- assertThat(beforeRename).hasValue(group.id);
-
- String newName = name("Name2");
- gApi.groups().id(name).name(newName);
-
- Optional<String> afterRename =
- projectCache.get(project).get().getLocalGroups().stream()
- .filter(g -> g.getUUID().get().equals(group.id))
- .map(GroupReference::getName)
- .findAny();
- assertThat(afterRename).hasValue(newName);
- }
-
- @Test
public void groupDescription() throws Exception {
String name = name("group");
gApi.groups().create(name);
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index e99a6f5..d5fc1c1 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.api.project;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.block;
import static com.google.gerrit.server.project.ProjectState.INHERITED_FROM_GLOBAL;
@@ -39,7 +40,9 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.GroupReference;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
@@ -55,6 +58,7 @@
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.extensions.events.ProjectIndexedListener;
@@ -63,6 +67,7 @@
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.server.config.ProjectConfigEntry;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.inject.AbstractModule;
@@ -70,6 +75,7 @@
import com.google.inject.Module;
import java.util.HashMap;
import java.util.Map;
+import java.util.Optional;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
@@ -939,6 +945,42 @@
projectOperations.project(allProjects).getHead(RefNames.REFS_CONFIG).name()));
}
+ @Test
+ public void renamingGroupGetsPersisted() throws Exception {
+ String name = name("Name1");
+ GroupInfo group = gApi.groups().create(name).get();
+
+ // Use group in a permission
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allow(Permission.READ).ref(RefNames.REFS_CONFIG).group(AccountGroup.uuid(group.id)))
+ .update();
+ Optional<String> beforeRename =
+ projectCache.get(project).get().getLocalGroups().stream()
+ .filter(g -> g.getUUID().get().equals(group.id))
+ .map(GroupReference::getName)
+ .findAny();
+ // Groups created with ProjectOperations always have their UUID as local name
+ assertThat(beforeRename).hasValue(group.id);
+
+ // Rename the group directly on the project config
+ String newName = name("Name2");
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(project)) {
+ ProjectConfig config = projectConfigFactory.read(md);
+ config.renameGroup(AccountGroup.uuid(group.id), newName);
+ config.commit(md);
+ projectCache.evict(config.getProject());
+ }
+
+ Optional<String> afterRename =
+ projectCache.get(project).get().getLocalGroups().stream()
+ .filter(g -> g.getUUID().get().equals(group.id))
+ .map(GroupReference::getName)
+ .findAny();
+ assertThat(afterRename).hasValue(newName);
+ }
+
private CommentLinkInfo commentLinkInfo(String name, String match, String link) {
CommentLinkInfo info = new CommentLinkInfo();
info.name = name;
