[GitHub] [commons-collections] garydgregory commented on a change in pull request #287: COLLECTIONS-777 JUnit v5

2022-03-06 Thread GitBox


garydgregory commented on a change in pull request #287:
URL: 
https://github.com/apache/commons-collections/pull/287#discussion_r820275806



##
File path: 
src/test/java/org/apache/commons/collections4/CollectionUtilsTest.java
##
@@ -501,26 +499,29 @@ public void exists() {
 
 @Test
 public void extractSingleton() {
-ArrayList coll = null;
-try {
-CollectionUtils.extractSingleton(coll);
-fail("expected NullPointerException from extractSingleton(null)");
-} catch (final NullPointerException e) {
-}
-coll = new ArrayList<>();
-try {
-CollectionUtils.extractSingleton(coll);
-fail("expected IllegalArgumentException from 
extractSingleton(empty)");
-} catch (final IllegalArgumentException e) {
-}
-coll.add("foo");
-assertEquals("foo", CollectionUtils.extractSingleton(coll));
-coll.add("bar");
-try {
-CollectionUtils.extractSingleton(coll);
-fail("expected IllegalArgumentException from extractSingleton(size 
== 2)");
-} catch (final IllegalArgumentException e) {
-}
+assertAll(
+() -> {
+ArrayList collNull = null;
+assertThrows(NullPointerException.class, () -> 
CollectionUtils.extractSingleton(collNull),
+"expected NullPointerException from 
extractSingleton(null)");
+},
+

Review comment:
   No need for an extra blank line to separate method arguments.

##
File path: 
src/test/java/org/apache/commons/collections4/CollectionUtilsTest.java
##
@@ -2284,12 +2251,9 @@ public void testUnionNullColl2() {
 public void testUnmodifiableCollection() {
 final Collection col = 
CollectionUtils.unmodifiableCollection(new ArrayList<>());
 assertTrue(col instanceof UnmodifiableCollection, "Returned object 
should be a UnmodifiableCollection.");
-try {
-CollectionUtils.unmodifiableCollection(null);
-fail("Expecting NullPointerException for null collection.");
-} catch (final NullPointerException ex) {
-// expected
-}
+

Review comment:
   No need for an extra blank line.

##
File path: src/test/java/org/apache/commons/collections4/MapUtilsTest.java
##
@@ -889,11 +856,9 @@ public void testIterableMap() {
 
 @Test
 public void testIterableSortedMap() {
-try {
-MapUtils.iterableSortedMap(null);
-fail("Should throw NullPointerException");
-} catch (final NullPointerException e) {
-}
+assertThrows(NullPointerException.class, () -> 
MapUtils.iterableSortedMap(null),
+"Should throw NullPointerException");
+

Review comment:
   No need for an extra blank line.

##
File path: 
src/test/java/org/apache/commons/collections4/iterators/BoundedIteratorTest.java
##
@@ -364,10 +332,9 @@ public void remove() {
 final Iterator iter = new BoundedIterator<>(mockIterator, 1, 5);
 assertTrue(iter.hasNext());
 assertEquals("b", iter.next());
-try {
-iter.remove();
-fail("Expected UnsupportedOperationException.");
-} catch (final UnsupportedOperationException usoe) { /* Success case */
-}
+

Review comment:
   No need for an extra blank line.

##
File path: 
src/test/java/org/apache/commons/collections4/iterators/ObjectGraphIteratorTest.java
##
@@ -73,51 +74,31 @@ public void testIteratorConstructor_null1() {
 final Iterator it = new ObjectGraphIterator<>(null);
 
 assertFalse(it.hasNext());
-try {
-it.next();
-fail();
-} catch (final NoSuchElementException ex) {
-}
-try {
-it.remove();
-fail();
-} catch (final IllegalStateException ex) {
-}
+
+assertThrows(NoSuchElementException.class, () -> it.next());
+

Review comment:
   No need for an extra blank line.

##
File path: 
src/test/java/org/apache/commons/collections4/iterators/ObjectGraphIteratorTest.java
##
@@ -338,14 +295,12 @@ public void testIteration_Transformed3() {
 assertTrue(it.hasNext());
 assertSame(l5, it.next());
 assertFalse(it.hasNext());
-try {
-it.next();
-fail();
-} catch (final NoSuchElementException ex) {
-}
+

Review comment:
   No need for an extra blank line.

##
File path: 
src/test/java/org/apache/commons/collections4/bloomfilter/hasher/ShapeTest.java
##
@@ -266,123 +222,81 @@ public void constructor_items_probability_Test() {
  */
 @Test
 public void constructor_nm_noName() {
-try {
-new Shape(null, 5, 72);
-fail("Should throw NullPointerException");
-} catch (final NullPoint

[GitHub] [commons-collections] garydgregory commented on a change in pull request #287: COLLECTIONS-777 JUnit v5

2022-03-05 Thread GitBox


garydgregory commented on a change in pull request #287:
URL: 
https://github.com/apache/commons-collections/pull/287#discussion_r820088257



##
File path: src/test/java/org/apache/commons/collections4/ClosureUtilsTest.java
##
@@ -138,18 +135,13 @@ public void testWhileClosure() {
 ClosureUtils.whileClosure(PredicateUtils.uniquePredicate(), 
cmd).execute(null);
 assertEquals(1, cmd.count);
 
-try {
-ClosureUtils.whileClosure(null, ClosureUtils.nopClosure());
-fail();
-} catch (final NullPointerException ex) {}
-try {
-ClosureUtils.whileClosure(FalsePredicate.falsePredicate(), null);
-fail();
-} catch (final NullPointerException ex) {}
-try {
-ClosureUtils.whileClosure(null, null);
-fail();
-} catch (final NullPointerException ex) {}
+assertAll(
+() -> assertThrows(NullPointerException.class, () -> 
ClosureUtils.whileClosure(null, ClosureUtils.nopClosure())),
+

Review comment:
   Why are there extra blank lines between arguments? We don't do this for 
other method calls.




-- 
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-unsubscr...@commons.apache.org

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