[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-549187334 @Tibor17 @Seijan I'd like to take discussion over `@ParameterizedTest` over to a new PR that specifically deals with the problem: #252 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548949938 Yes it's a fix for this PR. I wouldn't really call this _"new"_ in the sense that it has non-covered code.  With this small change `@ParameterizedTest` support is fixed and there are already IT classes that cover this code. Its functionally identical except it no longer calls `RunListenerAdapter#toClassMethodName` _(Which means the `toClassMethodNameWithoutPlan` changes can be rolled back since they are no longer necessary)_ 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548937525 @Tibor17 Applied the changes using `UniqueId` and verified all tests in `JUnitPlatformProviderTest` pass including [the new one](https://gist.github.com/Seijan/a0c1900e2ba6dcc6642ea81f1ac6f065) created by @Seijan  What title do you want for the 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-544685214 I've seen your post and it didn't change behavior in this case, but I think it would be wise to implement just to be safe. And yes, `[0, 2]` can also work. When adding `@DisplayName` to a class using 0 or 1 both seemed to have the same behavior. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-544641900 This is actually a **one character** fix :man_facepalming: [JUnitPlatformProvider - Line 195](https://github.com/apache/maven-surefire/blob/d0230dcc93e6cd1b8171407237489fdf291cca48/surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java#L195) When writing `buildLauncherDiscoveryRequestForRerunFailures` I called `String[] toClassMethodNameWithoutPlan()` to fetch the method name `source[3]`, but should be using `source[2]` instead. * `source[3]` is the resolved method name _(Which uses `@DisplayName`'s value)_ * `source[2]` is the actual method name. Changing `3` to `2` is all that is necessary. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-543864271 I'll happily work on a fix for this bug as soon as an offending test case is produced :+1: 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-538729946 :+1: 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-538651514 > Since it is unit test try to add one more variant the your test at least. E.g. RerunFailingTestsCount=2 and the second rerun would be successfull. [Done.](https://github.com/apache/maven-surefire/blob/c844a729161a804a6cb88d367100d778b3c0119c/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java#L151) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-538613732 > but we have 527 new lines and this may worse the code coverage 430 of these lines belong to the ITs files. The 97 remaining lines are covered when `rerunFailingTestCount` is > `0`. > Would you please add unit tests in JUnitPlatformProviderTest and cover new lines I've added a unit test to cover this here: [JUnitPlatformProviderTest.java#L151](https://github.com/apache/maven-surefire/blob/dd9286fd7ca2a4425972d9e20fa2010fef9832a0/surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java#L151) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests
for JUnit5
URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537429513
> Did you introduce another IT in the next 6 commits too?
The only non-syntax changes are adding `debugLogging()` and updating JUnit
to `5.5.1`. No additional tests were added.
> You will see it when you use JDK7. Feel free to try.
I'm was on 8 which would explain why I didn't see it.
> We have such conditions in ITs, see `assumeJavaVersion( javaVersion )`.
> Surefire is compiled with Java 7 and JRE is between 7 - 13
So if I add the following to `JUnitPlatformRerunFailingTestsIT`:
```java
@Before
public void initialize()
{
assumeJavaVersion( 1.8 );
}
```
then this should be resolved?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537282390 > If you are done, you can squash it. There's one failing test. I guess you forgot to use JUnit assumption of Java 8. We have such conditions in ITs, see assumeJavaVersion( javaVersion ). Ok I'll squash after the Java 8 failure is resolved. I didn't see it locally and I can't find it on the latest build: https://builds.apache.org/job/maven-box/job/maven-surefire/job/SUREFIRE-1584/2/testReport/ Which ITs test needs `assumeJavaVersion( javaVersion )`? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537098853 See: [this discussion](https://github.com/apache/maven-surefire/pull/245#discussion_r330129552) for the new commit's reasoning. It resolves the ITS failures. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537007114 > Pls check if the difference between those two methods is what I used in the previous comment because I was too fast to compare in detail. Thx I noticed that: ```java source = testPlan.getParent( testIdentifier ) .map( this::toClassMethodName ) ``` should probably be: ```java source = testPlan.getParent( testIdentifier ) .map( i -> toClassMethodName( i, usePlan ) ) ``` I think that should be it for any possible regressions. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-523144813 Ok, all suggestions have been addressed except for the usage of `toClassMethodNameWithoutPlan`, which I responded to above. How would you like this to be addressed? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-523142533 > We of course have to mention JUnit 5 in JavaDoc. You can see the practice in another parameters that we say `Since version 3.0.0-M4 ...`. So `(JUnit 4+ providers and JUnit 5+ providers since 3.0.0-M4)` would be more appropriate then? > Additionally, we have to update the chart `maven-surefire-plugin/src/site/apt/examples/featurematrix.apt.vm` and update the line `re-run count` and type `Y` with a new remark `*3`. Is the remark for clarifying support as of 3.0.0-M4? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Col-E commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-523128544 > Pls open the MOJO classes. You will see `rerunFailingTestsCount`. Extend the JavaDoc and add a new support for JUnit5 with all known limitations if any exist. I found docs for `rerunFailingTestsCount` in `SurefirePlugin.java` and `IntegrationTestMojo.java` where it states: `(JUnit 4+ providers)` Would changing this to `(JUnit 4+ providers & JUnit 5+ providers)` be acceptable? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
