This is an automated email from the ASF dual-hosted git repository. tibordigana pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
commit 5a1c7430967710b2fe50c19cb6742f4d7dcdb9b7 Author: tibordigana <tibordig...@apache.org> AuthorDate: Mon May 4 17:22:59 2020 +0200 [SUREFIRE-1788] Unhandled native logs in SurefireForkChannel --- .../plugin/surefire/booterclient/ForkStarter.java | 6 ++--- .../output/NativeStdErrStreamConsumer.java | 14 +++++------ .../output/NativeStdOutStreamConsumer.java | 16 +++++++++---- .../surefire/extensions/LegacyForkChannel.java | 4 ++-- .../surefire/extensions/SurefireForkChannel.java | 13 +++++++--- .../maven/plugin/surefire/extensions/E2ETest.java | 7 +++++- .../maven/surefire/extensions/ForkChannelTest.java | 15 ++++++++---- .../maven/surefire/extensions/ForkChannel.java | 10 ++------ .../surefire/extensions/StdOutStreamLine.java | 28 ---------------------- 9 files changed, 52 insertions(+), 61 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java index 44c9f99..428f4e3 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java @@ -631,7 +631,7 @@ public class ForkStarter DefaultReporterFactory reporter = forkClient.getDefaultReporterFactory(); currentForkClients.add( forkClient ); CountdownCloseable countdownCloseable = - new CountdownCloseable( eventConsumer, 1 + ( forkChannel.useStdOut() ? 1 : 0 ) ); + new CountdownCloseable( eventConsumer, forkChannel.getCountdownCloseablePermits() ); try ( CommandlineExecutor exec = new CommandlineExecutor( cli, countdownCloseable ) ) { CommandlineStreams streams = exec.execute(); @@ -646,8 +646,8 @@ public class ForkStarter out = forkChannel.bindEventHandler( eventConsumer, countdownCloseable, streams.getStdOutChannel() ); out.start(); - EventHandler<String> errConsumer = new NativeStdErrStreamConsumer( reporter ); - err = new LineConsumerThread( "fork-" + forkNumber + "-err-thread-", streams.getStdErrChannel(), + EventHandler<String> errConsumer = new NativeStdErrStreamConsumer( log ); + err = new LineConsumerThread( "fork-" + forkNumber + "-err-thread", streams.getStdErrChannel(), errConsumer, countdownCloseable ); err.start(); diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdErrStreamConsumer.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdErrStreamConsumer.java index bc0bc7c..d7136ff 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdErrStreamConsumer.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdErrStreamConsumer.java @@ -19,13 +19,14 @@ package org.apache.maven.plugin.surefire.booterclient.output; * under the License. */ -import org.apache.maven.plugin.surefire.report.DefaultReporterFactory; +import org.apache.maven.plugin.surefire.log.api.ConsoleLogger; import org.apache.maven.surefire.extensions.EventHandler; import javax.annotation.Nonnull; /** - * Used by forked JMV, see {@link org.apache.maven.plugin.surefire.booterclient.ForkStarter}. + * The error logger for the error stream of the forked JMV, + * see {@link org.apache.maven.plugin.surefire.booterclient.ForkStarter}. * * @author <a href="mailto:tibordig...@apache.org">Tibor Digana (tibor17)</a> * @since 2.20 @@ -34,17 +35,16 @@ import javax.annotation.Nonnull; public final class NativeStdErrStreamConsumer implements EventHandler<String> { - private final DefaultReporterFactory defaultReporterFactory; + private final ConsoleLogger logger; - public NativeStdErrStreamConsumer( DefaultReporterFactory defaultReporterFactory ) + public NativeStdErrStreamConsumer( ConsoleLogger logger ) { - this.defaultReporterFactory = defaultReporterFactory; + this.logger = logger; } @Override public void handleEvent( @Nonnull String line ) { - InPluginProcessDumpSingleton.getSingleton() - .dumpStreamText( line, defaultReporterFactory.getReportsDirectory() ); + logger.error( line ); } } diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdOutStreamConsumer.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdOutStreamConsumer.java index 1f915ae..0d9cef2 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdOutStreamConsumer.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdOutStreamConsumer.java @@ -20,13 +20,19 @@ package org.apache.maven.plugin.surefire.booterclient.output; */ import org.apache.maven.plugin.surefire.log.api.ConsoleLogger; -import org.apache.maven.surefire.extensions.StdOutStreamLine; +import org.apache.maven.surefire.extensions.EventHandler; + +import javax.annotation.Nonnull; /** + * The output/INFO logger for the output stream of the forked JMV, + * see org.apache.maven.plugin.surefire.extensions.SurefireForkChannel. * + * @author <a href="mailto:tibordig...@apache.org">Tibor Digana (tibor17)</a> + * @since 3.0.0-M5 */ -public class NativeStdOutStreamConsumer - implements StdOutStreamLine +public final class NativeStdOutStreamConsumer + implements EventHandler<String> { private final ConsoleLogger logger; @@ -36,8 +42,8 @@ public class NativeStdOutStreamConsumer } @Override - public void handleLine( String line ) + public void handleEvent( @Nonnull String event ) { - logger.info( line ); + logger.info( event ); } } diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/LegacyForkChannel.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/LegacyForkChannel.java index e13846b..2edb824 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/LegacyForkChannel.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/LegacyForkChannel.java @@ -58,9 +58,9 @@ final class LegacyForkChannel extends ForkChannel } @Override - public boolean useStdOut() + public int getCountdownCloseablePermits() { - return true; + return 2; } @Override diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/SurefireForkChannel.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/SurefireForkChannel.java index 11aeb18..0cf7115 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/SurefireForkChannel.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/SurefireForkChannel.java @@ -19,6 +19,7 @@ package org.apache.maven.plugin.surefire.extensions; * under the License. */ +import org.apache.maven.plugin.surefire.booterclient.output.NativeStdOutStreamConsumer; import org.apache.maven.surefire.eventapi.Event; import org.apache.maven.surefire.extensions.CloseableDaemonThread; import org.apache.maven.surefire.extensions.CommandReader; @@ -26,6 +27,7 @@ import org.apache.maven.surefire.extensions.EventHandler; import org.apache.maven.surefire.extensions.ForkChannel; import org.apache.maven.surefire.extensions.ForkNodeArguments; import org.apache.maven.surefire.extensions.util.CountdownCloseable; +import org.apache.maven.surefire.extensions.util.LineConsumerThread; import javax.annotation.Nonnull; import java.io.Closeable; @@ -75,6 +77,7 @@ final class SurefireForkChannel extends ForkChannel private final String localHost; private final int localPort; private volatile AsynchronousSocketChannel worker; + private volatile LineConsumerThread out; SurefireForkChannel( @Nonnull ForkNodeArguments arguments ) throws IOException { @@ -130,9 +133,9 @@ final class SurefireForkChannel extends ForkChannel } @Override - public boolean useStdOut() + public int getCountdownCloseablePermits() { - return false; + return 3; } @Override @@ -152,6 +155,10 @@ final class SurefireForkChannel extends ForkChannel @Nonnull CountdownCloseable countdownCloseable, ReadableByteChannel stdOut ) { + out = new LineConsumerThread( "fork-" + getArguments().getForkChannelId() + "-out-thread", stdOut, + new NativeStdOutStreamConsumer( getArguments().getConsoleLogger() ), countdownCloseable ); + out.start(); + ReadableByteChannel channel = newBufferedChannel( newInputStream( worker ) ); return new EventConsumerThread( "fork-" + getArguments().getForkChannelId() + "-event-thread", channel, eventHandler, countdownCloseable, getArguments() ); @@ -161,7 +168,7 @@ final class SurefireForkChannel extends ForkChannel public void close() throws IOException { //noinspection unused,EmptyTryBlock,EmptyTryBlock - try ( Closeable c1 = worker; Closeable c2 = server ) + try ( Closeable c1 = worker; Closeable c2 = server; Closeable c3 = out ) { // only close all channels } diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/E2ETest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/E2ETest.java index 80a3474..5f63b9b 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/E2ETest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/E2ETest.java @@ -31,12 +31,15 @@ import org.junit.Test; import javax.annotation.Nonnull; import java.io.Closeable; +import java.nio.ByteBuffer; +import java.nio.channels.ReadableByteChannel; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -162,7 +165,9 @@ public class E2ETest } }; - server.bindEventHandler( h, new CountdownCloseable( c, 1 ), null ) + ReadableByteChannel stdOut = mock( ReadableByteChannel.class ); + when( stdOut.read( any( ByteBuffer.class ) ) ).thenReturn( -1 ); + server.bindEventHandler( h, new CountdownCloseable( c, 1 ), stdOut ) .start(); assertThat( awaitHandlerFinished.await( 30L, TimeUnit.SECONDS ) ) diff --git a/maven-surefire-common/src/test/java/org/apache/maven/surefire/extensions/ForkChannelTest.java b/maven-surefire-common/src/test/java/org/apache/maven/surefire/extensions/ForkChannelTest.java index 7b20825..070de1f 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/surefire/extensions/ForkChannelTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/surefire/extensions/ForkChannelTest.java @@ -35,6 +35,8 @@ import java.io.File; import java.io.IOException; import java.net.Socket; import java.net.URI; +import java.nio.ByteBuffer; +import java.nio.channels.ReadableByteChannel; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CountDownLatch; @@ -43,6 +45,9 @@ import java.util.concurrent.atomic.AtomicBoolean; import static java.nio.charset.StandardCharsets.US_ASCII; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * @@ -90,8 +95,8 @@ public class ForkChannelTest assertThat( channel.getArguments().getForkChannelId() ) .isEqualTo( 1 ); - assertThat( channel.useStdOut() ) - .isFalse(); + assertThat( channel.getCountdownCloseablePermits() ) + .isEqualTo( 3 ); assertThat( channel.getForkNodeConnectionString() ) .startsWith( "tcp://127.0.0.1:" ) @@ -113,7 +118,7 @@ public class ForkChannelTest isCloseableCalled.countDown(); } }; - CountdownCloseable cc = new CountdownCloseable( closeable, 1 ); + CountdownCloseable cc = new CountdownCloseable( closeable, 2 ); Consumer consumer = new Consumer(); Client client = new Client( uri.getPort() ); @@ -121,7 +126,9 @@ public class ForkChannelTest channel.connectToClient(); channel.bindCommandReader( commandReader, null ).start(); - channel.bindEventHandler( consumer, cc, null ).start(); + ReadableByteChannel stdOut = mock( ReadableByteChannel.class ); + when( stdOut.read( any( ByteBuffer.class ) ) ).thenReturn( -1 ); + channel.bindEventHandler( consumer, cc, stdOut ).start(); commandReader.noop(); diff --git a/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/ForkChannel.java b/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/ForkChannel.java index dac54f2..4b24bb6 100644 --- a/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/ForkChannel.java +++ b/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/ForkChannel.java @@ -62,15 +62,9 @@ public abstract class ForkChannel implements Closeable public abstract String getForkNodeConnectionString(); /** - * Determines which one of the two <em>bindEventHandler-s</em> to call in <em>ForkStarter</em>. - * Can be called anytime. - * - * @return If {@code true}, both {@link ReadableByteChannel} and {@link CountdownCloseable} must not be null - * in {@link #bindEventHandler(EventHandler, CountdownCloseable, ReadableByteChannel)}. If {@code false} then - * both {@link ReadableByteChannel} and {@link CountdownCloseable} have to be null - * in {@link #bindEventHandler(EventHandler, CountdownCloseable, ReadableByteChannel)}. + * the permits in {@link CountdownCloseable}. */ - public abstract boolean useStdOut(); + public abstract int getCountdownCloseablePermits(); /** * Binds command handler to the channel. diff --git a/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/StdOutStreamLine.java b/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/StdOutStreamLine.java deleted file mode 100644 index f615764..0000000 --- a/surefire-extensions-api/src/main/java/org/apache/maven/surefire/extensions/StdOutStreamLine.java +++ /dev/null @@ -1,28 +0,0 @@ -package org.apache.maven.surefire.extensions; - -/* - * 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. - */ - -/** - * The line handler of forked process standard-output. - */ -public interface StdOutStreamLine -{ - void handleLine( String line ); -}