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
The following commit(s) were added to refs/heads/master by this push:
new 90d5665 fixed tests: dump files printed fake data due to the indexes
of ByteBuffer are not shifted for reads after IOException
90d5665 is described below
commit 90d5665eb01d53e361c90ec89a1711fda775b324
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 );
