Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Migrate Junit 4 to Junit 5: showcase #2728

Closed
blakeli0 opened this issue May 3, 2024 · 0 comments · Fixed by #2757
Closed

Migrate Junit 4 to Junit 5: showcase #2728

blakeli0 opened this issue May 3, 2024 · 0 comments · Fixed by #2757
Assignees
@zhumin8
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.

Comments

@blakeli0
Copy link
Collaborator

blakeli0 commented May 3, 2024

See this doc for important differences and benefits of Junit 5.
See the official doc from Junit5 for migration tips.

In general, for each and every unit test

  • If the only change is the package name, change the import statement. For example, replace org.junit.Assert.assertEquals with org.junit.jupiter.api.Assertions.assertEquals.
  • If the feature is still supported, convert the Junit 4 syntax to Junit 5 syntax. For example, replace @Before with @BeforeEach
  • If the feature is not supported anymore, re-write the unsupported part with the alternative. For example, replace @Test(expected = …​) with assertThrows.

See example PR that adds Junit 5 dependencies to gax-java and migrates one test to Junit5.
@mpeddada1 mpeddada1 added type: cleanup An internal cleanup or hygiene concern. priority: p2 Moderately-important priority. Fix may not be included in next release. labels May 6, 2024
@zhumin8 zhumin8 self-assigned this May 8, 2024
@zhumin8 zhumin8 mentioned this issue May 9, 2024
Merged
lqiu96 added a commit that referenced this issue May 20, 2024
@zhumin8 @burkedavison @lqiu96
chore: Migrate Junit 4 to Junit 5 for showcase (#2757)
5799827 
fixes #2728, attempt to remove Junit 4 support after migration.
Other than POM dependency migrate, changes include:
- package name changes
- Junit 5 syntax upgrades, e.g. `@Before` --> `@BeforeEach`, Replace
assertion methods
- remove public modifier on tests and test classes.
- Refactor JUnit 4 TemporaryFolder `@Rule` in
[ITGdch.java](https://github.com/googleapis/sdk-platform-java/pull/2757/files#diff-6ae7755a0b038e1a2febae2d27e36c762f6751b8c7db577421667069399884b4)
to JUnit 5 `@TempDir`
- Replace `@Test(timeout = 15000L)` in
[ITClientShutdown.java](https://github.com/googleapis/sdk-platform-java/pull/2757/files#diff-70d1df57471178a7a63302f82e4a4855ffbbd642ea67d92d501bd1f7008957ca)
with `@Timeout(15)`
- Update `@RunWith(Parameterized.class)` test in
[ITHttpAnnotation.java](https://github.com/googleapis/sdk-platform-java/pull/2757/files#diff-03d420650ecc9fe78ad4887761043c4fdceaa978f464ce30cfc4ed5f8be9b64d)
to `@ParameterizedTest` with `@MethodSource("data")`

~~Note: #2737 creates a new test class with JUnit4 syntax. Depending on
merging order, I will either update in this pr, or #2737.~~ Updated.


Due to truth library depending on junit 4 ([see
issue](google/truth#333)), junit 4 cannot be
completely removed, or will encounter `java.lang.ClassNotFoundException:
org.junit.runner.notification.RunListener` running tests with maven
surefire. To keep things cleaner, excluding the implicitly junit brought
in from truth and `junit-vintage-engine`. We could also do the reverse,
and make a comment if that's prefered.

---------

Co-authored-by: Burke Davison <[email protected]>
Co-authored-by: Lawrence Qiu <[email protected]>
lqiu96 added a commit that referenced this issue May 22, 2024
@zhumin8 @burkedavison @lqiu96
chore: Migrate Junit 4 to Junit 5 for showcase (#2757)
fd91c65 
fixes #2728, attempt to remove Junit 4 support after migration.
Other than POM dependency migrate, changes include:
- package name changes
- Junit 5 syntax upgrades, e.g. `@Before` --> `@BeforeEach`, Replace
assertion methods
- remove public modifier on tests and test classes.
- Refactor JUnit 4 TemporaryFolder `@Rule` in
[ITGdch.java](https://github.com/googleapis/sdk-platform-java/pull/2757/files#diff-6ae7755a0b038e1a2febae2d27e36c762f6751b8c7db577421667069399884b4)
to JUnit 5 `@TempDir`
- Replace `@Test(timeout = 15000L)` in
[ITClientShutdown.java](https://github.com/googleapis/sdk-platform-java/pull/2757/files#diff-70d1df57471178a7a63302f82e4a4855ffbbd642ea67d92d501bd1f7008957ca)
with `@Timeout(15)`
- Update `@RunWith(Parameterized.class)` test in
[ITHttpAnnotation.java](https://github.com/googleapis/sdk-platform-java/pull/2757/files#diff-03d420650ecc9fe78ad4887761043c4fdceaa978f464ce30cfc4ed5f8be9b64d)
to `@ParameterizedTest` with `@MethodSource("data")`

~~Note: #2737 creates a new test class with JUnit4 syntax. Depending on
merging order, I will either update in this pr, or #2737.~~ Updated.


Due to truth library depending on junit 4 ([see
issue](google/truth#333)), junit 4 cannot be
completely removed, or will encounter `java.lang.ClassNotFoundException:
org.junit.runner.notification.RunListener` running tests with maven
surefire. To keep things cleaner, excluding the implicitly junit brought
in from truth and `junit-vintage-engine`. We could also do the reverse,
and make a comment if that's prefered.

---------

Co-authored-by: Burke Davison <[email protected]>
Co-authored-by: Lawrence Qiu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zhumin8 zhumin8

Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: Migrate Junit 4 to Junit 5 for showcase googleapis/sdk-platform-java
3 participants
@zhumin8 @blakeli0 @mpeddada1