[GitHub] [geode] jdeppe-pivotal commented on a diff in pull request #7519: GEODE-10160: fixes SizeableByteArrayList sizing

[GitBox <gi...@apache.org>]

jdeppe-pivotal commented on code in PR #7519:
URL: https://github.com/apache/geode/pull/7519#discussion_r843087114


##########
geode-for-redis/src/test/java/org/apache/geode/redis/internal/data/collections/SizeableByteArrayListTest.java:
##########
@@ -73,22 +74,190 @@ public void removeObjects_getSizeInBytesIsAccurate() {
     assertThat(list.size()).isEqualTo(0);
   }
 
+  @Test
+  public void removeObject_getSizeInBytesIsAccurate() {
+    // Create a list with an initial size and confirm that it correctly reports its size
+    SizeableByteArrayList list = createList();
+    assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
+
+    // Remove all the elements and assert that the size is correct after each remove
+    Random rand = new Random();
+    for (int i = 0; i < INITIAL_NUMBER_OF_ELEMENTS; ++i) {
+      list.remove(makeByteArrayOfSpecifiedLength(i + 1));
+      assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
+    }
+    assertThat(list.size()).isEqualTo(0);
+  }
+
   @Test
   public void removeIndexes_getSizeInBytesIsAccurate() {
     // Create a list with an initial size and confirm that it correctly reports its size
     SizeableByteArrayList list = createList();
     assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
 
+    // Remove all the elements and assert that the size is correct after each remove
+    for (int i = INITIAL_NUMBER_OF_ELEMENTS - 1; 0 <= i;) {
+      // Remove in batches of 5
+      List<Integer> indexesToRemove = new ArrayList<>(5);
+      for (int j = 0; j < 5 && i >= 0; j++) {
+        indexesToRemove.add(0, i--);
+      }
+      list.removeIndexes(indexesToRemove);
+      assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
+    }
+    assertThat(list.size()).isEqualTo(0);
+  }
+
+  @Test
+  public void removeIndex_getSizeInBytesIsAccurate() {
+    // Create a list with an initial size and confirm that it correctly reports its size
+    SizeableByteArrayList list = createList();
+    assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
+
     // Remove all the elements and assert that the size is correct after each remove
     for (int i = INITIAL_NUMBER_OF_ELEMENTS - 1; 0 <= i; --i) {
-      List<Integer> indexToRemove = new ArrayList<>(1);
-      indexToRemove.add(i);
-      list.removeIndexes(indexToRemove);
+      list.remove(i);
       assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
     }
     assertThat(list.size()).isEqualTo(0);
   }
 
+  @Test
+  public void set_getSizeInBytesIsAccurate() {
+    // Create a list with one initial element and confirm that it correctly reports its size
+    SizeableByteArrayList list = new SizeableByteArrayList();
+    byte[] element = "element name".getBytes(StandardCharsets.UTF_8);

Review Comment:
   Done



##########
geode-for-redis/src/test/java/org/apache/geode/redis/internal/data/collections/SizeableByteArrayListTest.java:
##########
@@ -73,22 +74,190 @@ public void removeObjects_getSizeInBytesIsAccurate() {
     assertThat(list.size()).isEqualTo(0);
   }
 
+  @Test
+  public void removeObject_getSizeInBytesIsAccurate() {
+    // Create a list with an initial size and confirm that it correctly reports its size
+    SizeableByteArrayList list = createList();
+    assertThat(list.getSizeInBytes()).isEqualTo(sizer.sizeof(list));
+
+    // Remove all the elements and assert that the size is correct after each remove
+    Random rand = new Random();

Review Comment:
   Removed



-- 
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-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org