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 2022/03/02 22:14:07 UTC
[GitHub] [commons-collections] kinow commented on a change in pull request #282: JUnit v5 assertThrows no2
kinow commented on a change in pull request #282:
URL: https://github.com/apache/commons-collections/pull/282#discussion_r818145413
##########
File path: src/test/java/org/apache/commons/collections4/comparators/ComparatorChainTest.java
##########
@@ -67,11 +68,8 @@ public void testBadNoopComparatorChain() {
final ComparatorChain<Integer> chain = new ComparatorChain<>();
final Integer i1 = 4;
final Integer i2 = 6;
- try {
- chain.compare(i1, i2);
- fail("An exception should be thrown when a chain contains zero comparators.");
- } catch (final UnsupportedOperationException e) {
- }
+
+ assertThrows(UnsupportedOperationException.class, () -> chain.compare(i1, i2));
Review comment:
I think the error message would be helpful in this case :point_up: maybe add that to the `assertThrows`
##########
File path: src/test/java/org/apache/commons/collections4/iterators/ListIteratorWrapper2Test.java
##########
@@ -109,11 +110,7 @@ public void testRemove() {
assertEquals(-1, iter.previousIndex());
assertEquals(0, iter.nextIndex());
- try {
- iter.remove();
- fail("ListIteratorWrapper#remove() should fail; must be initially positioned first");
- } catch (final IllegalStateException e) {
- }
+ assertThrows(IllegalStateException.class, () -> iter.remove());
Review comment:
Ditto here...
##########
File path: src/test/java/org/apache/commons/collections4/iterators/ArrayListIteratorTest.java
##########
@@ -104,15 +105,8 @@ public void testListIteratorSet() {
// a call to set() before a call to next() or previous() should throw an IllegalStateException
iter = makeArrayListIterator(testArray);
- try {
- iter.set((E) "should fail");
- fail("ListIterator#set should fail if next() or previous() have not yet been called.");
- } catch (final IllegalStateException e) {
- // expected
- } catch (final Throwable t) { // should never happen
- fail(t.toString());
- }
-
+ ListIterator<E> finalIter = iter;
+ assertThrows(IllegalStateException.class, () -> finalIter.set((E) "should fail"));
Review comment:
Would be good to preserve this error message too, IMO.
##########
File path: src/test/java/org/apache/commons/collections4/collection/PredicatedCollectionTest.java
##########
@@ -99,12 +98,9 @@ public void testIllegalAddAll() {
elements.add((E) "two");
elements.add((E) Integer.valueOf(3));
elements.add((E) "four");
- try {
- c.addAll(elements);
- fail("Integer should fail string predicate.");
- } catch (final IllegalArgumentException e) {
- // expected
- }
+
+ assertThrows(IllegalArgumentException.class, () -> c.addAll(elements));
Review comment:
Maybe keep the `fail` message here.
##########
File path: src/test/java/org/apache/commons/collections4/iterators/ListIteratorWrapper2Test.java
##########
@@ -134,11 +131,7 @@ public void testRemove() {
assertEquals(-1, iter.previousIndex());
assertEquals(0, iter.nextIndex());
- try {
- iter.remove();
- fail("ListIteratorWrapper#remove() should fail; must be repositioned first");
- } catch (final IllegalStateException e) {
- }
+ assertThrows(IllegalStateException.class, () -> iter.remove());
Review comment:
And here...
##########
File path: src/test/java/org/apache/commons/collections4/collection/PredicatedCollectionTest.java
##########
@@ -82,12 +84,9 @@ public PredicatedCollectionTest(final String name) {
public void testIllegalAdd() {
final Collection<E> c = makeTestCollection();
final Integer i = 3;
- try {
- c.add((E) i);
- fail("Integer should fail string predicate.");
- } catch (final IllegalArgumentException e) {
- // expected
- }
+
+ assertThrows(IllegalArgumentException.class, () -> c.add((E) i));
Review comment:
Maybe keep the `fail` message here.
##########
File path: src/test/java/org/apache/commons/collections4/iterators/ObjectArrayListIteratorTest.java
##########
@@ -101,15 +102,8 @@ public void testListIteratorSet() {
// a call to set() before a call to next() or previous() should throw an IllegalStateException
iter = makeArrayListIterator((E[]) testArray);
- try {
- iter.set((E) "should fail");
- fail("ListIterator#set should fail if next() or previous() have not yet been called.");
- } catch (final IllegalStateException e) {
- // expected
- } catch (final Throwable t) { // should never happen
- fail(t.toString());
- }
-
+ ListIterator<E> finalIter = iter;
+ assertThrows(IllegalStateException.class, () -> finalIter.set((E) "should fail"));
Review comment:
Ditto.
--
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