[GitHub] [commons-collections] samabcde commented on pull request #391: [COLLECTIONS-839] migrate Map tests to use junit5 assert

["samabcde (via GitHub)" <gi...@apache.org>]

samabcde commented on PR #391:
URL: https://github.com/apache/commons-collections/pull/391#issuecomment-1514906658

   @garydgregory 
   
   Thx for your comment. The reason I don't use static import is when I use static import, for instance `assertEquals`. 
   The method will actually refer to `junit.framework.TestCase.assertEquals` instead of `org.junit.jupiter.api.Assertions.assertEquals`.
   It only refer to `org.junit.jupiter.api.Assertions.assertEquals` when the argument don't match `junit.framework.TestCase.assertEquals` like `assertEquals(2, 2, "foo")`.
   
   Some possible solution can be:
   1. Use `Assertions.assertXXX` only for those method with `message` argument position changed.
     e.g. `Assertions.assertEquals(expected, actual, message). and keep others using `junit.framework.TestCase`, as they can be changed easily.
   2. Complete all test classes API migration in one PR, hence `BulkTest` does not extend `TestCase` and we can use static import for all assert API.
   
   Which one do you prefer? Or any other idea?
   
    


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