You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/07/19 11:09:23 UTC
[GitHub] [commons-beanutils] SethFalco opened a new pull request #93: test: update to junit5
SethFalco opened a new pull request #93:
URL: https://github.com/apache/commons-beanutils/pull/93
Just wanted to clean up some tests, but PR is also a proposal and demonstration before I do the rest of the repository.
There are 2 changes I'd like to make:
* Upgrade to JUnit 5
* Reduce code setup, for example in `MethodUtilsTestCase.java`, we can just use `String` instead of creating a custom `A` class just for the test.
Upgrading to JUnit 5 has a few notable benefits:
* We can use `assertAll` when there are multiple assertions. (So we do all assertions, currently if one fails then the rest can't run until `mvn test` is run again.)
* Significantly less code surface, just doing 2 files has already removed 337 lines.
* Significantly less boilerplate for test classes.
* JUnit 4+ offers great failure messages and exception handling by default, so there's no need to handle that.
* Structure will be more familiar to developers that started in recent years. (including myself)
---
This could be done in 2 ways:
* Over time in multiple PRs, so each change is smaller. (This will require having 3 dependencies for testing until it's finished.)
* All at once in one PR. (We can replace JUnit 4 with JUnit 5, but it'll be one hell of a large PR.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-beanutils] SethFalco commented on a change in pull request #93: test: update to junit5
Posted by GitBox <gi...@apache.org>.
SethFalco commented on a change in pull request #93:
URL: https://github.com/apache/commons-beanutils/pull/93#discussion_r675191160
##########
File path: pom.xml
##########
@@ -344,12 +344,24 @@
<version>3.2.1</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter</artifactId>
+ <version>5.7.2</version>
+ <scope>test</scope>
+ </dependency>
<dependency>
Review comment:
Oh, I thought we'd have to keep the JUnit 4 dependency until all tests were migrated to JUnit 5.
I guess `junit-vintage-engine` doesn't require that.
At least that's the impression the docs gave me.
> The JUnit Platform can run JUnit 4 based tests as long as you configure a testImplementation dependency on JUnit 4 and a testRuntimeOnly dependency on the JUnit Vintage TestEngine implementation similar to the following.
>
> ```gradle
> dependencies {
> testImplementation("junit:junit:4.13")
> testRuntimeOnly("org.junit.vintage:junit-vintage-engine:5.7.2")
> }
> ```
> \- https://junit.org/junit5/docs/current/user-guide/#running-tests-build-gradle-engines-configure
Just tried now and it worked perfectly fine without the JUnit 4 dependency, though. 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-beanutils] garydgregory merged pull request #93: Start migration to JUnit 5
Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #93:
URL: https://github.com/apache/commons-beanutils/pull/93
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-beanutils] garydgregory commented on pull request #93: test: update to junit5
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #93:
URL: https://github.com/apache/commons-beanutils/pull/93#issuecomment-885259352
Agreed that we should drop JUnit 4 in favor of 5.
Note that Commons VFS is the component that needs this update the most, as it is both released regularly and still has some JUnit 3 style tests! ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-beanutils] garydgregory commented on a change in pull request #93: test: update to junit5
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #93:
URL: https://github.com/apache/commons-beanutils/pull/93#discussion_r675186125
##########
File path: pom.xml
##########
@@ -344,12 +344,24 @@
<version>3.2.1</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter</artifactId>
+ <version>5.7.2</version>
+ <scope>test</scope>
+ </dependency>
<dependency>
Review comment:
JUnit 4 should be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org