You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "samabcde (via GitHub)" <gi...@apache.org> on 2023/04/16 14:21:16 UTC
[GitHub] [commons-collections] samabcde opened a new pull request, #391: [COLLECTIONS-839] migrate Map tests to use junit5 assert
samabcde opened a new pull request, #391:
URL: https://github.com/apache/commons-collections/pull/391
As part of the migration to Junit 5, target to replace all assertion api to Junit 5, for all Map related test class.
So there is no need for `BulkTest` to extend `TestCase`.
--
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
[GitHub] [commons-collections] garydgregory merged pull request #391: [COLLECTIONS-839] migrate Map tests to use junit5 assert
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #391:
URL: https://github.com/apache/commons-collections/pull/391
--
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
[GitHub] [commons-collections] samabcde commented on pull request #391: [COLLECTIONS-839] migrate Map tests to use junit5 assert
Posted by "samabcde (via GitHub)" <gi...@apache.org>.
samabcde commented on PR #391:
URL: https://github.com/apache/commons-collections/pull/391#issuecomment-1519006362
> > > 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.
> >
> >
> > I think it'd be simpler to migrate that part first, or include that in this change? That way we would be sure that tests are passing with JUnit5 API, without running the risk of some JUnit4 API leaking into the tests and failing later when we change the rest of the code.
>
> Agreed.
@garydgregory , @kinow updated as suggested, please kindly help to review.
--
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
[GitHub] [commons-collections] garydgregory commented on pull request #391: [COLLECTIONS-839] migrate Map tests to use junit5 assert
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #391:
URL: https://github.com/apache/commons-collections/pull/391#issuecomment-1515481291
> > 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.
>
> I think it'd be simpler to migrate that part first, or include that in this change? That way we would be sure that tests are passing with JUnit5 API, without running the risk of some JUnit4 API leaking into the tests and failing later when we change the rest of the code.
Agreed.
--
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
[GitHub] [commons-collections] codecov-commenter commented on pull request #391: [COLLECTIONS-839] migrate Map tests to use junit5 assert
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #391:
URL: https://github.com/apache/commons-collections/pull/391#issuecomment-1510401747
## [Codecov](https://codecov.io/gh/apache/commons-collections/pull/391?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#391](https://codecov.io/gh/apache/commons-collections/pull/391?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1a5bb2) into [master](https://codecov.io/gh/apache/commons-collections/commit/d9c8708d6786966089ea991fa0c621854cfe9644?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d9c8708) will **not change** coverage.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #391 +/- ##
=========================================
Coverage 81.19% 81.19%
Complexity 4604 4604
=========================================
Files 288 288
Lines 13423 13423
Branches 1982 1982
=========================================
Hits 10899 10899
Misses 1932 1932
Partials 592 592
```
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: notifications-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] samabcde commented on pull request #391: [COLLECTIONS-839] migrate Map tests to use junit5 assert
Posted by "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