[
https://issues.apache.org/jira/browse/SUREFIRE-2039?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17510872#comment-17510872
]
Tibor Digana edited comment on SUREFIRE-2039 at 3/22/22, 9:06 PM:
------------------------------------------------------------------
Hey Sammy, no no, this has nothing to do with you. I had only a problem with
the reproducible project because I have not noticed it exists and the code in
TestSetRunListener. Pls see the pull requests on Github in Surefire, you will
see that there are several PRs which touch the class TestSetRunListener and
StatelessXmlReporter. It really does not make sense to report any pull requests
against these two classes with small workarounds or patches or fixes for three
reasons:
1. they are problematic and they exist for many years
2. we have changes or prerequisite for (3)
3. we have to refactor both classes
but as you may have noticed the implementation relies on ordering of classes
and methods, and the report is finished in one point of time when the test set
has finished and not when the Provider has finished. Now we have problem with
re-run feature, this issue, natural ordering, parallel tests executions.
We have this refactoring in our roadmap and we have to do one thing, rework
these two classes and employ the {{testRunId}} and {{RunMode}} and not to
accept patches because these would definitely overload us. We have to smesh
down the current implementation and then verify your requirements are met by
the new implementation.
So this effort you have made is pushing us but unfortunately it won't help,
even if you want to help us, but the turn is on our side. So nowadays we should
release M5, and then we should rework the implementation and we can do it
together with your ITs in a new PR.
What would definitely help us is to provide us with integration tests which
fail now in master, and you can do it in a PR even if it would have a broken
CI, that's not a problem, and after we have got these classes reimplemented, we
would make an agreement to adopt these ITs and push the entire work to the
master as one complete solution.
Currently accepting the patches into an archaic reporter would be problematic
and cannot be accepted, otherwise we as maintainers would blow up the problem
to bigger and bigger one, and of course this has to be stopped.
was (Author: tibor17):
Hey Sammy, no no, this has nothing to do with you. I had only a problem with
the reproducible project because I have not noticed it exists and the code in
TestSetRunListener. Pls see the pull requests on Github in Surefire, you will
see that there are several PRs which touch the class TestSetRunListener and
StatelessXmlReporter. It really does not make sense to report any pull requests
against these two classes with small workarounds or patches or fixes for three
reasons:
1. they are problematic and they exist for many years
2. we have changes or prerequisite for (3)
3. we have to refactor both classes but but...
but as you have noticed the implementation relies on ordering of classes and
methods, and the report is finished in one point of when test set has finished
and not when the Provider has finished and thus we have problem with re-run
feature, this issue, natural ordering.
We have this in our roadmap and we have to do one thing, rework these two
classes and employ the {{testRunId}} and {{RunMode}} and not to accept patches
because these would kill us definitely. We have to smesh down the current
implementation and then verify your requirements are met by the new
implementation.
So this effort you have made is pushing us but unfortunately it won't help,
even if you want to help us, but the turn is on our side. So nowadays we should
release M5, and then we should rework the implementation and we can do it
together with your ITs in a new PR.
What would definitely help us is to provide us with integration tests which
fail now in master, and you can do it in a PR even if it would have a broken
CI, that's not a problem, and after we have got these classes reimplemented, we
would make an agreement to adopt these ITs and push the entire work to the
master as one complete solution.
Currently accepting the patches into an archaic reporter would be problematic
and cannot be accepted, otherwise we as maintainers would blow up the problem
to bigger and bigger one, and of course this has to be stopped.
> Skipped test classes are getting into the non-skipped test classes reports
> --------------------------------------------------------------------------
>
> Key: SUREFIRE-2039
> URL: https://issues.apache.org/jira/browse/SUREFIRE-2039
> Project: Maven Surefire
> Issue Type: Bug
> Affects Versions: 3.0.0-M5
> Reporter: Semyon Danilov
> Priority: Major
>
> The setup: maven-surefire-plugin 3.0.0-M5 and junit 5.8.2.
> Consider a simple scenario, two test classes, *ATest* and *BTest*, one
> +Disabled+ and one not respectively, each has three test methods. After
> running _mvn surefire:test_ the report will indicate that 4 tests were run
> and one of them was skipped:
> {noformat}
> [INFO] T E S T S
> [INFO] -------------------------------------------------------
> [INFO] Running org.example.test.BTest
> [WARNING] Tests run: 4, Failures: 0, Errors: 0, Skipped: 1, Time elapsed:
> 0.062 s - in org.example.test.BTest
> [INFO]
> [INFO] Results:
> [INFO]
> [WARNING] Tests run: 4, Failures: 0, Errors: 0, Skipped: 1
> {noformat}
> And if we enable *ATest* and disable *BTest*, then the report will only
> mention running three tests, none skipped:
> {noformat}
> [INFO] T E S T S
> [INFO] -------------------------------------------------------
> [INFO] Running org.example.test.ATest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.079
> s - in org.example.test.ATest
> [INFO]
> [INFO] Results:
> [INFO]
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0
> {noformat}
> This behavior is the result of the way how
> *org.apache.maven.plugin.surefire.report.TestSetRunListener* handles skipped
> tests. It adds them to the *detailsForThis* and only prints after the test
> set is finished which happens when a non-disabled test set is finished thus
> adding skipped test results to the statistics of a non-skipped test set. And,
> of course, if the disabled test is ran after the non-disabled test, it won't
> be reported at all.
> The reproducer can be found here:
> https://github.com/SammyVimesFiledIssues/SUREFIRE-2039
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
