You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/10/15 11:40:22 UTC

[GitHub] [commons-collections] nhojpatrick opened a new pull request #191: Junit assert throws

nhojpatrick opened a new pull request #191:
URL: https://github.com/apache/commons-collections/pull/191


   


----------------------------------------------------------------
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.

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



[GitHub] [commons-collections] nhojpatrick closed pull request #191: WIP DO NOT MERGE YET Junit assert throws

Posted by GitBox <gi...@apache.org>.
nhojpatrick closed pull request #191:
URL: https://github.com/apache/commons-collections/pull/191


   


-- 
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: issues-unsubscribe@commons.apache.org

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



[GitHub] [commons-collections] garydgregory commented on a change in pull request #191: WIP DO NOT MERGE YET Junit assert throws

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #191:
URL: https://github.com/apache/commons-collections/pull/191#discussion_r505569975



##########
File path: src/test/java/org/apache/commons/collections4/IteratorUtilsTest.java
##########
@@ -436,9 +442,11 @@ public void testArrayListIterator() {
         }
     }
 
-    @Test(expected = NullPointerException.class)
+    @Test
     public void testAsEnumerationNull() {
-        IteratorUtils.asEnumeration(null);
+        final Executable testMethod = () -> IteratorUtils.asEnumeration(null);
+        final NullPointerException thrown = assertThrows(NullPointerException.class, testMethod);
+        assertThat(thrown.getMessage(), is(equalTo("iterator")));

Review comment:
       -1: This is over the top IMO and gets too deep into white-box testing; this test now asserts way too much, all that we care is that the NPE is thrown, not the details of the message. This test will break as soon as we reimplement the guts of the method.
   ```
   assertThrows(NullPointerException.class, () -> IteratorUtils.asEnumeration(null));
   ```
   is enough
   




----------------------------------------------------------------
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.

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