pivotal-jbarrett commented on code in PR #7378:
URL: https://github.com/apache/geode/pull/7378#discussion_r857784455
##########
geode-core/src/test/java/org/apache/geode/distributed/internal/LocatorLoadSnapshotJUnitTest.java:
##########
@@ -49,9 +48,10 @@ public class LocatorLoadSnapshotJUnitTest {
@Test
Review Comment:
Please rename this test to the current convention, `LocatorLoadSnapshotTest`.
##########
geode-core/src/test/java/org/apache/geode/distributed/internal/LocatorLoadSnapshotJUnitTest.java:
##########
@@ -837,11 +899,11 @@ public void
testRemoveFromMapWithServerLocationAndGroups() {
loadSnapshot.removeFromMap(map, new String[] {"a"}, sl1);
- assertEquals(2, groupServers.size());
- assertNull(groupServers.get(sl1));
+ assertThat(groupServers.size()).isEqualTo(2);
+ assertThat(groupServers.get(sl1)).isNull();
Review Comment:
If you are asserting that the map lost this key?
```java
assertThat(groupServers).doesNotContainKeys(sl1);
```
##########
geode-core/src/test/java/org/apache/geode/distributed/internal/LocatorLoadSnapshotJUnitTest.java:
##########
@@ -837,11 +899,11 @@ public void
testRemoveFromMapWithServerLocationAndGroups() {
loadSnapshot.removeFromMap(map, new String[] {"a"}, sl1);
- assertEquals(2, groupServers.size());
- assertNull(groupServers.get(sl1));
+ assertThat(groupServers.size()).isEqualTo(2);
Review Comment:
```java
assertThat(groupServers).hasSize(2);
```
##########
geode-core/src/test/java/org/apache/geode/distributed/internal/LocatorLoadSnapshotJUnitTest.java:
##########
@@ -623,36 +626,101 @@ public void testFindBestServersCalledWithNegativeCount()
{
groupServers.put(sli3, loadHolder3);
List<LocatorLoadSnapshot.LoadHolder> result =
- loadSnapshot.findBestServers(groupServers, Collections.EMPTY_SET, -1);
- assertEquals(3, result.size());
- assertEquals(loadHolder2, result.get(0));
- assertEquals(loadHolder3, result.get(1));
- assertEquals(loadHolder1, result.get(2));
+ loadSnapshot.findBestServers(groupServers, Collections.emptySet(), -1);
+ assertThat(result.size()).isEqualTo(3);
Review Comment:
These 4 lines can be replaced with:
```java
assertThat(result).containsExactly(loadHolder2, loadHolder3, loadHolder1);
```
##########
geode-core/src/test/java/org/apache/geode/distributed/internal/LocatorLoadSnapshotJUnitTest.java:
##########
@@ -871,17 +933,17 @@ public void testAddGroupsWithServerLocationAndMemberId() {
loadSnapshot.addGroups(map, new String[] {"a", "b"}, loadHolder2,
uniqueId2);
loadSnapshot.addGroups(map, new String[] {}, loadHolder3, uniqueId3);
- assertEquals(3, map.get(null).size());
- assertEquals(loadHolder1, map.get(null).get(sli1));
- assertEquals(loadHolder2, map.get(null).get(sli2));
- assertEquals(loadHolder3, map.get(null).get(sli3));
+ assertThat(map.get(null).size()).isEqualTo(3);
Review Comment:
These 4 lines can be replaced with:
```java
assertThat(map.get(null)).containsOnly(entry(sli1, loadHolder1), entry(sli2,
loadHolder2), entry(sli3, loadHolder3));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
