Repository: maven-surefire Updated Branches: refs/heads/master 2360dacc9 -> 572954801
[SUREFIRE-1322] Surefire and Failsafe should dump critical errors in dump file and console Project: http://git-wip-us.apache.org/repos/asf/maven-surefire/repo Commit: http://git-wip-us.apache.org/repos/asf/maven-surefire/commit/57295480 Tree: http://git-wip-us.apache.org/repos/asf/maven-surefire/tree/57295480 Diff: http://git-wip-us.apache.org/repos/asf/maven-surefire/diff/57295480 Branch: refs/heads/master Commit: 572954801c0d192a68b42116f28a7dd4c4c20ba6 Parents: 2360dac Author: Tibor17 <tibo...@lycos.com> Authored: Mon Feb 13 18:20:26 2017 +0100 Committer: Tibor17 <tibo...@lycos.com> Committed: Mon Feb 13 18:20:26 2017 +0100 ---------------------------------------------------------------------- .../surefire/booterclient/ForkStarter.java | 13 ++- .../booterclient/output/ForkClient.java | 18 ++- .../output/InPluginProcessDumpSingleton.java | 65 +++++++++++ .../output/LostCommandsDumpSingleton.java | 1 - .../output/MultipleFailureException.java | 72 ++++++++++++ .../output/NativeStdErrStreamConsumer.java | 2 +- .../output/ThreadedStreamConsumer.java | 112 ++++++++++++------- .../maven/surefire/booter/CommandReader.java | 26 ++--- .../surefire/booter/DumpErrorSingleton.java | 99 ++++++++++++++++ .../surefire/booter/ForkingRunListener.java | 13 +++ .../surefire/booter/MasterProcessCommand.java | 23 +--- .../maven/surefire/booter/ForkedBooter.java | 25 +---- ...urefire1295AttributeJvmCrashesToTestsIT.java | 6 +- .../jiras/Surefire141PluggableProvidersIT.java | 75 ++++++++++--- ...e735ForkFailWithRedirectConsoleOutputIT.java | 29 +++-- 15 files changed, 439 insertions(+), 140 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/ForkStarter.java ---------------------------------------------------------------------- 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 05c4cc2..54d304a 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 @@ -28,6 +28,7 @@ import org.apache.maven.plugin.surefire.booterclient.lazytestprovider.OutputStre import org.apache.maven.plugin.surefire.booterclient.lazytestprovider.TestLessInputStream; import org.apache.maven.plugin.surefire.booterclient.lazytestprovider.TestProvidingInputStream; import org.apache.maven.plugin.surefire.booterclient.output.ForkClient; +import org.apache.maven.plugin.surefire.booterclient.output.InPluginProcessDumpSingleton; import org.apache.maven.plugin.surefire.booterclient.output.NativeStdErrStreamConsumer; import org.apache.maven.plugin.surefire.booterclient.output.ThreadedStreamConsumer; import org.apache.maven.plugin.surefire.log.api.ConsoleLogger; @@ -158,7 +159,7 @@ public class ForkStarter /** * Closes stuff, with a shutdown hook to make sure things really get closed. */ - private static class CloseableCloser + private final class CloseableCloser implements Runnable, Closeable { private final Queue<Closeable> testProvidingInputStream; @@ -191,7 +192,15 @@ public class ForkStarter } catch ( IOException e ) { - // ignore + // This error does not fail a test and does not necessarily mean that the forked JVM std/out stream + // was not closed, see ThreadedStreamConsumer. This error means that JVM wrote messages to a native + // stream which could not be parsed or report failed. The tests may still correctly run nevertheless + // this exception happened => warning on console. The user would see hint to check dump file only + // if tests failed, but if this does not happen then printing warning to console is the only way to + // inform the users. + String msg = "ForkStarter IOException: " + e.getLocalizedMessage(); + log.warning( msg ); + InPluginProcessDumpSingleton.getSingleton().dumpException( e, msg, defaultReporterFactory ); } } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java index 68ce40d..d6afcb2 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClient.java @@ -25,7 +25,6 @@ import org.apache.maven.plugin.surefire.report.DefaultReporterFactory; import org.apache.maven.shared.utils.cli.StreamConsumer; import org.apache.maven.surefire.report.ConsoleOutputReceiver; import org.apache.maven.surefire.report.ReportEntry; -import org.apache.maven.surefire.report.ReporterException; import org.apache.maven.surefire.report.RunListener; import org.apache.maven.surefire.report.StackTraceWriter; @@ -279,22 +278,29 @@ public class ForkClient .warning( createConsoleMessage( remaining ) ); break; default: - LostCommandsDumpSingleton.getSingleton().dumpText( s, defaultReporterFactory ); + InPluginProcessDumpSingleton.getSingleton().dumpText( s, defaultReporterFactory ); } } catch ( NumberFormatException e ) { // SUREFIRE-859 - LostCommandsDumpSingleton.getSingleton().dumpException( e, s, defaultReporterFactory ); + InPluginProcessDumpSingleton.getSingleton().dumpException( e, s, defaultReporterFactory ); } catch ( NoSuchElementException e ) { // SUREFIRE-859 - LostCommandsDumpSingleton.getSingleton().dumpException( e, s, defaultReporterFactory ); + InPluginProcessDumpSingleton.getSingleton().dumpException( e, s, defaultReporterFactory ); } - catch ( ReporterException e ) + catch ( IndexOutOfBoundsException e ) { - LostCommandsDumpSingleton.getSingleton().dumpException( e, s, defaultReporterFactory ); + // native stream sent a text e.g. GC verbose + InPluginProcessDumpSingleton.getSingleton().dumpException( e, s, defaultReporterFactory ); + throw e; + } + catch ( RuntimeException e ) + { + // e.g. ReporterException + InPluginProcessDumpSingleton.getSingleton().dumpException( e, s, defaultReporterFactory ); throw e; } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/InPluginProcessDumpSingleton.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/InPluginProcessDumpSingleton.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/InPluginProcessDumpSingleton.java new file mode 100644 index 0000000..6580c43 --- /dev/null +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/InPluginProcessDumpSingleton.java @@ -0,0 +1,65 @@ +package org.apache.maven.plugin.surefire.booterclient.output; + +/* + * 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.apache.maven.plugin.surefire.report.DefaultReporterFactory; +import org.apache.maven.surefire.util.internal.DumpFileUtils; +import java.io.File; + +/** + * Reports errors to dump file. + * Used only within java process of the plugin itself and not the forked JVM. + */ +public final class InPluginProcessDumpSingleton +{ + private static final InPluginProcessDumpSingleton SINGLETON = new InPluginProcessDumpSingleton(); + + private final String creationDate = DumpFileUtils.newFormattedDateFileName(); + + private InPluginProcessDumpSingleton() + { + } + + public static InPluginProcessDumpSingleton getSingleton() + { + return SINGLETON; + } + + public synchronized void dumpException( Throwable t, String msg, DefaultReporterFactory defaultReporterFactory ) + { + DumpFileUtils.dumpException( t, msg == null ? "null" : msg, newDumpFile( defaultReporterFactory ) ); + } + + public synchronized void dumpException( Throwable t, DefaultReporterFactory defaultReporterFactory ) + { + DumpFileUtils.dumpException( t, newDumpFile( defaultReporterFactory ) ); + } + + public synchronized void dumpText( String msg, DefaultReporterFactory defaultReporterFactory ) + { + DumpFileUtils.dumpText( msg == null ? "null" : msg, newDumpFile( defaultReporterFactory ) ); + } + + private File newDumpFile( DefaultReporterFactory defaultReporterFactory ) + { + File reportsDirectory = defaultReporterFactory.getReportsDirectory(); + return new File( reportsDirectory, creationDate + ".dumpstream" ); + } +} http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/LostCommandsDumpSingleton.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/LostCommandsDumpSingleton.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/LostCommandsDumpSingleton.java deleted file mode 100644 index 364d8c3..0000000 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/LostCommandsDumpSingleton.java +++ /dev/null @@ -1 +0,0 @@ -package org.apache.maven.plugin.surefire.booterclient.output; /* * 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.apache.maven.plugin.surefire.report.DefaultReporterFactory; import org.apache.maven.surefire.util.internal.DumpF ileUtils; import java.io.File; /** * Dumps lost commands and caused exceptions in {@link ForkClient}. * * @author <a href="mailto:tibordig...@apache.org">Tibor Digana (tibor17)</a> * @since 2.19.2 */ final class LostCommandsDumpSingleton { private static final LostCommandsDumpSingleton SINGLETON = new LostCommandsDumpSingleton(); private final String creationDate = DumpFileUtils.newFormattedDateFileName(); private LostCommandsDumpSingleton() { } static LostCommandsDumpSingleton getSingleton() { return SINGLETON; } synchronized void dumpException( Throwable t, String msg, DefaultReporterFactory defaultReporterFactory ) { File reportsDirectory = defaultReporterFactory.getReportsDirectory(); File dumpFile = new File( reportsDirectory, creationDate + ".dumpstream" ); DumpFileUtils.dumpException( t, msg, dumpFile ); } synchronized void dumpException( Throwable t, DefaultReporterFactory defaultRepo rterFactory ) { dumpException( t, null, defaultReporterFactory ); } synchronized void dumpText( String msg, DefaultReporterFactory defaultReporterFactory ) { File reportsDirectory = defaultReporterFactory.getReportsDirectory(); File dumpFile = new File( reportsDirectory, creationDate + ".dumpstream" ); DumpFileUtils.dumpText( msg, dumpFile ); } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/MultipleFailureException.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/MultipleFailureException.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/MultipleFailureException.java new file mode 100644 index 0000000..4149ad9 --- /dev/null +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/MultipleFailureException.java @@ -0,0 +1,72 @@ +package org.apache.maven.plugin.surefire.booterclient.output; + +/* + * 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 java.io.IOException; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; + +final class MultipleFailureException + extends IOException +{ + private final Queue<Throwable> exceptions = new ConcurrentLinkedQueue<Throwable>(); + + void addException( Throwable exception ) + { + exceptions.add( exception ); + } + + boolean hasNestedExceptions() + { + return !exceptions.isEmpty(); + } + + @Override + public String getLocalizedMessage() + { + StringBuilder messages = new StringBuilder(); + for ( Throwable exception = exceptions.peek(); exception != null; exception = exceptions.peek() ) + { + if ( messages.length() != 0 ) + { + messages.append( '\n' ); + } + String message = exception.getLocalizedMessage(); + messages.append( message == null ? exception.toString() : message ); + } + return messages.toString(); + } + + @Override + public String getMessage() + { + StringBuilder messages = new StringBuilder(); + for ( Throwable exception = exceptions.peek(); exception != null; exception = exceptions.peek() ) + { + if ( messages.length() != 0 ) + { + messages.append( '\n' ); + } + String message = exception.getMessage(); + messages.append( message == null ? exception.toString() : message ); + } + return messages.toString(); + } +} http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/NativeStdErrStreamConsumer.java ---------------------------------------------------------------------- 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 c92361d..48063d3 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 @@ -41,6 +41,6 @@ public final class NativeStdErrStreamConsumer public void consumeLine( String line ) { - LostCommandsDumpSingleton.getSingleton().dumpText( line, defaultReporterFactory ); + InPluginProcessDumpSingleton.getSingleton().dumpText( line, defaultReporterFactory ); } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java ---------------------------------------------------------------------- diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java index 3f1abd8..71831c0 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/output/ThreadedStreamConsumer.java @@ -22,9 +22,12 @@ package org.apache.maven.plugin.surefire.booterclient.output; import org.apache.maven.shared.utils.cli.StreamConsumer; import org.apache.maven.surefire.util.internal.DaemonThreadFactory; -import java.util.concurrent.BlockingQueue; import java.io.Closeable; -import java.util.concurrent.LinkedBlockingQueue; +import java.io.IOException; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; + +import static java.lang.Thread.currentThread; /** * Knows how to reconstruct *all* the state transmitted over stdout by the forked process. @@ -32,100 +35,123 @@ import java.util.concurrent.LinkedBlockingQueue; * @author Kristian Rosenvold */ public final class ThreadedStreamConsumer - implements StreamConsumer, Closeable + implements StreamConsumer, Closeable { - private static final String POISON = "Pioson"; + private static final String END_ITEM = ""; private static final int ITEM_LIMIT_BEFORE_SLEEP = 10000; - private final BlockingQueue<String> items = new LinkedBlockingQueue<String>(); + private final BlockingQueue<String> items = new ArrayBlockingQueue<String>( ITEM_LIMIT_BEFORE_SLEEP ); private final Thread thread; private final Pumper pumper; - static class Pumper - implements Runnable - { - private final BlockingQueue<String> queue; + private volatile boolean closed; + final class Pumper + implements Runnable + { private final StreamConsumer target; - private volatile Throwable throwable; - + private final MultipleFailureException errors = new MultipleFailureException(); - Pumper( BlockingQueue<String> queue, StreamConsumer target ) + Pumper( StreamConsumer target ) { - this.queue = queue; this.target = target; } + /** + * Calls {@link ForkClient#consumeLine(String)} which may throw any {@link RuntimeException}.<p/> + * Even if {@link ForkClient} is not fault-tolerant, this method MUST be fault-tolerant and thus the + * try-catch block must be inside of the loop which prevents from loosing events from {@link StreamConsumer}. + * <p/> + * If {@link org.apache.maven.plugin.surefire.report.ConsoleOutputFileReporter#writeTestOutput} throws + * {@link java.io.IOException} and then <em>target.consumeLine()</em> throws any RuntimeException, this method + * MUST NOT skip reading the events from the forked JVM; otherwise we could simply lost events + * e.g. acquire-next-test which means that {@link ForkClient} could hang on waiting for old test to complete + * and therefore the plugin could be permanently in progress. + */ public void run() { - try + while ( !ThreadedStreamConsumer.this.closed ) { - String item = queue.take(); - //noinspection StringEquality - while ( item != POISON ) + try { + String item = ThreadedStreamConsumer.this.items.take(); + if ( shouldStopQueueing( item ) ) + { + break; + } target.consumeLine( item ); - item = queue.take(); } - } - catch ( Throwable t ) - { - // Think about what happens if the producer overruns us and creates an OOME. Not nice. - // Maybe limit length of blocking queue - this.throwable = t; + catch ( Throwable t ) + { + errors.addException( t ); + } } } - public Throwable getThrowable() + boolean hasErrors() { - return throwable; + return errors.hasNestedExceptions(); + } + + void throwErrors() throws IOException + { + throw errors; } } public ThreadedStreamConsumer( StreamConsumer target ) { - pumper = new Pumper( items, target ); + pumper = new Pumper( target ); thread = DaemonThreadFactory.newDaemonThread( pumper, "ThreadedStreamConsumer" ); thread.start(); } - @SuppressWarnings( "checkstyle:emptyblock" ) public void consumeLine( String s ) { - items.add( s ); - if ( items.size() > ITEM_LIMIT_BEFORE_SLEEP ) + if ( closed && !thread.isAlive() ) { - try - { - Thread.sleep( 100 ); - } - catch ( InterruptedException ignore ) - { - } + items.clear(); + return; } - } + try + { + items.put( s ); + } + catch ( InterruptedException e ) + { + currentThread().interrupt(); + throw new IllegalStateException( e ); + } + } public void close() + throws IOException { try { - items.add( POISON ); + closed = true; + items.put( END_ITEM ); thread.join(); } catch ( InterruptedException e ) { - throw new RuntimeException( e ); + currentThread().interrupt(); + throw new IOException( e ); } - //noinspection ThrowableResultOfMethodCallIgnored - if ( pumper.getThrowable() != null ) + if ( pumper.hasErrors() ) { - throw new RuntimeException( pumper.getThrowable() ); + pumper.throwErrors(); } } + + private boolean shouldStopQueueing( String item ) + { + return closed && item == END_ITEM; + } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java ---------------------------------------------------------------------- diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java b/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java index 408e4a4..57f1c2c 100644 --- a/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java +++ b/surefire-api/src/main/java/org/apache/maven/surefire/booter/CommandReader.java @@ -22,11 +22,9 @@ package org.apache.maven.surefire.booter; import org.apache.maven.plugin.surefire.log.api.ConsoleLogger; import org.apache.maven.plugin.surefire.log.api.NullConsoleLogger; import org.apache.maven.surefire.testset.TestSetFailedException; -import org.apache.maven.surefire.util.internal.DumpFileUtils; import java.io.DataInputStream; import java.io.EOFException; -import java.io.File; import java.io.IOException; import java.io.PrintStream; import java.util.Iterator; @@ -87,8 +85,6 @@ public final class CommandReader private volatile ConsoleLogger logger = new NullConsoleLogger(); - private volatile File dumpFile; - private CommandReader() { } @@ -103,11 +99,6 @@ public final class CommandReader return reader; } - public void setDumpFile( File dumpFile ) - { - this.dumpFile = dumpFile; - } - public CommandReader setShutdown( Shutdown shutdown ) { this.shutdown = shutdown; @@ -132,7 +123,7 @@ public final class CommandReader } catch ( InterruptedException e ) { - DumpFileUtils.dumpException( e, dumpFile ); + DumpErrorSingleton.getSingleton().dumpException( e ); throw new TestSetFailedException( e.getLocalizedMessage() ); } } @@ -386,7 +377,7 @@ public final class CommandReader if ( command == null ) { String errorMessage = "[SUREFIRE] std/in stream corrupted: first sequence not recognized"; - DumpFileUtils.dumpText( errorMessage, dumpFile ); + DumpErrorSingleton.getSingleton().dumpStreamText( errorMessage ); logger.error( errorMessage ); break; } @@ -423,24 +414,27 @@ public final class CommandReader } catch ( EOFException e ) { - DumpFileUtils.dumpException( e, dumpFile ); - CommandReader.this.state.set( TERMINATED ); if ( !isTestSetFinished ) { + String msg = "TestSet has not finished before stream error has appeared >> " + + "initializing exit by non-null configuration: " + + CommandReader.this.shutdown; + DumpErrorSingleton.getSingleton().dumpStreamException( e, msg ); + exitByConfiguration(); // does not go to finally } } catch ( IOException e ) { - DumpFileUtils.dumpException( e, dumpFile ); - CommandReader.this.state.set( TERMINATED ); // If #stop() method is called, reader thread is interrupted and cause is InterruptedException. if ( !( e.getCause() instanceof InterruptedException ) ) { - logger.error( "[SUREFIRE] std/in stream corrupted", e ); + String msg = "[SUREFIRE] std/in stream corrupted"; + DumpErrorSingleton.getSingleton().dumpStreamException( e, msg ); + logger.error( msg, e ); } } finally http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/surefire-api/src/main/java/org/apache/maven/surefire/booter/DumpErrorSingleton.java ---------------------------------------------------------------------- diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/booter/DumpErrorSingleton.java b/surefire-api/src/main/java/org/apache/maven/surefire/booter/DumpErrorSingleton.java new file mode 100644 index 0000000..5df6d59 --- /dev/null +++ b/surefire-api/src/main/java/org/apache/maven/surefire/booter/DumpErrorSingleton.java @@ -0,0 +1,99 @@ +package org.apache.maven.surefire.booter; + +/* + * 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.apache.maven.surefire.report.ReporterConfiguration; +import org.apache.maven.surefire.util.internal.DumpFileUtils; + +import java.io.File; + +import static org.apache.maven.surefire.util.internal.DumpFileUtils.newDumpFile; + +/** + * Dumps lost commands and caused exceptions in forked JVM. <p/> + * Fail-safe. + * + * @author <a href="mailto:tibordig...@apache.org">Tibor Digana (tibor17)</a> + * @since 2.19.2 + */ +public final class DumpErrorSingleton +{ + private static final String DUMP_FILE_EXT = ".dump"; + private static final String DUMPSTREAM_FILE_EXT = ".dumpstream"; + private static final DumpErrorSingleton SINGLETON = new DumpErrorSingleton(); + + private File dumpFile; + private File dumpStreamFile; + + private DumpErrorSingleton() + { + } + + public static DumpErrorSingleton getSingleton() + { + return SINGLETON; + } + + public synchronized void init( String dumpFileName, ReporterConfiguration configuration ) + { + dumpFile = createDumpFile( dumpFileName, configuration ); + dumpStreamFile = createDumpStreamFile( dumpFileName, configuration ); + } + + public synchronized void dumpException( Throwable t, String msg ) + { + DumpFileUtils.dumpException( t, msg == null ? "null" : msg, dumpFile ); + } + + public synchronized void dumpException( Throwable t ) + { + DumpFileUtils.dumpException( t, dumpFile ); + } + + public synchronized void dumpText( String msg ) + { + DumpFileUtils.dumpText( msg == null ? "null" : msg, dumpFile ); + } + + public synchronized void dumpStreamException( Throwable t, String msg ) + { + DumpFileUtils.dumpException( t, msg == null ? "null" : msg, dumpStreamFile ); + } + + public synchronized void dumpStreamException( Throwable t ) + { + DumpFileUtils.dumpException( t, dumpStreamFile ); + } + + public synchronized void dumpStreamText( String msg ) + { + DumpFileUtils.dumpText( msg == null ? "null" : msg, dumpStreamFile ); + } + + private File createDumpFile( String dumpFileName, ReporterConfiguration configuration ) + { + return newDumpFile( dumpFileName + DUMP_FILE_EXT, configuration ); + } + + private File createDumpStreamFile( String dumpFileName, ReporterConfiguration configuration ) + { + return newDumpFile( dumpFileName + DUMPSTREAM_FILE_EXT, configuration ); + } +} http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/surefire-api/src/main/java/org/apache/maven/surefire/booter/ForkingRunListener.java ---------------------------------------------------------------------- diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/booter/ForkingRunListener.java b/surefire-api/src/main/java/org/apache/maven/surefire/booter/ForkingRunListener.java index 282c4d4..1216502 100644 --- a/surefire-api/src/main/java/org/apache/maven/surefire/booter/ForkingRunListener.java +++ b/surefire-api/src/main/java/org/apache/maven/surefire/booter/ForkingRunListener.java @@ -206,6 +206,13 @@ public class ForkingRunListener synchronized ( target ) // See notes about synchronization/thread safety in class javadoc { target.write( encodeBytes, 0, encodeBytes.length ); + if ( target.checkError() ) + { + // We MUST NOT throw any exception from this method; otherwise we are in loop and CPU goes up: + // ForkingRunListener -> Exception -> JUnit Notifier and RunListener -> ForkingRunListener -> Exception + DumpErrorSingleton.getSingleton() + .dumpStreamText( "Unexpected IOException with stream: " + new String( buf, off, len ) ); + } } } @@ -268,6 +275,12 @@ public class ForkingRunListener synchronized ( target ) // See notes about synchronization/thread safety in class javadoc { target.write( encodeBytes, 0, encodeBytes.length ); + if ( target.checkError() ) + { + // We MUST NOT throw any exception from this method; otherwise we are in loop and CPU goes up: + // ForkingRunListener -> Exception -> JUnit Notifier and RunListener -> ForkingRunListener -> Exception + DumpErrorSingleton.getSingleton().dumpStreamText( "Unexpected IOException: " + string ); + } } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java ---------------------------------------------------------------------- diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java b/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java index a53a046..d5e314a 100644 --- a/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java +++ b/surefire-api/src/main/java/org/apache/maven/surefire/booter/MasterProcessCommand.java @@ -22,7 +22,6 @@ package org.apache.maven.surefire.booter; import org.apache.maven.surefire.util.internal.StringUtils; import java.io.DataInputStream; -import java.io.EOFException; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; @@ -124,33 +123,15 @@ public enum MasterProcessCommand int dataLength = is.readInt(); if ( dataLength > 0 ) { - byte[] buffer = new byte[dataLength]; - int read = 0; - int total = 0; - do - { - total += read; - read = is.read( buffer, total, dataLength - total ); - } while ( read > 0 ); + byte[] buffer = new byte[ dataLength ]; + is.readFully( buffer ); if ( command.getDataType() == Void.class ) { - // must read entire sequence to get to the next command; cannot be above the loop throw new IOException( format( "Command %s unexpectedly read Void data with length %d.", command, dataLength ) ); } - if ( total != dataLength ) - { - if ( read == -1 ) - { - throw new EOFException( "stream closed" ); - } - - throw new IOException( format( "%s read %d out of %d bytes", - MasterProcessCommand.class, total, dataLength ) ); - } - String data = command.toDataTypeAsString( buffer ); return new Command( command, data ); } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java ---------------------------------------------------------------------- diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java index 29047f2..b76df2f 100644 --- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java +++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkedBooter.java @@ -54,8 +54,6 @@ import static org.apache.maven.surefire.booter.Shutdown.KILL; import static org.apache.maven.surefire.booter.SystemPropertyManager.setSystemProperties; import static org.apache.maven.surefire.util.ReflectionUtils.instantiateOneArg; import static org.apache.maven.surefire.util.internal.DaemonThreadFactory.newDaemonThreadFactory; -import static org.apache.maven.surefire.util.internal.DumpFileUtils.dumpException; -import static org.apache.maven.surefire.util.internal.DumpFileUtils.newDumpFile; import static org.apache.maven.surefire.util.internal.StringUtils.encodeStringForForkCommunication; /** @@ -73,8 +71,6 @@ public final class ForkedBooter private static final long DEFAULT_SYSTEM_EXIT_TIMEOUT_IN_SECONDS = 30; private static final long PING_TIMEOUT_IN_SECONDS = 20; private static final ScheduledExecutorService JVM_TERMINATOR = createJvmTerminator(); - private static final String DUMP_FILE_EXT = ".dump"; - private static final String DUMPSTREAM_FILE_EXT = ".dumpstream"; private static volatile long systemExitTimeoutInSeconds = DEFAULT_SYSTEM_EXIT_TIMEOUT_IN_SECONDS; @@ -89,7 +85,6 @@ public final class ForkedBooter final CommandReader reader = startupMasterProcessReader(); final ScheduledFuture<?> pingScheduler = listenToShutdownCommands( reader ); final PrintStream originalOut = out; - File dumpFile = null; try { final String tmpDir = args[0]; @@ -105,9 +100,7 @@ public final class ForkedBooter } final ProviderConfiguration providerConfiguration = booterDeserializer.deserialize(); - - dumpFile = createDumpFile( dumpFileName, providerConfiguration ); - reader.setDumpFile( createDumpstreamFile( dumpFileName, providerConfiguration ) ); + DumpErrorSingleton.getSingleton().init( dumpFileName, providerConfiguration.getReporterConfiguration() ); final StartupConfiguration startupConfiguration = booterDeserializer.getProviderConfiguration(); systemExitTimeoutInSeconds = @@ -145,7 +138,7 @@ public final class ForkedBooter } catch ( InvocationTargetException t ) { - dumpException( t, dumpFile ); + DumpErrorSingleton.getSingleton().dumpException( t ); StackTraceWriter stackTraceWriter = new LegacyPojoStackTraceWriter( "test subsystem", "no method", t.getTargetException() ); StringBuilder stringBuilder = new StringBuilder(); @@ -154,7 +147,7 @@ public final class ForkedBooter } catch ( Throwable t ) { - dumpException( t, dumpFile ); + DumpErrorSingleton.getSingleton().dumpException( t ); StackTraceWriter stackTraceWriter = new LegacyPojoStackTraceWriter( "test subsystem", "no method", t ); StringBuilder stringBuilder = new StringBuilder(); encode( stringBuilder, stackTraceWriter, false ); @@ -168,7 +161,7 @@ public final class ForkedBooter } catch ( Throwable t ) { - dumpException( t, dumpFile ); + DumpErrorSingleton.getSingleton().dumpException( t ); // Just throwing does getMessage() and a local trace - we want to call printStackTrace for a full trace // noinspection UseOfSystemOutOrSystemErr t.printStackTrace( err ); @@ -354,14 +347,4 @@ public final class ForkedBooter File surefirePropertiesFile = new File( tmpDir, propFileName ); return surefirePropertiesFile.exists() ? new FileInputStream( surefirePropertiesFile ) : null; } - - private static File createDumpFile( String dumpFileName, ProviderConfiguration providerConfiguration ) - { - return newDumpFile( dumpFileName + DUMP_FILE_EXT, providerConfiguration.getReporterConfiguration() ); - } - - private static File createDumpstreamFile( String dumpFileName, ProviderConfiguration providerConfiguration ) - { - return newDumpFile( dumpFileName + DUMPSTREAM_FILE_EXT, providerConfiguration.getReporterConfiguration() ); - } } http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1295AttributeJvmCrashesToTestsIT.java ---------------------------------------------------------------------- diff --git a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1295AttributeJvmCrashesToTestsIT.java b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1295AttributeJvmCrashesToTestsIT.java index f7f518d..710a236 100644 --- a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1295AttributeJvmCrashesToTestsIT.java +++ b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1295AttributeJvmCrashesToTestsIT.java @@ -27,10 +27,11 @@ import org.junit.Before; import org.junit.Test; import java.util.Iterator; +import java.util.Locale; import static org.fest.assertions.Assertions.assertThat; import static org.junit.Assert.fail; -import static org.junit.Assume.assumeFalse; +import static org.junit.Assume.assumeTrue; /** * https://issues.apache.org/jira/browse/SUREFIRE-1295 @@ -45,7 +46,8 @@ public class Surefire1295AttributeJvmCrashesToTestsIT @Before public void skipWindows() { - assumeFalse( System.getProperty( "os.name" ).startsWith( "Windows" ) ); + String os = System.getProperty( "os.name" ).toLowerCase( Locale.ROOT ); + assumeTrue( os.equals( "mac os x" ) || os.equals( "linux" ) /*|| os.contains( "windows" )*/ ); } @Test http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire141PluggableProvidersIT.java ---------------------------------------------------------------------- diff --git a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire141PluggableProvidersIT.java b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire141PluggableProvidersIT.java index fdc10a8..75aa743 100644 --- a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire141PluggableProvidersIT.java +++ b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire141PluggableProvidersIT.java @@ -22,6 +22,7 @@ package org.apache.maven.surefire.its.jiras; import org.apache.maven.it.VerificationException; import org.apache.maven.surefire.its.fixture.OutputValidator; import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase; +import org.apache.maven.surefire.its.fixture.SurefireVerifierException; import org.junit.BeforeClass; import org.junit.Test; @@ -64,52 +65,96 @@ public class Surefire141PluggableProvidersIT public void invokeRuntimeException() throws Exception { - unpack( "surefire-141-pluggableproviders" ) + final String errorText = "Let's fail with a runtimeException"; + + OutputValidator validator = unpack( "surefire-141-pluggableproviders" ) .sysProp( "invokeCrash", "runtimeException" ) .maven() .withFailure() .executeTest() - .verifyTextInLog( "Let's fail with a runtimeException" ); + .verifyTextInLog( errorText ); + + assertErrorMessage( validator, errorText ); } @Test public void invokeReporterException() throws Exception { - unpack( "surefire-141-pluggableproviders" ) + final String errorText = "Let's fail with a reporterexception"; + + OutputValidator validator = unpack( "surefire-141-pluggableproviders" ) .sysProp( "invokeCrash", "reporterException" ) .maven() .withFailure() .executeTest() - .verifyTextInLog( "Let's fail with a reporterexception" ); + .verifyTextInLog( errorText ); + + assertErrorMessage( validator, errorText ); } @Test public void constructorRuntimeException() throws Exception { + final String errorText = "Let's fail with a runtimeException"; + OutputValidator validator = unpack( "surefire-141-pluggableproviders" ) .sysProp( "constructorCrash", "runtimeException" ) .maven() .withFailure() - .executeTest() - .verifyTextInLog( "Let's fail with a runtimeException" ); + .executeTest(); + + boolean verifiedInLog = verifiedErrorInLog( validator, errorText ); + boolean verifiedInDump = verifiedErrorInDump( validator, errorText ); + assertThat( verifiedInLog || verifiedInDump ) + .describedAs( "'" + errorText + "' could not be verified in log.txt nor *.dump file." ) + .isTrue(); + } + private static void assertErrorMessage( OutputValidator validator, String message ) + { File reportDir = validator.getSurefireReportsDirectory(); String[] dumpFiles = reportDir.list( new FilenameFilter() - { - @Override - public boolean accept( File dir, String name ) - { - return name.endsWith( ".dump" ); - } - }); + { + @Override + public boolean accept( File dir, String name ) + { + return name.endsWith( ".dump" ); + } + }); assertThat( dumpFiles ).isNotEmpty(); for ( String dump : dumpFiles ) { validator.getSurefireReportsFile( dump ) - .assertContainsText( "Let's fail with a runtimeException" ); + .assertContainsText( message ); + } + } + + private static boolean verifiedErrorInLog( OutputValidator validator, String errorText ) + { + try + { + validator.verifyTextInLog( errorText ); + return true; + } + catch ( SurefireVerifierException e ) + { + return false; + } + } + + private static boolean verifiedErrorInDump( OutputValidator validator, String errorText ) + { + try + { + assertErrorMessage( validator, errorText ); + return true; + } + catch ( AssertionError e ) + { + return false; } } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/maven-surefire/blob/57295480/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire735ForkFailWithRedirectConsoleOutputIT.java ---------------------------------------------------------------------- diff --git a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire735ForkFailWithRedirectConsoleOutputIT.java b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire735ForkFailWithRedirectConsoleOutputIT.java index 4117d8c..34504c1 100644 --- a/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire735ForkFailWithRedirectConsoleOutputIT.java +++ b/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire735ForkFailWithRedirectConsoleOutputIT.java @@ -41,7 +41,24 @@ public class Surefire735ForkFailWithRedirectConsoleOutputIT throws Exception { OutputValidator outputValidator = unpack().failNever().executeTest(); + assertJvmCrashed( outputValidator ); + } + + @Test + public void vmStartFailShouldFailBuildk() + throws Exception + { + OutputValidator outputValidator = unpack().maven().withFailure().executeTest(); + assertJvmCrashed( outputValidator ); + } + + private SurefireLauncher unpack() + { + return unpack( "fork-fail" ); + } + private static void assertJvmCrashed( OutputValidator outputValidator ) + { File reportDir = outputValidator.getSurefireReportsDirectory(); String[] dumpFiles = reportDir.list( new FilenameFilter() { @@ -60,16 +77,4 @@ public class Surefire735ForkFailWithRedirectConsoleOutputIT } } - @Test - public void vmStartFailShouldFailBuildk() - throws Exception - { - unpack().maven().withFailure().executeTest(); - } - - public SurefireLauncher unpack() - { - return unpack( "fork-fail" ); - } - }