Migrate servicetalk-client-api tests from jUnit4 to jUnit5
#1612
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 tasks
idelpivnitskiy
changed the title
servicetalk-client-api tests from jUnit4 to jUnit5
tkountis
reviewed
Jun 14, 2021
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright © 2018, 2020 Apple Inc. and the ServiceTalk project authors | |||
| * Copyright © 2018, 2021 Apple Inc. and the ServiceTalk project authors | |||
There was a problem hiding this comment.
please change to 2018, 2020-2021
tkountis
reviewed
Jun 14, 2021
servicetalk-client-api/build.gradle
Outdated
| testImplementation "junit:junit:$junitVersion" | ||
| testImplementation "org.junit.jupiter:junit-jupiter-api:$junit5Version" | ||
| testImplementation "org.junit.jupiter:junit-jupiter-params:$junit5Version" | ||
| testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:$junit5Version" |
There was a problem hiding this comment.
mind moving this to the end of the file?
tkountis
reviewed
Jun 14, 2021
|
@tkountis Thanks for the review. I have made the changes. |
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright © 2019 Apple Inc. and the ServiceTalk project authors | |||
| * Copyright © 2021 Apple Inc. and the ServiceTalk project authors | |||
There was a problem hiding this comment.
Suggested change
| * Copyright © 2021 Apple Inc. and the ServiceTalk project authors | |
| * Copyright © 2019, 2021 Apple Inc. and the ServiceTalk project authors |
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright © 2018 Apple Inc. and the ServiceTalk project authors | |||
| * Copyright © 2021 Apple Inc. and the ServiceTalk project authors | |||
There was a problem hiding this comment.
Suggested change
| * Copyright © 2021 Apple Inc. and the ServiceTalk project authors | |
| * Copyright © 2018, 2021 Apple Inc. and the ServiceTalk project authors |
There was a problem hiding this comment.
Btw, you can skip updating copyright headers (we planning to automate this task) or use ./scripts/update-copyright.sh main and that script will update everything correctly.
Comment on lines
76
to
79
| Exception e = assertThrows(ExecutionException.class, () -> { | ||
| cf.newConnection("c1", null).toFuture().get(); | ||
| cf.newConnection("c2", null).toFuture().get(); | ||
| }); |
There was a problem hiding this comment.
The first newConnection invocation is not expected to throw:
Suggested change
| Exception e = assertThrows(ExecutionException.class, () -> { | |
| cf.newConnection("c1", null).toFuture().get(); | |
| cf.newConnection("c2", null).toFuture().get(); | |
| }); | |
| cf.newConnection("c1", null).toFuture().get(); | |
| Exception e = assertThrows(ExecutionException.class, () -> cf.newConnection("c2", null).toFuture().get()); |
servicetalk-client-api/build.gradle
Outdated
Comment on lines
35
to
37
| testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:$junit5Version" | ||
| testImplementation "org.mockito:mockito-core:$mockitoCoreVersion" | ||
| testImplementation "org.hamcrest:hamcrest-library:$hamcrestVersion" |
There was a problem hiding this comment.
For future PRs, please place testRuntimeOnly dependency after all testImplementation dependencies. We try to sort by the scope first:
Suggested change
| testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:$junit5Version" | |
| testImplementation "org.mockito:mockito-core:$mockitoCoreVersion" | |
| testImplementation "org.hamcrest:hamcrest-library:$hamcrestVersion" | |
| testImplementation "org.mockito:mockito-core:$mockitoCoreVersion" | |
| testImplementation "org.hamcrest:hamcrest-library:$hamcrestVersion" | |
| testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:$junit5Version" |
|
@amitvc I will apply my suggestions. Please take a look at them and keep in mind for future PRs ❤️ |
…/DefaultAutoRetryStrategyProviderTest.java
…/DefaultClientGroupTest.java
…/LimitingConnectionFactoryFilterTest.java
|
@idelpivnitskiy Thank you very much. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
JUnit 5 leverages features from Java 8 or later, such as lambda functions, making tests more powerful and easier to maintain.
JUnit 5 has added some very useful new features for describing, organizing, and executing tests. For instance, tests get better display names and can be organized hierarchically.
JUnit 5 is organized into multiple libraries, so only the features you need are imported into your project. With build systems such as Maven and Gradle, including the right libraries is easy.
JUnit 5 can use more than one extension at a time, which JUnit 4 could not (only one runner could be used at a time). This means you can easily combine the Spring extension with other extensions (such as your own custom extension).
Modifications:
Unit tests have been migrated from JUnit 4 to JUnit 5
Result:
successfully ran ./gradlew :servicetalk-client-api:build
Module servicetalk-client-api now runs tests using JUnit 5