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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
