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.

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

test: Migrate gax unit tests to Junit 5 #2724

Merged
merged 14 commits into from
May 17, 2024
Merged

test: Migrate gax unit tests to Junit 5 #2724

merged 14 commits into from
May 17, 2024

Conversation

blakeli0
Copy link
Collaborator

@blakeli0 blakeli0 commented May 3, 2024

This PR migrate all unit tests in Gax to Junit 5.

Other than standard direct replacements, some tests need to be rewritten due to the following scenarios:

  • @Rule does not exist anymore and there is no direct replacement. We mostly use it to initialize a Mockito stub, replaced with @ExtendWith(MockitoExtension.class).
  • Some tests were relying on try-catch to assert exceptions, replaced with assertThrows. e.g. OperationsClientTest
  • Parameterized tests in AbstractRetryingExecutorTest

There are a few environment variable tests can be re-written with Junit 5, so we don't need to configure a profile for it anymore, but they are not in the scope of this PR.

fixes: #1611.

@product-auto-label product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels May 17, 2024
@blakeli0 blakeli0 changed the title test: Add Junit5 support to Gax. test: Migrate gax unit tests to Junit 5 May 17, 2024
import org.threeten.bp.Duration;

@RunWith(JUnit4.class)
public class TimeoutTest {
@ExtendWith(MockitoExtension.class)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because there is a @Rule for Mockito being removed below.

gax-java/pom.xml Outdated
<version>4.13.2</version>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>4.11.0</version>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using mockito bom?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not aware of a mockito bom, sounds like a good idea. Also, I think adding the bom to this pom should fine, because gax-parent is not included in the gapic-generator-java-bom, only gax-bom is. Is that correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so.

You can verify this by running mvn -pl gapic-generator-java-bom help:effective-pom at repo root.

Copy link

sonarcloud bot commented May 17, 2024

Quality Gate Passed Quality Gate passed for 'gapic-generator-java-root'

Issues
63 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

sonarcloud bot commented May 17, 2024

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Issues
63 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@blakeli0 blakeli0 merged commit 38ebcc3 into main May 17, 2024
46 of 47 checks passed
@blakeli0 blakeli0 deleted the add-junit5 branch May 17, 2024 21:21
lqiu96 pushed a commit that referenced this pull request May 22, 2024
This PR migrate all unit tests in Gax to Junit 5.

Other than standard direct replacements, some tests need to be rewritten
due to the following scenarios:
- `@Rule` does not exist anymore and there is no direct replacement. We
mostly use it to initialize a Mockito stub, replaced with
`@ExtendWith(MockitoExtension.class)`.
- Some tests were relying on try-catch to assert exceptions, replaced
with `assertThrows`. e.g.
[OperationsClientTest](https://github.com/googleapis/sdk-platform-java/pull/2724/files#diff-4530df761eff0854357165d951e1667d3810a5448ec2aa4b853a6331516cbde0)
- Parameterized tests in
[AbstractRetryingExecutorTest](https://github.com/googleapis/sdk-platform-java/pull/2724/files#diff-9c5f5c1d2fcef6c4164fc0171d01e7020aa7ebb7aa49615cf3743dc89c9b3d1d)

There are a few environment variable tests can be re-written with Junit
5, so we don't need to configure a
[profile](https://github.com/googleapis/sdk-platform-java/blob/main/gax-java/gax/pom.xml#L115-L128)
for it anymore, but they are not in the scope of this PR.

fixes: #1611.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate Junit 4 to Junit 5: gax-java
3 participants