[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-25 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814704624



##
File path: 
surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java
##
@@ -128,7 +128,7 @@ public void onJvmExit()
 }
 
 @Override
-public void systemProperties( Map sysProps )
+public void systemProperties( Map sysProps, RunMode rm, 
Long testRunId )

Review comment:
   exactly




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-25 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814702942



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java
##
@@ -68,7 +68,7 @@ public void testSetStarting( TestSetReportEntry report )
 @Override
 public void testSetCompleted( TestSetReportEntry report )
 {
-target.systemProperties( report.getSystemProperties() );
+target.systemProperties( report.getSystemProperties(), null, null );

Review comment:
   `runMode` field will be removed in the next PR.
   This PR only changed some method signatures in the same manner with the 
previous one.
   After this the implementation would come in 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-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-25 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814702942



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java
##
@@ -68,7 +68,7 @@ public void testSetStarting( TestSetReportEntry report )
 @Override
 public void testSetCompleted( TestSetReportEntry report )
 {
-target.systemProperties( report.getSystemProperties() );
+target.systemProperties( report.getSystemProperties(), null, null );

Review comment:
   `runMode` field will be removed in the next PR.
   This PR only changed some method signatures the same as the previous one.
   After this the implementation would come in 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-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814332010



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/MasterProcessChannelEncoder.java
##
@@ -37,7 +38,7 @@
 
 void onJvmExit();
 
-void systemProperties( Map sysProps );
+void systemProperties( Map sysProps, RunMode runMode, Long 
testRunId );

Review comment:
   The testRunId says - this is the test set.
   The runMode says - normal mode, or 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.

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814330891



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/MasterProcessChannelEncoder.java
##
@@ -37,7 +38,7 @@
 
 void onJvmExit();
 
-void systemProperties( Map sysProps );
+void systemProperties( Map sysProps, RunMode runMode, Long 
testRunId );

Review comment:
   I agree and so I have added the Javadoc, pls see the commit 
[2840bfa](https://github.com/apache/maven-surefire/pull/475/commits/2840bfaf14abd4574620ce6825047230636d928c).




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814101709



##
File path: 
surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java
##
@@ -128,7 +128,7 @@ public void onJvmExit()
 }
 
 @Override
-public void systemProperties( Map sysProps )
+public void systemProperties( Map sysProps, RunMode rm, 
Long testRunId )

Review comment:
   You'r right, the `runMode` as a test status should not be in 3 classes. 
It should be only in one, e.g. in the:
   `JUnit4RunListener` which implements framework's listener 
`org.junit.runner.notification.RunListener`.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814101709



##
File path: 
surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java
##
@@ -128,7 +128,7 @@ public void onJvmExit()
 }
 
 @Override
-public void systemProperties( Map sysProps )
+public void systemProperties( Map sysProps, RunMode rm, 
Long testRunId )

Review comment:
   Your right, the `runMode` as a test status should not be in 3 classes. 
It should be only in one, e.g. in the:
   `JUnit4RunListener` which implements framework's listener 
`org.junit.runner.notification.RunListener`.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814101709



##
File path: 
surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java
##
@@ -128,7 +128,7 @@ public void onJvmExit()
 }
 
 @Override
-public void systemProperties( Map sysProps )
+public void systemProperties( Map sysProps, RunMode rm, 
Long testRunId )

Review comment:
   Your right, the `runMode` as a status should not be in 3 classes. It 
should be only in one, e.g. in the:
   `JUnit4RunListener` which implements framework's listener 
`org.junit.runner.notification.RunListener`.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814101709



##
File path: 
surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java
##
@@ -128,7 +128,7 @@ public void onJvmExit()
 }
 
 @Override
-public void systemProperties( Map sysProps )
+public void systemProperties( Map sysProps, RunMode rm, 
Long testRunId )

Review comment:
   Your right, the `runMode` as a status should not be in 3 classes. It 
should be only in one, e.g. in the:
   `JUnit4RunListener` which implements framework's listene 
`org.junit.runner.notification.RunListener`.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814097522



##
File path: 
surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java
##
@@ -128,7 +128,7 @@ public void onJvmExit()
 }
 
 @Override
-public void systemProperties( Map sysProps )
+public void systemProperties( Map sysProps, RunMode rm, 
Long testRunId )

Review comment:
   The fields `runMode` will be deleted in `ForkinRunListener` and 
`EventChannelEncoder`.
   This PR has only prepared changes in one method signature. The 
implementation of this abstract method comes right after. This PR is only an 
intermediate step.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814097522



##
File path: 
surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java
##
@@ -128,7 +128,7 @@ public void onJvmExit()
 }
 
 @Override
-public void systemProperties( Map sysProps )
+public void systemProperties( Map sysProps, RunMode rm, 
Long testRunId )

Review comment:
   The fields `runMode` will be deleted in `ForkinRunListener` and 
`EventChannelEncoder`.
   This PR has only prepared changes in one method signature. The 
implementation of this abstract method comes right after.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814074238



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java
##
@@ -68,7 +68,7 @@ public void testSetStarting( TestSetReportEntry report )
 @Override
 public void testSetCompleted( TestSetReportEntry report )
 {
-target.systemProperties( report.getSystemProperties() );
+target.systemProperties( report.getSystemProperties(), null, null );

Review comment:
   The `target` is not JUnit's RunListener implementation.
   Only the framework's listener may have this data `runMode` and be "stateful".
   The `target` is a pure encoder, and so it should not have any notion about 
stateful data regarding test status. It should only receive data, encode it, 
and send it to a channel.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814074238



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java
##
@@ -68,7 +68,7 @@ public void testSetStarting( TestSetReportEntry report )
 @Override
 public void testSetCompleted( TestSetReportEntry report )
 {
-target.systemProperties( report.getSystemProperties() );
+target.systemProperties( report.getSystemProperties(), null, null );

Review comment:
   The `target` is not JUnit's RunListener implementation.
   Only the framework's listener may have this data `runMode` and be "stateful".
   The `target` is a pure encoder, and so it should not have any notion about 
stateful data regarding test status. It should only receive data, encode it, 
and send to a channel.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814074238



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java
##
@@ -68,7 +68,7 @@ public void testSetStarting( TestSetReportEntry report )
 @Override
 public void testSetCompleted( TestSetReportEntry report )
 {
-target.systemProperties( report.getSystemProperties() );
+target.systemProperties( report.getSystemProperties(), null, null );

Review comment:
   The `target` is not JUnit's RunListener implementation.
   Only the framework's listener may have this data and be "stateful".
   The `target` is a pure encoder, and so it should not have any notion about 
stateful data regarding test status. It should only receive data, encode it, 
and send to a channel.




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814068564



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java
##
@@ -68,7 +68,7 @@ public void testSetStarting( TestSetReportEntry report )
 @Override
 public void testSetCompleted( TestSetReportEntry report )
 {
-target.systemProperties( report.getSystemProperties() );
+target.systemProperties( report.getSystemProperties(), null, null );

Review comment:
   I did split the entire work of 
[SUREFIRE-1860](https://issues.apache.org/jira/browse/SUREFIRE-1860) into 5 
pieces.
   The line you are asking for is in my IDEA:
   `target.systemProperties( report.getSystemProperties(), report.getRunMode(), 
report.getTestRunId() );`
   Both getters will come in the next PR right after, see 
[SUREFIRE-2015](https://issues.apache.org/jira/browse/SUREFIRE-2015)
   
   




-- 
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




[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #475: [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test

2022-02-24 Thread GitBox


Tibor17 commented on a change in pull request #475:
URL: https://github.com/apache/maven-surefire/pull/475#discussion_r814068564



##
File path: 
surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java
##
@@ -68,7 +68,7 @@ public void testSetStarting( TestSetReportEntry report )
 @Override
 public void testSetCompleted( TestSetReportEntry report )
 {
-target.systemProperties( report.getSystemProperties() );
+target.systemProperties( report.getSystemProperties(), null, null );

Review comment:
   I did split the entire work of 
[SUREFIRE-1860](https://issues.apache.org/jira/browse/SUREFIRE-1860) into 5 
pieces.
   The line you are asking for is in my IDEA:
   `target.systemProperties( report.getSystemProperties(), report.getRunMode(), 
report.getTestRunId() );`
   Both getters will come in the next PR right after - 
[SUREFIRE-2015](https://issues.apache.org/jira/browse/SUREFIRE-2015)
   
   




-- 
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