Skip to content

[JUnit 5] Migrate tests which can be easily converted #2878

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

Merged
merged 3 commits into from
Dec 2, 2021

Conversation

Bennett-Lynch
Copy link
Contributor

@Bennett-Lynch Bennett-Lynch commented Dec 1, 2021

JUnit 4 -> 5 migrations are often done in a 3-step process:

  1. Update dependencies to JUnit 5, including the vintage module, causing all tests to leverage the new JUnit 5 Platform (completed in [JUnit 5] Migrate to JUnit 5 Platform (with existing JUnit 4 test cases) #2850)
  2. Incrementally migrate JUnit 4 tests to JUnit 5 tests
  3. Once no JUnit 4 usage remains, remove the dependency on the vintage module

This PR begins step 2 by migrating tests which can be easily converted.

IntelliJ provides a convenient way to migrate tests:

image

But unfortunately there is no support for automatically migrating JUnit 4 tests which leverage any of the following:

  1. The expected parameter in test annotations
  2. The timeout parameter in test annotations
  3. The class-level @RunWith annotation (e.g., @RunWith(Parameterized.class) and @RunWith(MockitoJUnitRunner.class))

Therefore, this PR leverages the migrate functionality but defers the above scenarios as follows:

grep -r -e "@Test(" -e "@RunWith(" . | cut -d ':' -f1 | xargs -I {} git restore {}

All changes in this PR are fully automated with the migrate functionality, except for the changes to the top-level pom.xml to add org.junit.* to both ignoredUsedUndeclaredDependency and ignoredUnusedDeclaredDependency.

Additionally, there were a few cases of the migrate functionality migrating a superclass but not being able to migrate a subclass (due to the above limitations). The superclass was restored in this case.

License

  • I confirm that this pull request can be released under the Apache 2 license

JUnit 4 -> 5 migrations are often done in a 3-step process:

1. Update dependencies to JUnit 5, including the vintage module,
causing all tests to leverage the new JUnit 5 Platform
2. Incrementally migrate JUnit 4 tests to JUnit 5 tests
3. Once no JUnit 4 usage remains, remove the dependency on the
vintage module

This PR begins step 2 by migrating tests which can be easily converted.

IntelliJ provides a convenient way to migrate tests, but unfortunately
there is no support for automatically migrating JUnit 4 tests which
leverage any of the following:

1. The `expected` parameter in test annotations
2. The `timeout` parameter in test annotations
3. The class-level ``@RunWith` annotation (e.g.,
`@RunWith(Parameterized.class)` and
`@RunWith(MockitoJUnitRunner.class)`)

Therefore, this PR leverages the migrate functionality but defers the
above scenarios as follows:

```
grep -r -e "@test(" -e "@RunWith(" . | cut -d ':' -f1 | xargs -I {} git restore {}
```
@Bennett-Lynch Bennett-Lynch requested a review from a team as a code owner December 1, 2021 19:04
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 2, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2995 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Bennett-Lynch Bennett-Lynch merged commit 4289e9a into aws:master Dec 2, 2021
aws-sdk-java-automation pushed a commit that referenced this pull request Dec 2, 2021
…" because it broke integration tests.

This reverts commit 4289e9a98c8db6b25643c40c9e77fdae6af1d5td5.
aws-sdk-java-automation pushed a commit that referenced this pull request Dec 2, 2021
Revert "[JUnit 5] Migrate tests which can be easily converted (#2878)…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants