This is an automated email from the ASF dual-hosted git repository. tibordigana pushed a commit to branch comm in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
commit 5c910839c743515bc68f62a1954e7ff7b05b0f3c Author: tibordigana <tibor.dig...@gmail.com> AuthorDate: Thu Jan 21 23:09:16 2021 +0100 fixed tests: dump files printed fake data due to the indexes of ByteBuffer are not shifted for reads after IOException --- .../surefire/extensions/EventConsumerThread.java | 3 +- .../apache/maven/surefire/stream/EventDecoder.java | 1 + .../extensions/ForkedProcessEventNotifierTest.java | 122 +++++++++++++++++++++ .../surefire/api/stream/AbstractStreamDecoder.java | 47 +++++--- .../maven/surefire/booter/CommandReader.java | 3 +- .../surefire/booter/stream/CommandDecoder.java | 1 + 6 files changed, 160 insertions(+), 17 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/EventConsumerThread.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/EventConsumerThread.java index 1371dbb..a2b1c02 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/EventConsumerThread.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/extensions/EventConsumerThread.java @@ -30,6 +30,7 @@ import org.apache.maven.surefire.stream.EventDecoder; import javax.annotation.Nonnull; import java.io.EOFException; import java.io.IOException; +import java.nio.channels.ClosedChannelException; import java.nio.channels.ReadableByteChannel; /** @@ -76,7 +77,7 @@ public class EventConsumerThread extends CloseableDaemonThread } while ( true ); } - catch ( EOFException e ) + catch ( EOFException | ClosedChannelException e ) { // } diff --git a/maven-surefire-common/src/main/java/org/apache/maven/surefire/stream/EventDecoder.java b/maven-surefire-common/src/main/java/org/apache/maven/surefire/stream/EventDecoder.java index 3698511..8170bac 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/surefire/stream/EventDecoder.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/surefire/stream/EventDecoder.java @@ -173,6 +173,7 @@ public class EventDecoder extends AbstractStreamDecoder<Event, ForkedProcessEven break; case END_OF_FRAME: memento.getLine().setPositionByteBuffer( memento.getByteBuffer().position() ); + memento.getLine().clear(); return toMessage( eventType, runMode, memento ); default: memento.getLine().setPositionByteBuffer( NO_POSITION ); diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/ForkedProcessEventNotifierTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/ForkedProcessEventNotifierTest.java index abbdeb0..9094392 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/ForkedProcessEventNotifierTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/extensions/ForkedProcessEventNotifierTest.java @@ -129,6 +129,12 @@ public class ForkedProcessEventNotifierTest notifier.notifyEvent( eventHandler.pullEvent() ); } } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -169,6 +175,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -209,6 +221,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.dumpStreamText ) @@ -263,6 +281,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -297,6 +321,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -327,6 +357,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -357,6 +393,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -389,6 +431,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -421,6 +469,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -453,6 +507,11 @@ public class ForkedProcessEventNotifierTest notifier.notifyEvent( eventHandler.pullEvent() ); } + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); @@ -489,6 +548,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -522,6 +587,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -555,6 +626,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -588,6 +665,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -621,6 +704,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -654,6 +743,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -687,6 +782,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -719,6 +820,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -805,6 +912,12 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); assertThat( arguments.isCalled() ) @@ -917,8 +1030,15 @@ public class ForkedProcessEventNotifierTest t.start(); notifier.notifyEvent( eventHandler.pullEvent() ); } + + assertThat( logger.error ).isEmpty(); + assertThat( logger.warning ).isEmpty(); + assertThat( logger.info ).isEmpty(); + assertThat( logger.debug ).isEmpty(); + assertThat( logger.isCalled() ) .isFalse(); + assertThat( arguments.isCalled() ) .isFalse(); } @@ -1209,6 +1329,7 @@ public class ForkedProcessEventNotifierTest { final ConcurrentLinkedQueue<String> debug = new ConcurrentLinkedQueue<>(); final ConcurrentLinkedQueue<String> info = new ConcurrentLinkedQueue<>(); + final ConcurrentLinkedQueue<String> warning = new ConcurrentLinkedQueue<>(); final ConcurrentLinkedQueue<String> error = new ConcurrentLinkedQueue<>(); final boolean isDebug; final boolean isInfo; @@ -1266,6 +1387,7 @@ public class ForkedProcessEventNotifierTest @Override public synchronized void warning( String message ) { + warning.add( message ); called = true; } diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamDecoder.java b/surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamDecoder.java index 862820c..ff69da8 100644 --- a/surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamDecoder.java +++ b/surefire-api/src/main/java/org/apache/maven/surefire/api/stream/AbstractStreamDecoder.java @@ -379,7 +379,7 @@ public abstract class AbstractStreamDecoder<M, MT extends Enum<MT>, ST extends E { printCorruptedStream( memento ); memento.getLine().printExistingLine(); - memento.getLine().setCount( 0 ); + memento.getLine().clear(); } /** @@ -456,26 +456,43 @@ public abstract class AbstractStreamDecoder<M, MT extends Enum<MT>, ST extends E int mark = buffer.position(); buffer.position( buffer.limit() ); buffer.limit( min( buffer.position() + recommendedCount, buffer.capacity() ) ); - boolean isEnd = false; - while ( !isEnd && buffer.position() - mark < recommendedCount && buffer.position() < buffer.limit() ) + return read( buffer, mark, recommendedCount ); + } + } + + private StreamReadStatus read( ByteBuffer buffer, int oldPosition, int recommendedCount ) + throws IOException + { + StreamReadStatus readStatus = null; + boolean isEnd = false; + try + { + while ( !isEnd && buffer.position() - oldPosition < recommendedCount && buffer.position() < buffer.limit() ) { isEnd = channel.read( buffer ) == -1; } - + } + finally + { buffer.limit( buffer.position() ); - buffer.position( mark ); + buffer.position( oldPosition ); int readBytes = buffer.remaining(); - - if ( isEnd && readBytes < recommendedCount ) - { - throw new EOFException(); - } - else + boolean readComplete = readBytes >= recommendedCount; + if ( !isEnd || readComplete ) { debugStream( buffer.array(), buffer.arrayOffset() + buffer.position(), buffer.remaining() ); - return readBytes >= recommendedCount ? OVERFLOW : UNDERFLOW; + readStatus = readComplete ? OVERFLOW : UNDERFLOW; } } + + if ( readStatus == null ) + { + throw new EOFException(); + } + else + { + return readStatus; + } } /** @@ -595,9 +612,9 @@ public abstract class AbstractStreamDecoder<M, MT extends Enum<MT>, ST extends E } } - public void setCount( int count ) + public void clear() { - this.count = count; + count = 0; } @Override @@ -627,7 +644,7 @@ public abstract class AbstractStreamDecoder<M, MT extends Enum<MT>, ST extends E } } - public void printExistingLine() + void printExistingLine() { if ( isEmpty() ) { diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/CommandReader.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/CommandReader.java index 6599433..5509095 100644 --- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/CommandReader.java +++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/CommandReader.java @@ -33,6 +33,7 @@ import org.apache.maven.surefire.api.testset.TestSetFailedException; import java.io.EOFException; import java.io.IOException; +import java.nio.channels.ClosedChannelException; import java.util.Iterator; import java.util.NoSuchElementException; import java.util.Queue; @@ -375,7 +376,7 @@ public final class CommandReader implements CommandChainReader } } } - catch ( EOFException e ) + catch ( EOFException | ClosedChannelException e ) { CommandReader.this.state.set( TERMINATED ); if ( !isTestSetFinished ) diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/stream/CommandDecoder.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/stream/CommandDecoder.java index b660147..e161be7 100644 --- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/stream/CommandDecoder.java +++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/stream/CommandDecoder.java @@ -119,6 +119,7 @@ public class CommandDecoder extends AbstractStreamDecoder<Command, MasterProcess break; case END_OF_FRAME: memento.getLine().setPositionByteBuffer( memento.getByteBuffer().position() ); + memento.getLine().clear(); return toMessage( commandType, runMode, memento ); default: memento.getLine().setPositionByteBuffer( NO_POSITION );