garydgregory commented on code in PR #460:
URL: 
https://github.com/apache/commons-collections/pull/460#discussion_r1510347734


##########
src/test/java/org/apache/commons/collections4/iterators/ListIteratorWrapperTest.java:
##########
@@ -16,10 +16,6 @@
  */
 package org.apache.commons.collections4.iterators;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;

Review Comment:
   Please don't change the import style, we do not use star imports.



##########
src/test/java/org/apache/commons/collections4/bidimap/UnmodifiableBidiMapTest.java:
##########
@@ -96,4 +102,31 @@ public void testUnmodifiable() {
         assertTrue(makeFullMap() instanceof Unmodifiable);
     }
 
+    @Test
+    public void testToArray() {
+        // arrange
+        Set<Map.Entry<K, V>> x = makeCustomFullMap();

Review Comment:
   Use final.
   



##########
src/test/java/org/apache/commons/collections4/ArrayStackTest.java:
##########
@@ -101,6 +101,24 @@ public void testSearch() {
                 "Cannot find 'Missing Item'");
     }
 
+    @Test
+    public void testPeekForItemsDownInStack() {
+        // arrange
+        final ArrayStack<E> stack = makeObject();
+        final ArrayStack<E> emptyStack = makeObject();
+        stack.push((E) "First");
+        stack.push((E) "Second");
+        stack.push((E) "Third");
+
+        // act and assert
+        // check for valid index
+        assertEquals("Second", stack.peek(1));
+        // check for invalid index
+        assertThrows(EmptyStackException.class, () -> {stack.peek(5);});

Review Comment:
   Lambda block is not needed.



##########
src/test/java/org/apache/commons/collections4/iterators/ListIteratorWrapperTest.java:
##########
@@ -223,4 +226,21 @@ public void testReset() {
         }
     }
 
+    @Test
+    public void testAdd() {
+
+        // arrange part starts
+        final ResettableListIterator<E> iter1 = makeCustomObject();
+        final ResettableListIterator<E> iter2 = makeObject();
+        int[] x = new int[]{1};

Review Comment:
   Use final where you can.



##########
src/test/java/org/apache/commons/collections4/bidimap/UnmodifiableBidiMapTest.java:
##########
@@ -16,18 +16,18 @@
  */
 package org.apache.commons.collections4.bidimap;
 
-import static org.junit.jupiter.api.Assertions.assertSame;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.collections4.BidiMap;
 import org.apache.commons.collections4.Unmodifiable;
 import org.apache.commons.collections4.collection.AbstractCollectionTest;
+import org.apache.commons.collections4.map.UnmodifiableEntrySet;
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.*;

Review Comment:
   Please don't change the import style, we do not use star imports.
   



##########
src/test/java/org/apache/commons/collections4/EnumerationUtilsTest.java:
##########
@@ -124,4 +119,30 @@ public void testToListWithStringTokenizer() {
         assertEquals(expectedList2, actualList);
     }
 
+    @Test
+    public void testRemove() {
+        // arrange
+        List<String> list = new ArrayList<>(Arrays.asList("First", "Second", 
"Third"));

Review Comment:
   Use final.



##########
src/test/java/org/apache/commons/collections4/iterators/ListIteratorWrapperTest.java:
##########
@@ -223,4 +226,21 @@ public void testReset() {
         }
     }
 
+    @Test
+    public void testAdd() {
+
+        // arrange part starts
+        final ResettableListIterator<E> iter1 = makeCustomObject();
+        final ResettableListIterator<E> iter2 = makeObject();
+        int[] x = new int[]{1};
+        // arrange part ends
+
+        // act and assert starts
+        // check for no exception
+        assertDoesNotThrow(()->{iter1.add((E) x);});

Review Comment:
   Missing space around arrow, block not needed.
   



##########
src/test/java/org/apache/commons/collections4/ArrayStackTest.java:
##########
@@ -101,6 +101,24 @@ public void testSearch() {
                 "Cannot find 'Missing Item'");
     }
 
+    @Test
+    public void testPeekForItemsDownInStack() {
+        // arrange
+        final ArrayStack<E> stack = makeObject();
+        final ArrayStack<E> emptyStack = makeObject();
+        stack.push((E) "First");
+        stack.push((E) "Second");
+        stack.push((E) "Third");
+
+        // act and assert
+        // check for valid index
+        assertEquals("Second", stack.peek(1));
+        // check for invalid index
+        assertThrows(EmptyStackException.class, () -> {stack.peek(5);});
+        // check if stack is empty.
+        assertThrows(EmptyStackException.class, () -> {emptyStack.peek(1);});

Review Comment:
   Lambda block is not needed.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to