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 2021/01/16 06:56:02 UTC

[GitHub] [commons-collections] arturobernalg opened a new pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

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


   


----------------------------------------------------------------
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] garydgregory commented on a change in pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

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



##########
File path: src/test/java/org/apache/commons/collections4/iterators/LoopingListIteratorTest.java
##########
@@ -21,8 +21,8 @@
 import java.util.List;
 import java.util.NoSuchElementException;
 
-import org.junit.Test;
-import static org.junit.Assert.*;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;

Review comment:
       @arturobernalg 
   Did you forget to flip the arguments on the `assert*` API calls in this file and subsequent files?




----------------------------------------------------------------
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] garydgregory commented on a change in pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

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



##########
File path: src/test/java/org/apache/commons/collections4/collection/PredicatedCollectionBuilderTest.java
##########
@@ -86,14 +89,14 @@ public void createPredicatedCollectionWithNotNullPredicate() {
     }
 
     private void checkPredicatedCollection1(final Collection<String> collection) {
-        Assert.assertEquals(1, collection.size());
+        assertEquals(1, collection.size());
 
         collection.add("test2");
-        Assert.assertEquals(2, collection.size());
+        assertEquals(2, collection.size());
 
         try {
             collection.add(null);
-            Assert.fail("Expecting IllegalArgumentException for failing predicate!");
+            fail("Expecting IllegalArgumentException for failing predicate!");

Review comment:
       Use assertThrows()

##########
File path: src/test/java/org/apache/commons/collections4/collection/PredicatedCollectionBuilderTest.java
##########
@@ -122,17 +125,17 @@ public void createPredicatedCollectionWithPredicate() {
     }
 
     private void checkPredicatedCollection2(final Collection<Integer> collection) {
-        Assert.assertEquals(2, collection.size());
+        assertEquals(2, collection.size());
 
         try {
             collection.add(4);
-            Assert.fail("Expecting IllegalArgumentException for failing predicate!");
+            fail("Expecting IllegalArgumentException for failing predicate!");

Review comment:
       Use assertThrows()




----------------------------------------------------------------
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] arturobernalg commented on a change in pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

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



##########
File path: src/test/java/org/apache/commons/collections4/iterators/LoopingListIteratorTest.java
##########
@@ -21,8 +21,8 @@
 import java.util.List;
 import java.util.NoSuchElementException;
 
-import org.junit.Test;
-import static org.junit.Assert.*;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;

Review comment:
       HI @garydgregory 
   
   Nop. I don't.  I working on change/migrate the rest of the class. Trying to find out how migrate the BulkTest that extends TestCase hehehe. 
   
   Any advice is more than welcome




----------------------------------------------------------------
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] coveralls commented on pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #209:
URL: https://github.com/apache/commons-collections/pull/209#issuecomment-761616672


   
   [![Coverage Status](https://coveralls.io/builds/36376167/badge)](https://coveralls.io/builds/36376167)
   
   Coverage decreased (-0.05%) to 90.136% when pulling **7f5215ca2e21f07d64d7f83f3c3bca574457ff7a on arturobernalg:feature/COLLECTIONS-779** into **9691a48354d1a97d470b9184bb98da088c97e9d9 on apache:master**.
   


----------------------------------------------------------------
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] garydgregory merged pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #209:
URL: https://github.com/apache/commons-collections/pull/209


   


----------------------------------------------------------------
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] arturobernalg commented on a change in pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

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



##########
File path: src/test/java/org/apache/commons/collections4/collection/PredicatedCollectionBuilderTest.java
##########
@@ -86,14 +89,14 @@ public void createPredicatedCollectionWithNotNullPredicate() {
     }
 
     private void checkPredicatedCollection1(final Collection<String> collection) {
-        Assert.assertEquals(1, collection.size());
+        assertEquals(1, collection.size());
 
         collection.add("test2");
-        Assert.assertEquals(2, collection.size());
+        assertEquals(2, collection.size());
 
         try {
             collection.add(null);
-            Assert.fail("Expecting IllegalArgumentException for failing predicate!");
+            fail("Expecting IllegalArgumentException for failing predicate!");

Review comment:
       Done

##########
File path: src/test/java/org/apache/commons/collections4/collection/PredicatedCollectionBuilderTest.java
##########
@@ -122,17 +125,17 @@ public void createPredicatedCollectionWithPredicate() {
     }
 
     private void checkPredicatedCollection2(final Collection<Integer> collection) {
-        Assert.assertEquals(2, collection.size());
+        assertEquals(2, collection.size());
 
         try {
             collection.add(4);
-            Assert.fail("Expecting IllegalArgumentException for failing predicate!");
+            fail("Expecting IllegalArgumentException for failing predicate!");

Review comment:
       Done




----------------------------------------------------------------
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] garydgregory commented on a change in pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

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



##########
File path: src/test/java/org/apache/commons/collections4/iterators/LoopingListIteratorTest.java
##########
@@ -21,8 +21,8 @@
 import java.util.List;
 import java.util.NoSuchElementException;
 
-import org.junit.Test;
-import static org.junit.Assert.*;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;

Review comment:
       I don't have specific advice, that is some older code initially based on JUnit 3.




----------------------------------------------------------------
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] arturobernalg commented on a change in pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

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



##########
File path: src/test/java/org/apache/commons/collections4/iterators/LoopingListIteratorTest.java
##########
@@ -21,8 +21,8 @@
 import java.util.List;
 import java.util.NoSuchElementException;
 
-import org.junit.Test;
-import static org.junit.Assert.*;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;

Review comment:
       I can imagine. My idea it's split the migration in several PR.  In that way i can reduce the number of file to review and reduce the errors




----------------------------------------------------------------
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] arturobernalg commented on a change in pull request #209: [COLLECTIONS-779] - Migrate assert to Assertions

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



##########
File path: src/test/java/org/apache/commons/collections4/iterators/LoopingListIteratorTest.java
##########
@@ -21,8 +21,8 @@
 import java.util.List;
 import java.util.NoSuchElementException;
 
-import org.junit.Test;
-import static org.junit.Assert.*;
+import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.*;

Review comment:
       HI @garydgregory  
   
   I can imagine. My idea it's split the migration in several PR.  In that way i can reduce the number of file to review and reduce the errors




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