-
Notifications
You must be signed in to change notification settings - Fork 641
[ISSUE #4478] Upgrade JUnit to JUnit Jupiter #4475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adding empty arguments adds no value, and will just interfere with future efforts to migrate to JUnit Jupiter.
PowerMockito is known to be slow and cumbersome compared to Mockito. In most cases, removing it was a straight forward change of removing the PowerMockito annotations and use the MockitoJUnitRunner. In the single non-trivial case, WebHookProcessorTest, PowerMockito's WhiteBox was removed and replaced with Mockito's @Injectmocks annotation.
mockito-inline is intended to replace mockito-core in cerain situations (e.g., mocking static or final classes), and there's no need to have them both as dependencies in the same module. This patch cleans up these duplications and leaves a "slimmer" dependency tree with just the required mockito dependency in each module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to the Apache EventMesh community!!
This is your first PR in our project. We're very excited to have you onboard contributing. Your contributions are greatly appreciated!
Please make sure that the changes are covered by tests.
We will be here shortly.
Let us know if you need any help!
Want to get closer to the community?
| WeChat Assistant | WeChat Public Account | Slack |
|---|---|---|
![]() |
![]() |
Join Slack Chat |
Mailing Lists:
| Name | Description | Subscribe | Unsubscribe | Archive |
|---|---|---|---|---|
| Users | User support and questions mailing list | Subscribe | Unsubscribe | Mail Archives |
| Development | Development related discussions | Subscribe | Unsubscribe | Mail Archives |
| Commits | All commits to repositories | Subscribe | Unsubscribe | Mail Archives |
| Issues | Issues or PRs comments and reviews | Subscribe | Unsubscribe | Mail Archives |
Codecov Report
@@ Coverage Diff @@
## master #4475 +/- ##
============================================
- Coverage 15.58% 15.57% -0.01%
+ Complexity 1475 1474 -1
============================================
Files 698 698
Lines 28205 28205
Branches 2633 2633
============================================
- Hits 4396 4394 -2
- Misses 23361 23362 +1
- Partials 448 449 +1 see 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
mxsm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mureinik Please fix CI
|
Need to create an issue for this pr and link to this pr. |
Pil0tXia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please create a corresponding issue and standardize your PR title and Fixes.
In its current form, the project has some JUnit 4 and some JUnit 5
(Jupiter) tests. This patch aims to align all the tests to a single
modern framework, JUnit Jupiter, in order to make future development
easier.
As this patch is already pretty large as-is, it attempts to be
non-opinionated and simply replace JUnit 4 calls with the closed JUnit
Jupiter equivalents. Subsequent work may want to change some tests to
take further advantage of JUnit Jupiter's features.
This patch includes the following changes:
1. Gradle dependencies:
a. All the dependencies under org.junit.jupiter were consolidated
to use the single artifact org.junit.jupiter:junit-jupiter.
b. The junit:junit dependency was removed in favor of
org.junit.jupiter:junit-jupiter as mentioned in 1.a..
c. The org.mockito:mockito-junit-jupiter dependency was added to
provide the integration with Mockito.
d. The com.github.stefanbirkner:system-rules dependency was
removed in favor of org.junit-pioneer:junit-pioneer that was
used to provide the same functionality as mentioned in 2.i.
2. Annotations
a. org.junit.jupiter.api.BeforeEach was used as a drop-in
replacement for org.junit.Before.
b. org.junit.jupiter.api.BeforeAll was used as a drop-in
replacement for org.junit.BeforeClass.
c. org.junit.jupiter.api.AfterEach was used as a drop-in
replacement for org.junit.After.
d. org.junit.jupiter.api.AfterAll was used as a drop-in
replacement for org.junit.AfterClass.
e. org.junit.jupiter.api.AfterEach was used as a drop-in
replacement for org.junit.After.
f. org.junit.jupiter.api.Disabled was used as a drop-in
replacement for org.junit.Ignore.
g. org.junit.jupiter.api.Test was used as a replacement for
org.junit.Test, although with some caveats:
1. For the simple case with no arguments,
org.junit.jupiter.api.Test was used as a drop-in replacement
for org.junit.Test.
2. For the case where org.junit.Test was used with a timeout
argument, a combination of org.junit.jupiter.api.Test and
org.junit.jupiter.api.Timeout was used.
3. For the case where org.junit.Test was used with an expected
argument, org.junit.jupiter.api.Test was used, but the
assertion on the exception begin thrown is done explicitly in
the test's code, see 3.e. below.
h. org.junit.jupiter.api.extension.ExtendWith was used as a
replacement for org.junit.runner.RunWith with an extension
corresponding to the runner being replaced.
a. org.mockito.junit.jupiter.MockitoExtension was used to
provide the same functionality as
org.mockito.junit.MockitoJUnitRunner. Since the extension is
stricter than the runner, in some cases
org.mockito.junit.jupiter.MockitoSettings was used to
explicitly make the mocking more lenient.
i. org.junitpioneer.jupiter.SetEnvironmentVariable was used in
order to set environment variables in the tests instead of
explicitly calling
org.junit.contrib.java.lang.system.EnvironmentVariables in the
test's body. As an added bonus, using this annotation also
cleans up the changes to the environment variables when the test
is over and prevents it from inadvertently effecting other
tests.
3. Assertions
a. org.junit.jupiter.api.Assertions was used as a drop-in
replacement for org.junit.Assert for the case where the
assertion was performed without a message.
b. org.junit.jupiter.api.Assertions was used instead of
org.junit.Assert in the cases where an assertion method was used
with a message, but the argument were permuted to fit the new
method's signature.
c. org.junit.jupiter.api.Assertions does not have an equivalent of
org.junit.Assert's assertThat method, but luckily it was only
used in a few places, and always used the
org.hamcrest.CoreMatchers.is matcher. These assertions were
rewritten as straight-forward assertEquals assertions.
c. org.junit.jupiter.api.Assertions does not have an equivalent of
org.junit.Assert's assertThat method, but luckily it was only
used in a few places, and always used the
org.hamcrest.CoreMatchers.is matcher. These assertions were
rewritten as straight-forward assertEquals assertions.
d. org.junit.jupiter.api.Assertions does not have an equivalent of
org.hamcrest.MatcherAssert's assertThat method, but luckily it
was only used in a few places, and always used the
org.hamcrest.CoreMatchers.is matcher. These assertions were
rewritten as straight-forward assertEquals assertions.
e. org.junit.jupiter.api.Assertions' assertThrows was used to
assert an expected exception is throws instead of using
org.junit.Test with the expected annotation.
b0969d7 to
0a327cf
Compare
@Pil0tXia once the explicit dependency on @Pil0tXia / @mxsm does the PR need to be re-approved in order for the CI to run again? |
@Pil0tXia done. |
mxsm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* Fix JUnit dependecy issue in eventmesh-storage-kafka * Clean up JUnit dependency in eventmesh-protocol-grpc * Remove empty arguments to @test annotations Adding empty arguments adds no value, and will just interfere with future efforts to migrate to JUnit Jupiter. * Remove PowerMockito usages PowerMockito is known to be slow and cumbersome compared to Mockito. In most cases, removing it was a straight forward change of removing the PowerMockito annotations and use the MockitoJUnitRunner. In the single non-trivial case, WebHookProcessorTest, PowerMockito's WhiteBox was removed and replaced with Mockito's @Injectmocks annotation. * Clean up Mockito dependencies mockito-inline is intended to replace mockito-core in cerain situations (e.g., mocking static or final classes), and there's no need to have them both as dependencies in the same module. This patch cleans up these duplications and leaves a "slimmer" dependency tree with just the required mockito dependency in each module. * Remove unneeded "org.apache.rocketmq:rocketmq-test dependency * Migrate testing to JUnit Jupiter In its current form, the project has some JUnit 4 and some JUnit 5 (Jupiter) tests. This patch aims to align all the tests to a single modern framework, JUnit Jupiter, in order to make future development easier. As this patch is already pretty large as-is, it attempts to be non-opinionated and simply replace JUnit 4 calls with the closed JUnit Jupiter equivalents. Subsequent work may want to change some tests to take further advantage of JUnit Jupiter's features. This patch includes the following changes: 1. Gradle dependencies: a. All the dependencies under org.junit.jupiter were consolidated to use the single artifact org.junit.jupiter:junit-jupiter. b. The junit:junit dependency was removed in favor of org.junit.jupiter:junit-jupiter as mentioned in 1.a.. c. The org.mockito:mockito-junit-jupiter dependency was added to provide the integration with Mockito. d. The com.github.stefanbirkner:system-rules dependency was removed in favor of org.junit-pioneer:junit-pioneer that was used to provide the same functionality as mentioned in 2.i. 2. Annotations a. org.junit.jupiter.api.BeforeEach was used as a drop-in replacement for org.junit.Before. b. org.junit.jupiter.api.BeforeAll was used as a drop-in replacement for org.junit.BeforeClass. c. org.junit.jupiter.api.AfterEach was used as a drop-in replacement for org.junit.After. d. org.junit.jupiter.api.AfterAll was used as a drop-in replacement for org.junit.AfterClass. e. org.junit.jupiter.api.AfterEach was used as a drop-in replacement for org.junit.After. f. org.junit.jupiter.api.Disabled was used as a drop-in replacement for org.junit.Ignore. g. org.junit.jupiter.api.Test was used as a replacement for org.junit.Test, although with some caveats: 1. For the simple case with no arguments, org.junit.jupiter.api.Test was used as a drop-in replacement for org.junit.Test. 2. For the case where org.junit.Test was used with a timeout argument, a combination of org.junit.jupiter.api.Test and org.junit.jupiter.api.Timeout was used. 3. For the case where org.junit.Test was used with an expected argument, org.junit.jupiter.api.Test was used, but the assertion on the exception begin thrown is done explicitly in the test's code, see 3.e. below. h. org.junit.jupiter.api.extension.ExtendWith was used as a replacement for org.junit.runner.RunWith with an extension corresponding to the runner being replaced. a. org.mockito.junit.jupiter.MockitoExtension was used to provide the same functionality as org.mockito.junit.MockitoJUnitRunner. Since the extension is stricter than the runner, in some cases org.mockito.junit.jupiter.MockitoSettings was used to explicitly make the mocking more lenient. i. org.junitpioneer.jupiter.SetEnvironmentVariable was used in order to set environment variables in the tests instead of explicitly calling org.junit.contrib.java.lang.system.EnvironmentVariables in the test's body. As an added bonus, using this annotation also cleans up the changes to the environment variables when the test is over and prevents it from inadvertently effecting other tests. 3. Assertions a. org.junit.jupiter.api.Assertions was used as a drop-in replacement for org.junit.Assert for the case where the assertion was performed without a message. b. org.junit.jupiter.api.Assertions was used instead of org.junit.Assert in the cases where an assertion method was used with a message, but the argument were permuted to fit the new method's signature. c. org.junit.jupiter.api.Assertions does not have an equivalent of org.junit.Assert's assertThat method, but luckily it was only used in a few places, and always used the org.hamcrest.CoreMatchers.is matcher. These assertions were rewritten as straight-forward assertEquals assertions. c. org.junit.jupiter.api.Assertions does not have an equivalent of org.junit.Assert's assertThat method, but luckily it was only used in a few places, and always used the org.hamcrest.CoreMatchers.is matcher. These assertions were rewritten as straight-forward assertEquals assertions. d. org.junit.jupiter.api.Assertions does not have an equivalent of org.hamcrest.MatcherAssert's assertThat method, but luckily it was only used in a few places, and always used the org.hamcrest.CoreMatchers.is matcher. These assertions were rewritten as straight-forward assertEquals assertions. e. org.junit.jupiter.api.Assertions' assertThrows was used to assert an expected exception is throws instead of using org.junit.Test with the expected annotation. * Fix dependency check




Fixes #4478
Motivation
In its current form, the project has some JUnit 4 and some JUnit 5 (Jupiter) tests. This PR aims to align all the tests to a single modern framework, JUnit Jupiter, in order to make future development easier.
Modifications
As this PR is already pretty large as-is, it attempts to be non-opinionated and simply replace JUnit 4 calls with the closed JUnit Jupiter equivalents. Subsequent work may want to change some tests to take further advantage of JUnit Jupiter's features.
This PR includes the following changes:
Gradle dependencies:
org.junit.jupiterwere consolidated to use the single artifactorg.junit.jupiter:junit-jupiter.junit:junitdependency was removed in favor oforg.junit.jupiter:junit-jupitermentioned in 1.i.org.mockito:mockito-coreandorg.mockito:mockito-inlineduplications were cleaned up, and each module was left only with the relevant dependency.org.mockito:mockito-junit-jupiterdependency was added to provide the integration with Mockito.org.powermock:powermock-module-junit4dependency was removed.org.powermock:powermock-api-mockito2dependency was removed.com.github.stefanbirkner:system-rulesdependency was removed in favor oforg.junit-pioneer:junit-pioneerthat was used to provide the same functionality as mentioned in 2.ix."org.apache.rocketmq:rocketmq-testdependency was never really used, and was removedAnnotations
org.junit.jupiter.api.BeforeEachwas used as a drop-in replacement fororg.junit.Before.org.junit.jupiter.api.BeforeAllwas used as a drop-in replacement fororg.junit.BeforeClass.org.junit.jupiter.api.AfterEachwas used as a drop-in replacement fororg.junit.After.org.junit.jupiter.api.AfterAllwas used as a drop-in replacement fororg.junit.AfterClass.org.junit.jupiter.api.AfterEachwas used as a drop-in replacement fororg.junit.After.org.junit.jupiter.api.Disabledwas used as a drop-in replacement fororg.junit.Ignore.org.junit.jupiter.api.Testwas used as a replacement fororg.junit.Test, although with some caveats:org.junit.jupiter.api.Testwas used as a drop-in replacement fororg.junit.Test.org.junit.Testwas used with atimeoutargument, a combination oforg.junit.jupiter.api.Testandorg.junit.jupiter.api.Timeoutwas used.org.junit.Testwas used with anexpectedargument,org.junit.jupiter.api.Testwas used, but the assertion on the exception begin thrown is done explicitly in the test's code, see 3.i. below.org.junit.jupiter.api.extension.ExtendWithwas used as a replacement fororg.junit.runner.RunWithwith an extension corresponding to the runner being replaced.org.mockito.junit.jupiter.MockitoExtensionwas used to provide the same functionality asorg.mockito.junit.MockitoJUnitRunner. Since the extension is stricter than the runner, in some casesorg.mockito.junit.jupiter.MockitoSettingswas used to explicitly make the mocking more lenient.org.mockito.junit.jupiter.MockitoExtensionwas used to replace usages oforg.powermock.modules.junit4.PowerMockRunnerand its associated annotations.org.junitpioneer.jupiter.SetEnvironmentVariablewas used in order to set environment variables in the tests instead ofexplicitly calling
org.junit.contrib.java.lang.system.EnvironmentVariablesin the test's body. As an added bonus, using this annotation also cleans up the changes to the environment variables when the test is over and prevents it from inadvertently effecting other tests.Assertions
org.junit.jupiter.api.Assertionswas used as a drop-in replacement fororg.junit.Assertfor the case where the assertion was performed without a message.org.junit.jupiter.api.Assertionswas used instead oforg.junit.Assertin the cases where an assertion method was used with a message, but the argument were permuted to fit the new method's signature.org.junit.jupiter.api.Assertionsdoes not have an equivalent oforg.junit.Assert'sassertThatmethod, but luckily it was only used in a few places, and always used theorg.hamcrest.CoreMatchers.ismatcher. These assertions were rewritten as straight-forwardassertEqualsassertions.org.junit.jupiter.api.Assertionsdoes not have an equivalent oforg.hamcrest.MatcherAssert'sassertThatmethod, but luckily it was only used in a few places, and always used theorg.hamcrest.CoreMatchers.ismatcher. These assertions were rewritten as straight-forwardassertEqualsassertions.org.junit.jupiter.api.Assertions'assertThrowswas used to assert an expected exception is thrown instead of usingorg.junit.Testwith theexpectedannotation.Mocking
org.powermock.api.mockito.PowerMockitowere deemed to be unnescary, and replaced with straight-forward usages oforg.mockito.Mockito.org.powermock.reflect.Whiteboxwere cleaned, and theorg.mockito.InjectMockswas used instead in order to inject mocks to the object under test.Documentation