Tibor17 commented on a change in pull request #469:
URL: https://github.com/apache/maven-surefire/pull/469#discussion_r805221122
##########
File path:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
##########
@@ -51,13 +50,13 @@
* @author Kristian Rosenvold
*/
public class TestSetRunListener
- implements RunListener, ConsoleOutputReceiver, ConsoleLogger
+ implements TestRunListener, ConsoleLogger
Review comment:
Yes of course becasue the console logger is used to report errors from
JVM back to plugin JVM in cases when internal code throws unexpected exception
(not the tests).
##########
File path:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
##########
@@ -51,13 +50,13 @@
* @author Kristian Rosenvold
*/
public class TestSetRunListener
- implements RunListener, ConsoleOutputReceiver, ConsoleLogger
+ implements TestRunListener, ConsoleLogger
Review comment:
Yes of course becasue the console logger is used to report errors from a
forked surefire JVM back to plugin JVM in cases when internal code throws
unexpected exception (not the tests).
##########
File path:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
##########
@@ -51,13 +50,13 @@
* @author Kristian Rosenvold
*/
public class TestSetRunListener
- implements RunListener, ConsoleOutputReceiver, ConsoleLogger
+ implements TestRunListener, ConsoleLogger
Review comment:
Yes of course becasue the console logger is used to report errors from a
forked surefire JVM back to the plugin JVM in cases when internal code throws
unexpected exception (not the tests).
##########
File path:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/report/TestSetRunListener.java
##########
@@ -51,13 +50,13 @@
* @author Kristian Rosenvold
*/
public class TestSetRunListener
- implements RunListener, ConsoleOutputReceiver, ConsoleLogger
+ implements TestRunListener, ConsoleLogger
Review comment:
Yes of course becasue the console logger is used to report internal
errors from a forked surefire JVM back to the plugin JVM in cases when internal
code throws unexpected exception (not the tests).
##########
File path:
surefire-api/src/main/java/org/apache/maven/surefire/api/report/ReporterFactory.java
##########
@@ -33,7 +33,7 @@
*
* @return A reporter instance
*/
- RunListener createReporter();
+ TestRunListener createReporter();
Review comment:
In this architecture we have two sides (forked JVM, plugin JVM) having
implemented the same interface RunListener:
1. side is ForkingRunListener in the fork JVM
2. TestSetRunListener in the plugin JVM
The serialization between these two JVM ensures that the ReportEntries are
transfered from the fork to the plugin.
This is the whole idea behind them.
We can see these renaming issues from the another angle, and so that there
is a reporter. Listener means that our surefire `ForkingRunListener` is a
delegate within JUnit's RunListeners in the fork. On the other side, the
TestSetRunListener is a listener registered in the `ForkClient`.
We can think about it a bit deeper and we will find the best names for these
interfaces and methods.
Anyway, we will remove deprecated code, we will create Builders for
constructors with too many parameters, so we will have a chance to refactor a
lot.
##########
File path:
surefire-api/src/main/java/org/apache/maven/surefire/api/report/ReporterFactory.java
##########
@@ -33,7 +33,7 @@
*
* @return A reporter instance
*/
- RunListener createReporter();
+ TestRunListener createReporter();
Review comment:
In this architecture we have two sides (forked JVM, plugin JVM) having
implemented the same interface RunListener:
1. side is ForkingRunListener in the fork JVM
2. TestSetRunListener in the plugin JVM
The serialization between these two JVM ensures that the ReportEntries are
transfered from the fork to the plugin.
This is the whole idea behind them.
We can see these renaming issues from the another angle, and so that there
is a reporter. Listener means that our surefire `ForkingRunListener` is a
delegate within JUnit's RunListeners in the fork. On the other side, the
`TestSetRunListener` is a listener registered in the `ForkClient` which is part
of plugin JVM.
We can think about it a bit deeper and we will find the best names for these
interfaces and methods.
Anyway, we will remove deprecated code, we will create Builders for
constructors with too many parameters, so we will have a chance to refactor a
lot.
--
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-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
