You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/03/26 14:50:32 UTC
[GitHub] [maven-surefire] kalgon opened a new pull request #342: Fix for SUREFIRE-1659
kalgon opened a new pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342
Fix candidate for SUREFIRE-1659
--
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:
users@infra.apache.org
[GitHub] [maven-surefire] kalgon commented on a change in pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
kalgon commented on a change in pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#discussion_r603340521
##########
File path: surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformStreamCorruptionIT.java
##########
@@ -49,12 +49,42 @@ public void warningIsNotEmitted() throws VerificationException
{
OutputValidator validator = unpack( "/surefire-1614-stream-corruption" )
.executeTest()
- .verifyErrorFree( 1 );
+ .verifyErrorFree( 2 );
Review comment:
You're right, this line should not have changed. I first tried to add a new test in `surefire-1614-stream-corruption` (that's where the `2` comes from) but in the end, I created a new mini-project in `surefire-1659-stream-corruption` because the easiest way to trigger the problem is to have a `junit-platform.properties` and a redirection from `java.util.logging` to any other logging framework (I tested with both log4j and slf4j/logback). I'll change that `2` back to `1` ASAP. Do you have any other remark?
--
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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#discussion_r603314143
##########
File path: surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformStreamCorruptionIT.java
##########
@@ -49,12 +49,42 @@ public void warningIsNotEmitted() throws VerificationException
{
OutputValidator validator = unpack( "/surefire-1614-stream-corruption" )
.executeTest()
- .verifyErrorFree( 1 );
+ .verifyErrorFree( 2 );
Review comment:
Did you change the project within `surefire-1614-stream-corruption`? I do not understand why `2` 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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#issuecomment-809546751
Finally, squash the commits and rename the final commit to the name of 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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#discussion_r603455276
##########
File path: surefire-its/src/test/resources/surefire-1659-stream-corruption/pom.xml
##########
@@ -0,0 +1,88 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>org.apache.maven.plugins.surefire</groupId>
+ <artifactId>junit-platform-1.0.0</artifactId>
+ <version>1.0</version>
+ <name>[SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT.</name>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>1.8</maven.compiler.source>
Review comment:
Please turn `1.8` to `${java.specification.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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#discussion_r603456583
##########
File path: surefire-its/src/test/resources/surefire-1659-stream-corruption/pom.xml
##########
@@ -0,0 +1,88 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>org.apache.maven.plugins.surefire</groupId>
+ <artifactId>junit-platform-1.0.0</artifactId>
+ <version>1.0</version>
+ <name>[SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT.</name>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>1.8</maven.compiler.source>
+ <maven.compiler.target>1.8</maven.compiler.target>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <version>5.7.0</version>
Review comment:
Is `5.7.0` mandatory?
Can we use `5.7.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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#discussion_r602853109
##########
File path: surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/LazyLauncher.java
##########
@@ -0,0 +1,60 @@
+package org.apache.maven.surefire.junitplatform;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.junit.platform.launcher.Launcher;
+import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.TestExecutionListener;
+import org.junit.platform.launcher.TestPlan;
+import org.junit.platform.launcher.core.LauncherFactory;
+
+class LazyLauncher implements Launcher
Review comment:
Pls put a Javadoc for the class with the purpose of the class.
--
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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 merged pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 merged pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342
--
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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#discussion_r603457199
##########
File path: surefire-its/src/test/java/org/apache/maven/surefire/its/JUnitPlatformStreamCorruptionIT.java
##########
@@ -49,12 +49,42 @@ public void warningIsNotEmitted() throws VerificationException
{
OutputValidator validator = unpack( "/surefire-1614-stream-corruption" )
.executeTest()
- .verifyErrorFree( 1 );
+ .verifyErrorFree( 2 );
Review comment:
Thx for making it clear. Yes there are some little changes only.
--
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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#issuecomment-808871391
@kalgon Can you extend the integration test `JUnitPlatformStreamCorruptionIT`? Its purpose is to assert that the console logs do not contain the message `Corrupted channel by directly writing to native stream in forked JVM`. The POM and project which is tested by this IT appears in `surefire-its/src/test/resources/surefire-1614-stream-corruption`. All you need to do is to add a new maven profile in the `pom.xml` and the motivation can be found in the [comment](https://issues.apache.org/jira/browse/SUREFIRE-1659?focusedCommentId=17075165&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17075165) where you can see the zip file `src.zip`.
--
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:
users@infra.apache.org
[GitHub] [maven-surefire] kalgon commented on pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
kalgon commented on pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#issuecomment-809300165
@Tibor17 Thanks for the feedback and for pointing me how to test the fix (I really had no idea how to test that). I'll take a look at 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:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on pull request #342: [SUREFIRE-1659] Log4j logger in TestExecutionListener corrupts Surefire's STDOUT
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on pull request #342:
URL: https://github.com/apache/maven-surefire/pull/342#issuecomment-810932158
@kalgon
Thx for contributing! I am going to publish pull requests where I need to have clever participants. Feel free to track the pull requests in the project.
--
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:
users@infra.apache.org