[GitHub] [maven-surefire] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548970053 @Col-E Regarding the change in `String[2]` we talked about before, we have to be careful and run the integration tests for JUnit5. This means `*Platform*IT`. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548968926 @Col-E I think they call it "legacy" in junit5 becuse it's not related to legacy version of JUnit5 but the `description` of methods similar to the JUnit4. See the JavaDoc. But here I also did not see the returned value. It would be first interesting to debug it, put the breakpoint and see the string. Feel free to show ithe string how it looks like before we make a real change. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548942695 @Col-E It is fix for this PR. It is new code. It was not before, right? So we have to report issue and ake PR the same as we did it before. And then the next issue would go with ParmeterizedTest only and not rerun saying that the Parameterize test should have different name for every combination of parameters. If we use `getLegacyReportingName()` instead of pure method name in the String[2] then integration test will pass. I mean this test from @Seijan was not IT. It was unit test but we need to have real IT and we have to check the XML report as well as the number of calls of the same method and we do not have to employ the rerun because it was fixed previously by you. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548821652 @Col-E From @marcphilipp answer https://github.com/apache/maven-surefire/pull/245#issuecomment-548818775 it means that `getLegacyReportingName()` should be a real fix for the rerun on `@ParameterizedTest`. Maybe we should open a new ticket and @Seijan can provide us with the ralistic integration test. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548784706 @Col-E Can you pls rework https://github.com/apache/maven-surefire/pull/249/commits/4f16fcde178c95d170e3d883bbf6fc0ab28998fb#diff-b3a2904e92be87f11f2622fbfd122e31R198 and use `UniqueId` as @marcphilipp described in https://github.com/apache/maven-surefire/pull/245#issuecomment-548777170 ? It will be generic fix for all types of tests including the re-run. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548782829 @Col-E @Seijan @marcphilipp What would be the best text representation of each run of param.test? Although the code has only one Java method, we should represent each combination of parameter as if it was a separate method. So we know that the class name is `pkg.MyTest`, method name is called `test` and the parameter is `Hello [1]`. And my question is what is the best `description` of the method in the report. What a string 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548556737 @Seijan If I use two parameters, I always have the same description on the method. So we cannot recognize which run it is and the logic guessed that it was already the rerun. That's the problem. Instead using `[engine:junit-jupiter]/[class:pkg.ParamTest]/[test-template:test(java.lang.String)]/[test-template-invocation:#1]` for the method name solves the unique identity of the runs but it does not look so well in the report. And the next issue I found with `@ParameterizedTest` is this the method `getDisplayName()` which returns `Hello [1]` but it is the method parameter and no display name. I do not know why it does not return `null` if I have no annotation `@DisplayName`. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-548552528 @Seijan @marcphilipp I have a trivial test and I have several observations with ParameterizedTest. ``` @ParameterizedTest @ValueSource(strings = { "Hello" }) void test(String world) { if (i++ == 0) { throw new RuntimeException(); } } ``` This `discoveryRequest` does not find our test to run which is this line in our code: `launcher.execute( discoveryRequest, adapter )` It looks like this in debugger. Does it have good values? ![](https://gist.github.com/Tibor17/7e5115d4c7ce4b0c1d5bb762ce469e3e) The ID of the method is this: `[engine:junit-jupiter]/[class:pkg.ParamTest]/[test-template:test(java.lang.String)]/[test-template-invocation:#1]` Should we filter the method in the request like we do it in JUnit4 and use filter the test method `test[1]` instead of `test`? 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-547965436 @Col-E @Seijan IMO the problem would be in the filter. Maybe the name of test method is `myTest[0]` instead of `myTest`. My guess. Having the IT would show us the root cause in debugger. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-547962196 @Seijan I agree with you that parameterized tests are very important. Would you write an integration test in our project and show us how this feature works together with the parameterized tests? If we see the test result, we can better understand the problem and make a fix. Maybe worth to start with parameterized test first without this feature and then continue with second IT having this feature. And we can see the difference. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-544674581 @Col-E you want to change 3 to 2? Should not it be either: [0, 2] or [1, 3] ? 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-544671501 @Col-E How you see my suspicion written in https://github.com/apache/maven-surefire/pull/245#issuecomment-543712421 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-544501410 @quiram [this line means that every test should fail|https://github.com/quiram/surefire-retry-failed-tests-issue/blob/master/src/test/java/com/github/quiram/SurefireIssueNoRetryTest.java#L13] 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-544499915 @quiram thx a lot. pls create a new issue on Jira. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-543901329 @quiram If I understood the bug right in our code, it should be possible to reproduce it even without Spring Boot. You are trying to use the original commercial project and then you are reducing it somehow? Take your time, no worries, I will wait because this issue is important for reliability of this feature. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-543869144 @Col-E ok, :-) thx, but do it in a new PR and new branch. Delete the old branch because I modified yours (added tests and `restart()`) and now it is in master. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-543774689 > > > > @quiram > > An exception or assertion failure does not crash the code in Surefire and JUnit. These exceptions are properly handled by the JUnit. > > Sorry, did you mean exception or extension? I was talking about JUnit Jupiter Extensions, not exceptions. I thought maybe the extension interacts with the test lifecycle in a way that clashes with surefire. However, it seems you have found the likely root cause, so I guess the extension is not a problem. I understood `exception`, sorry. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-543745287 @quiram I will wait for @Col-E because i have investigated an issue in the code, see https://github.com/apache/maven-surefire/pull/245#issuecomment-543712421. After @Col-E would confirm the bug, we would fix it and prove it with a test from @quiram . Then we would be able to cute a new release version. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-543742706 @quiram An exception or assertion failure does not crash the code in Surefire and JUnit. These exceptions are properly handled by the JUnit. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-543712421 @Col-E I think I nderstand the problem. We consider the `adapter` been `stateless` in every loop. In reality we modify the collection of failures from the first set of failure and the adapter is `stateful`. https://github.com/apache/maven-surefire/pull/245/commits/fe17751602f597d7b694ec8a65fc389aee0bac25#diff-b3a2904e92be87f11f2622fbfd122e31R152 So my proposal is to grap the collection of failures and make a copy, and then iterate over the elements. I hope I got it right. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-543702810 Hi @Col-E Matt, How are you. We have some issue with our feature. Can you pls talk with one of our user, for more information see his comment https://issues.apache.org/jira/browse/SUREFIRE-1584?focusedCommentId=16954490=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16954490 Perhaps you already know what's going on. Thx 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-538689637 closing as merged https://gitbox.apache.org/repos/asf?p=maven-surefire.git;a=commit;h=d0230dcc93e6cd1b8171407237489fdf291cca48 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-538675085 @Col-E I have extended the tests a bit and added `reset()` in the adapter. See the branch https://github.com/apache/maven-surefire/tree/SUREFIRE-1584 The Ci is running https://builds.apache.org/job/maven-box/job/maven-surefire/job/SUREFIRE-1584/ 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-538645839 @Col-E 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. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537677972 @Col-E sorry that I have so many requirements but we have 527 new lines and this may worse the code coverage. Would you please add unit tests in `JUnitPlatformProviderTest` and cover new lines in `RunListenerAdapter` and `JUnitPlatformProvider`? Thx 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537552191 The Jenkins build is currently running https://builds.apache.org/job/maven-box/job/maven-surefire/job/SUREFIRE-1584/ 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537478286 @Col-E > then this should be resolved? yes, this should the trick. Thx 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537426830 @Col-E > I didn't see it locally You will see it when you use JDK7. Feel free to try. Thx 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537425160 In the logs I saw this test project `junit-platform-rerun-failing-tests` but you have to use the assumption in every IT you added in this PR due to Surefire is at Java 7 and JRE is between 7 - 13. This assumption skips the execution for JRE 7. I saw the ITs in the commit https://github.com/apache/maven-surefire/pull/245/commits/2558e288edfa92171f9ad44645b10c9a0b071a5f Did you introduce another IT in the next 6 commits too? Thx 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537231848 @Col-E 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 )`. ``` 19:21:48 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:testCompile (default-testCompile) on project junit-platform-rerun-failing-tests: Fatal error compiling: invalid target release: 1.8 ``` 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537041815 @Col-E These are our new test? https://builds.apache.org/job/maven-box/job/maven-surefire/job/SUREFIRE-1584/1/testReport/ There are 4 failed 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [maven-surefire] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-537003525 @Col-E thx, the build is running https://builds.apache.org/job/maven-box/job/maven-surefire/job/SUREFIRE-1584/ 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-536994438 One can see that the method must alter the behavior . In practice we do it easily the way that we have algorithm in one method because of 90% reusability and we introduce a private method `toClassMethodName(TestIdentifier, boolean)` which is called in the accessor methods: ``` private toClassMethodName(TestIdentifier ) { return toClassMethodName( testIdentifier, false ); }``` and another public method: ``` toClassMethodNameWithoutPlan( testIdentifier ) { return toClassMethodName( testIdentifier, true ); } ``` true: real class/method names without plan false: with display names if possible 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-536989553 @jon-bell @Col-E Sorry for the delay. Let's continue, guys! So I have compare these two methods, namely: `private String[] toClassMethodName( TestIdentifier testIdentifier )` `public String[] toClassMethodNameWithoutPlan( TestIdentifier testIdentifier )` (btw, hide the information/methoid as much as possible, thus use `private` at first). The solution is very simple: `add a new boolean parameter in the old method "toClassMethodName"`. The above methods are cca 90% similar and that breaks the reusability. Our problem is only this diff: ``` String[] source = testPlan.getParent( testIdentifier ) .map( this::toClassMethodName ) .map( s -> new String[] { s[0], s[1] } ) .orElse( new String[] { realClassName, realClassName } ); ``` and your change: ``` String[] source = new String[] {realClassName, realClassName}; ``` 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-530054837 @jon-bell @Col-E Thx for reply. I am busy this week and my colleagues have been busy too. I will try to have a look on the method perhaps today late evening. This issue is assigned to the release. Don't worry we will not loose it. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-523145553 The diff in Github is not enough for me, so I have to see the entire class for `toClassMethodNameWithoutPlan` and think about it. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-523143928 > (JUnit 4+ providers and JUnit 5+ providers since 3.0.0-M4) > Is the remark for clarifying support as of 3.0.0-M4? yes, for both, that's ok and enough as you have mentioned. The reason is that the users sometimes are not aware that it is not the version they use in the company but the latest with a praticular feature. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-523138627 @jon-bell 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`. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-523136911 > > > > 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? 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 ...`. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-522944865 @jon-bell Generally, it looks good. I have left only few comments. Pls open the MOJO classes. You will see `rerunFailingTestsCount`. Extend the JavaDoc and add a new support for JUnit5 will all limitations if any exist. 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] Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5
Tibor17 commented on issue #245: Surefire-1584: Add option to rerun failing tests for JUnit5 URL: https://github.com/apache/maven-surefire/pull/245#issuecomment-522658458 @jon-bell I don't this huge number of commits and merge commits. Can you make only one commit on the top of master HEAD? There are two authors. Did you pull some commits also from other pull requests? Important: pls run the build `mvn install -P run-its` and setup your IDEA or Eclipse to the code style (there are config files for Ides) https://maven.apache.org/developers/conventions/code.html Do not change white spaces and code style against origin/master. Only add/remove/modify code. 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