opendev/gerrit
Code Issues Proposed changes

Add an extended subject for maps

Browse Source 
Truth's MapSubject has some shortcomings due to which tests need to
use cumbersome constructs to do assertions. Hence, add an extension of
it which contains additional, helpful methods. Those allow for
example to omit hasSize() or isNotNull() assertions. Some existing tests
are adjusted to show the use of the new methods.

The new methods will also be helpful for I8689b5701.

Change-Id: I14d75ff75ad7ec6d289b137c3f10db26aef62b57
This commit is contained in:
Alice Kober-Sotzek
2020-03-31 20:58:59 +02:00
committed by David Pursehouse
parent 449b37c102
commit 772880a027
4 changed files with 98 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
java/com/google/gerrit/truth/MapSubject.java Normal file
View File

@@ -0,0 +1,56 @@
/*
* Copyright (C) 2020 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.gerrit.truth;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureMetadata;
import com.google.common.truth.IterableSubject;
import com.google.common.truth.Subject;
import java.util.Map;
/**
* A Truth subject for maps providing additional methods simplifying tests but missing on Truth's
* {@link com.google.common.truth.MapSubject}.
*/
public class MapSubject extends com.google.common.truth.MapSubject {
private final Map<?, ?> map;
public static MapSubject assertThatMap(Map<?, ?> map) {
return assertAbout(mapEntries()).that(map);
}
public static Subject.Factory<MapSubject, Map<?, ?>> mapEntries() {
return MapSubject::new;
}
private MapSubject(FailureMetadata failureMetadata, Map<?, ?> map) {
super(failureMetadata, map);
this.map = map;
}
public IterableSubject keys() {
isNotNull();
return check("keys()").that(map.keySet());
}
public IterableSubject values() {
isNotNull();
return check("values()").that(map.values());
}
}

30
javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
View File

@@ -28,6 +28,7 @@ import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.a
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.MapSubject.assertThatMap;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static java.util.stream.Collectors.toList;
@@ -928,7 +929,8 @@ public class GroupsIT extends AbstractDaemonTest {
List<String> expectedGroups =
groups.getAllGroupReferences().map(GroupReference::getName).sorted().collect(toList());
assertThat(expectedGroups.size()).isAtLeast(2);
assertThat(gApi.groups().list().getAsMap().keySet())
assertThatMap(gApi.groups().list().getAsMap())
.keys()
.containsExactlyElementsIn(expectedGroups)
.inOrder();
}
@@ -975,20 +977,19 @@ public class GroupsIT extends AbstractDaemonTest {
gApi.groups().create(in);
requestScopeOperations.setApiUser(user.id());
assertThat(gApi.groups().list().getAsMap()).doesNotContainKey(newGroupName);
assertThatMap(gApi.groups().list().getAsMap()).keys().doesNotContain(newGroupName);
requestScopeOperations.setApiUser(admin.id());
gApi.groups().id(newGroupName).addMembers(user.username());
requestScopeOperations.setApiUser(user.id());
assertThat(gApi.groups().list().getAsMap()).containsKey(newGroupName);
assertThatMap(gApi.groups().list().getAsMap()).keys().contains(newGroupName);
}
@Test
public void suggestGroup() throws Exception {
Map<String, GroupInfo> groups = gApi.groups().list().withSuggest("adm").getAsMap();
assertThat(groups).containsKey("Administrators");
assertThat(groups).hasSize(1);
assertThatMap(groups).keys().containsExactly("Administrators");
assertBadRequest(gApi.groups().list().withSuggest("adm").withSubstring("foo"));
assertBadRequest(gApi.groups().list().withSuggest("adm").withRegex("foo.*"));
assertBadRequest(gApi.groups().list().withSuggest("adm").withUser("user"));
@@ -1005,19 +1006,15 @@ public class GroupsIT extends AbstractDaemonTest {
// Choose a substring which isn't part of any group or test method within this class.
String substring = "efghijk";
Map<String, GroupInfo> groups = gApi.groups().list().withSubstring(substring).getAsMap();
assertThat(groups).containsKey(group);
assertThat(groups).hasSize(1);
assertThatMap(groups).keys().containsExactly(group);
groups = gApi.groups().list().withSubstring("abcdefghi").getAsMap();
assertThat(groups).containsKey(group);
assertThat(groups).hasSize(1);
assertThatMap(groups).keys().containsExactly(group);
String otherGroup = name("Abcdefghijklmnop2");
gApi.groups().create(otherGroup);
groups = gApi.groups().list().withSubstring(substring).getAsMap();
assertThat(groups).hasSize(2);
assertThat(groups).containsKey(group);
assertThat(groups).containsKey(otherGroup);
assertThatMap(groups).keys().containsExactly(group, otherGroup);
groups = gApi.groups().list().withSubstring("non-existing-substring").getAsMap();
assertThat(groups).isEmpty();
@@ -1026,15 +1023,13 @@ public class GroupsIT extends AbstractDaemonTest {
@Test
public void withRegex() throws Exception {
Map<String, GroupInfo> groups = gApi.groups().list().withRegex("Admin.*").getAsMap();
assertThat(groups).containsKey("Administrators");
assertThat(groups).hasSize(1);
assertThatMap(groups).keys().containsExactly("Administrators");
groups = gApi.groups().list().withRegex("admin.*").getAsMap();
assertThat(groups).isEmpty();
groups = gApi.groups().list().withRegex(".*istrators").getAsMap();
assertThat(groups).containsKey("Administrators");
assertThat(groups).hasSize(1);
assertThatMap(groups).keys().containsExactly("Administrators");
assertBadRequest(gApi.groups().list().withRegex(".*istrators").withSubstring("s"));
}
@@ -1043,8 +1038,7 @@ public class GroupsIT extends AbstractDaemonTest {
public void allGroupInfoFieldsSetCorrectly() throws Exception {
InternalGroup adminGroup = adminGroup();
Map<String, GroupInfo> groups = gApi.groups().list().addGroup(adminGroup.getName()).getAsMap();
assertThat(groups).hasSize(1);
assertThat(groups).containsKey("Administrators");
assertThatMap(groups).keys().containsExactly("Administrators");
assertGroupInfo(adminGroup, Iterables.getOnlyElement(groups.values()));
}

18
javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
View File

@@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.extensions.client.ListChangesOption.CHANGE_ACTIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_ACTIONS;
import static com.google.gerrit.extensions.client.ListChangesOption.CURRENT_REVISION;
import static com.google.gerrit.truth.MapSubject.assertThatMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -89,21 +90,16 @@ public class ActionsIT extends AbstractDaemonTest {
gApi.changes().id(changeId2).current().review(ReviewInput.approve());
gApi.changes().id(changeId2).current().submit();
Map<String, ActionInfo> actions1 = getChangeActions(changeId1);
assertThat(actions1).containsKey("revert");
assertThat(actions1).containsKey("revert_submission");
assertThatMap(actions1).keys().containsAtLeast("revert", "revert_submission");
Map<String, ActionInfo> actions2 = getChangeActions(changeId2);
assertThat(actions2).containsKey("revert");
assertThat(actions2).containsKey("revert_submission");
assertThatMap(actions2).keys().containsAtLeast("revert", "revert_submission");
}
@Test
public void revisionActionsOneChangePerTopicUnapproved() throws Exception {
String changeId = createChangeWithTopic().getChangeId();
Map<String, ActionInfo> actions = getActions(changeId);
assertThat(actions).hasSize(3);
assertThat(actions).containsKey("cherrypick");
assertThat(actions).containsKey("rebase");
assertThat(actions).containsKey("description");
assertThatMap(actions).keys().containsExactly("cherrypick", "rebase", "description");
}
@Test
@@ -507,11 +503,7 @@ public class ActionsIT extends AbstractDaemonTest {
}
private void commonActionsAssertions(Map<String, ActionInfo> actions) {
assertThat(actions).hasSize(4);
assertThat(actions).containsKey("cherrypick");
assertThat(actions).containsKey("submit");
assertThat(actions).containsKey("description");
assertThat(actions).containsKey("rebase");
assertThatMap(actions).keys().containsExactly("cherrypick", "submit", "description", "rebase");
}
private PushOneCommit.Result createChangeWithTopic() throws Exception {

66
javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
View File

@@ -20,6 +20,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.ConfigSubject.assertThat;
import static com.google.gerrit.truth.MapSubject.assertThatMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.ExtensionRegistry;
@@ -430,7 +431,8 @@ public class AccessIT extends AbstractDaemonTest {
accessInput.add.put(REFS_ALL, accessSection);
ProjectAccessInfo result = pApi().access(accessInput);
assertThat(result.groups.keySet())
assertThatMap(result.groups)
.keys()
.containsExactly(
SystemGroupBackend.PROJECT_OWNERS.get(), SystemGroupBackend.ANONYMOUS_USERS.get());
@@ -443,7 +445,8 @@ public class AccessIT extends AbstractDaemonTest {
// Get call returns groups too.
ProjectAccessInfo loggedInResult = pApi().access();
assertThat(loggedInResult.groups.keySet())
assertThatMap(loggedInResult.groups)
.keys()
.containsExactly(
SystemGroupBackend.PROJECT_OWNERS.get(), SystemGroupBackend.ANONYMOUS_USERS.get());
@@ -456,7 +459,8 @@ public class AccessIT extends AbstractDaemonTest {
// PROJECT_OWNERS is invisible to anonymous user, but GetAccess disregards visibility.
requestScopeOperations.setApiUserAnonymous();
ProjectAccessInfo anonResult = pApi().access();
assertThat(anonResult.groups.keySet())
assertThatMap(anonResult.groups)
.keys()
.containsExactly(
SystemGroupBackend.PROJECT_OWNERS.get(), SystemGroupBackend.ANONYMOUS_USERS.get());
}
@@ -510,12 +514,8 @@ public class AccessIT extends AbstractDaemonTest {
ProjectAccessInfo updatedAccessSectionInfo =
gApi.projects().name(allProjects.get()).access(accessInput);
assertThat(
updatedAccessSectionInfo
.local
.get(AccessSection.GLOBAL_CAPABILITIES)
.permissions
.keySet())
assertThatMap(updatedAccessSectionInfo.local.get(AccessSection.GLOBAL_CAPABILITIES).permissions)
.keys()
.containsAtLeastElementsIn(accessSectionInfo.permissions.keySet());
}
@@ -544,12 +544,9 @@ public class AccessIT extends AbstractDaemonTest {
ProjectAccessInfo updatedAccessSectionInfo =
gApi.projects().name(allProjects.get()).access(accessInput);
assertThat(
updatedAccessSectionInfo
.local
.get(AccessSection.GLOBAL_CAPABILITIES)
.permissions
.keySet())
assertThatMap(
updatedAccessSectionInfo.local.get(AccessSection.GLOBAL_CAPABILITIES).permissions)
.keys()
.containsAtLeastElementsIn(accessSectionInfo.permissions.keySet());
}
}
@@ -644,12 +641,8 @@ public class AccessIT extends AbstractDaemonTest {
ProjectAccessInfo updatedProjectAccessInfo =
gApi.projects().name(allProjects.get()).access(accessInput);
assertThat(
updatedProjectAccessInfo
.local
.get(AccessSection.GLOBAL_CAPABILITIES)
.permissions
.keySet())
assertThatMap(updatedProjectAccessInfo.local.get(AccessSection.GLOBAL_CAPABILITIES).permissions)
.keys()
.containsAtLeastElementsIn(accessSectionInfo.permissions.keySet());
// Remove
@@ -657,12 +650,8 @@ public class AccessIT extends AbstractDaemonTest {
accessInput.remove.put(AccessSection.GLOBAL_CAPABILITIES, accessSectionInfo);
updatedProjectAccessInfo = gApi.projects().name(allProjects.get()).access(accessInput);
assertThat(
updatedProjectAccessInfo
.local
.get(AccessSection.GLOBAL_CAPABILITIES)
.permissions
.keySet())
assertThatMap(updatedProjectAccessInfo.local.get(AccessSection.GLOBAL_CAPABILITIES).permissions)
.keys()
.containsNoneIn(accessSectionInfo.permissions.keySet());
}
@@ -754,14 +743,12 @@ public class AccessIT extends AbstractDaemonTest {
// Assert that the permission was synced from All-Projects (global) to All-Users (ref)
Map<String, AccessSectionInfo> local = gApi.projects().name("All-Users").access().local;
assertThat(local).isNotNull();
assertThat(local).containsKey(RefNames.REFS_GROUPS + "*");
assertThatMap(local).keys().contains(RefNames.REFS_GROUPS + "*");
Map<String, PermissionInfo> permissions = local.get(RefNames.REFS_GROUPS + "*").permissions;
assertThat(permissions).hasSize(2);
// READ is the default permission and should be preserved by the syncer
assertThat(permissions.keySet()).containsExactly(Permission.READ, Permission.CREATE);
assertThatMap(permissions).keys().containsExactly(Permission.READ, Permission.CREATE);
Map<String, PermissionRuleInfo> rules = permissions.get(Permission.CREATE).rules;
assertThat(rules.values()).containsExactly(pri);
assertThatMap(rules).values().containsExactly(pri);
// Revoke the permission
accessInput.add.clear();
@@ -770,12 +757,10 @@ public class AccessIT extends AbstractDaemonTest {
// Assert that the permission was synced from All-Projects (global) to All-Users (ref)
Map<String, AccessSectionInfo> local2 = gApi.projects().name("All-Users").access().local;
assertThat(local2).isNotNull();
assertThat(local2).containsKey(RefNames.REFS_GROUPS + "*");
assertThatMap(local2).keys().contains(RefNames.REFS_GROUPS + "*");
Map<String, PermissionInfo> permissions2 = local2.get(RefNames.REFS_GROUPS + "*").permissions;
assertThat(permissions2).hasSize(1);
// READ is the default permission and should be preserved by the syncer
assertThat(permissions2.keySet()).containsExactly(Permission.READ);
assertThatMap(permissions2).keys().containsExactly(Permission.READ);
}
@Test
@@ -803,14 +788,13 @@ public class AccessIT extends AbstractDaemonTest {
// Assert that the permissions were synced from All-Projects (global) to All-Users (ref)
Map<String, AccessSectionInfo> local = gApi.projects().name("All-Users").access().local;
assertThat(local).isNotNull();
assertThat(local).containsKey(RefNames.REFS_GROUPS + "*");
assertThatMap(local).keys().contains(RefNames.REFS_GROUPS + "*");
Map<String, PermissionInfo> permissions = local.get(RefNames.REFS_GROUPS + "*").permissions;
assertThat(permissions).hasSize(2);
// READ is the default permission and should be preserved by the syncer
assertThat(permissions.keySet()).containsExactly(Permission.READ, Permission.CREATE);
assertThatMap(permissions).keys().containsExactly(Permission.READ, Permission.CREATE);
Map<String, PermissionRuleInfo> rules = permissions.get(Permission.CREATE).rules;
assertThat(rules.keySet())
assertThatMap(rules)
.keys()
.containsExactly(SystemGroupBackend.REGISTERED_USERS.get(), adminGroupUuid().get());
assertThat(rules.get(SystemGroupBackend.REGISTERED_USERS.get())).isEqualTo(pri);
assertThat(rules.get(adminGroupUuid().get())).isEqualTo(pri);