opendev/gerrit
Code Issues Proposed changes

AccessSection/Permission: Change return types to ImmutableList

Browse Source 
These were recently improved in I9a58b2e8 and Ic5514036 to return
immutable copies. Removing them causes ErrorProne failures in the
@Ignored tests, which intentionally try to modify these ImmutableLists.
The ErrorProne failure is testing the same thing as the tests
themselves, and much less verbosely; just remove the tests.

Change-Id: I1492113696166512056a2ce08ce2bff2d254c523
This commit is contained in:
Dave Borowitz
2018-12-03 15:23:42 -08:00
parent 933ec0bea3
commit 6e69d8a410
4 changed files with 2 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/common/data/AccessSection.java
View File

@@ -37,7 +37,7 @@ public class AccessSection extends RefConfigSection implements Comparable<Access
super(refPattern);
}
public List<Permission> getPermissions() {
public ImmutableList<Permission> getPermissions() {
return permissions == null ? ImmutableList.of() : ImmutableList.copyOf(permissions);
}

2
java/com/google/gerrit/common/data/Permission.java
View File

@@ -158,7 +158,7 @@ public class Permission implements Comparable<Permission> {
exclusiveGroup = newExclusiveGroup;
}
public List<PermissionRule> getRules() {
public ImmutableList<PermissionRule> getRules() {
return rules == null ? ImmutableList.of() : ImmutableList.copyOf(rules);
}

17
javatests/com/google/gerrit/common/data/AccessSectionTest.java
View File

@@ -22,7 +22,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
public class AccessSectionTest extends GerritBaseTests {
@@ -152,22 +151,6 @@ public class AccessSectionTest extends GerritBaseTests {
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
}
@Ignore
@Test
public void cannotAddPermissionByModifyingListThatWasRetrievedFromAccessSection() {
Permission submitPermission = new Permission(Permission.SUBMIT);
accessSection.getPermissions().add(submitPermission);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
List<Permission> permissions = new ArrayList<>();
permissions.add(new Permission(Permission.ABANDON));
permissions.add(new Permission(Permission.REBASE));
accessSection.setPermissions(permissions);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
accessSection.getPermissions().add(submitPermission);
assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
}
@Test
public void removePermission() {
Permission abandonPermission = new Permission(Permission.ABANDON);

18
javatests/com/google/gerrit/common/data/PermissionTest.java
View File

@@ -22,7 +22,6 @@ import com.google.gerrit.testing.GerritBaseTests;
import java.util.ArrayList;
import java.util.List;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
public class PermissionTest extends GerritBaseTests {
@@ -187,23 +186,6 @@ public class PermissionTest extends GerritBaseTests {
assertThat(permission.getRule(groupReference3)).isNull();
}
@Ignore
@Test
public void cannotAddPermissionByModifyingListThatWasRetrievedFromAccessSection() {
GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid-1"), "group1");
PermissionRule permissionRule1 = new PermissionRule(groupReference1);
permission.getRules().add(permissionRule1);
assertThat(permission.getRule(groupReference1)).isNull();
List<PermissionRule> rules = new ArrayList<>();
rules.add(new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2")));
rules.add(new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-3"), "group3")));
permission.setRules(rules);
assertThat(permission.getRule(groupReference1)).isNull();
permission.getRules().add(permissionRule1);
assertThat(permission.getRule(groupReference1)).isNull();
}
@Test
public void getNonExistingRule() {
GroupReference groupReference = new GroupReference(new AccountGroup.UUID("uuid-1"), "group1");