From e5e57cf39ff9802338569265deb9c290ad312925 Mon Sep 17 00:00:00 2001 From: "tibor.digana" Date: Thu, 24 Feb 2022 12:15:29 +0100 Subject: [PATCH] [SUREFIRE-2025] Updated abstractions which helps associating systemProperties() with a test context added Javadoc TestSetReportEntry already contains system properties and systemProperties() can be removed in MasterProcessChannelEncoder --- .../booterclient/output/ForkClient.java | 2 +- .../ForkedProcessEventNotifierTest.java | 89 +------------ .../api/booter/ForkingRunListener.java | 1 - .../booter/MasterProcessChannelEncoder.java | 125 ++++++++++++++++-- .../booter/spi/EventChannelEncoder.java | 9 +- .../booter/spi/EventChannelEncoderTest.java | 74 +++++------ 6 files changed, 165 insertions(+), 135 deletions(-) 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 37e50f5581..67b2ce7bab 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 @@ -72,7 +72,7 @@ public final class ForkClient /** * testSetStartedAt is set to non-zero after received - * {@link MasterProcessChannelEncoder#testSetStarting(ReportEntry, boolean)}. + * {@link MasterProcessChannelEncoder#testSetStarting(TestSetReportEntry, boolean)}. */ private final AtomicLong testSetStartedAt = new AtomicLong( START_TIME_ZERO ); 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 f9ad00c498..7aad5afc15 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 @@ -36,7 +36,7 @@ import org.apache.maven.surefire.api.report.RunMode; import org.apache.maven.surefire.api.report.SafeThrowable; import org.apache.maven.surefire.api.report.StackTraceWriter; -import org.apache.maven.surefire.api.util.internal.ObjectUtils; +import org.apache.maven.surefire.api.report.TestSetReportEntry; import org.apache.maven.surefire.api.util.internal.WritableBufferedByteChannel; import org.apache.maven.surefire.booter.spi.EventChannelEncoder; import org.apache.maven.surefire.extensions.EventHandler; @@ -79,6 +79,7 @@ import static org.apache.maven.surefire.api.report.TestOutputReportEntry.stdOutln; import static org.apache.maven.surefire.api.util.internal.Channels.newBufferedChannel; import static org.apache.maven.surefire.api.util.internal.Channels.newChannel; +import static org.apache.maven.surefire.api.util.internal.ObjectUtils.systemProps; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.rules.ExpectedException.none; @@ -104,48 +105,6 @@ public static class DecoderOperationsTest @Rule public final ExpectedException rule = none(); - @Test - public void shouldHaveSystemProperty() throws Exception - { - final Stream out = Stream.newStream(); - WritableBufferedByteChannel wChannel = newBufferedChannel( out ); - EventChannelEncoder encoder = new EventChannelEncoder( wChannel ); - Map props = ObjectUtils.systemProps(); - encoder.systemProperties( props ); - wChannel.close(); - - ForkedProcessEventNotifier notifier = new ForkedProcessEventNotifier(); - PropertyEventAssertionListener listener = new PropertyEventAssertionListener(); - notifier.setSystemPropertiesListener( listener ); - - ReadableByteChannel channel = newChannel( new ByteArrayInputStream( out.toByteArray() ) ); - - EH eventHandler = new EH(); - CountdownCloseable countdown = new CountdownCloseable( mock( Closeable.class ), 0 ); - ConsoleLoggerMock logger = new ConsoleLoggerMock( false, false, false, false ); - ForkNodeArgumentsMock arguments = new ForkNodeArgumentsMock( logger, new File( "" ) ); - try ( EventConsumerThread t = new EventConsumerThread( "t", channel, eventHandler, countdown, arguments ) ) - { - t.start(); - for ( int i = 0; i < props.size(); i++ ) - { - 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(); - assertThat( listener.counter.get() ) - .isEqualTo( props.size() ); - } - @Test public void shouldSendByeEvent() throws Exception { @@ -799,44 +758,6 @@ public void testStdErrStream() throws Exception .isTrue(); } - @Test - public void shouldCountSameNumberOfSystemProperties() throws Exception - { - final Stream out = Stream.newStream(); - WritableBufferedByteChannel wChannel = newBufferedChannel( out ); - EventChannelEncoder encoder = new EventChannelEncoder( wChannel ); - encoder.systemProperties( ObjectUtils.systemProps() ); - wChannel.close(); - - ReadableByteChannel channel = newChannel( new ByteArrayInputStream( out.toByteArray() ) ); - - ForkedProcessEventNotifier notifier = new ForkedProcessEventNotifier(); - PropertyEventAssertionListener listener = new PropertyEventAssertionListener(); - notifier.setSystemPropertiesListener( listener ); - - EH eventHandler = new EH(); - CountdownCloseable countdown = new CountdownCloseable( mock( Closeable.class ), 0 ); - ConsoleLoggerMock logger = new ConsoleLoggerMock( false, false, false, false ); - ForkNodeArgumentsMock arguments = new ForkNodeArgumentsMock( logger, new File( "" ) ); - try ( EventConsumerThread t = new EventConsumerThread( "t", channel, eventHandler, countdown, arguments ) ) - { - 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(); - assertThat( listener.called.get() ) - .isTrue(); - } - @Test public void shouldHandleErrorAfterNullLine() { @@ -999,7 +920,7 @@ public void testReportEntryOperations( @FromDataPoints( "operation" ) String[] o when( stackTraceWriter.writeTraceToString() ).thenReturn( exceptionStackTrace ); } - ReportEntry reportEntry = mock( ReportEntry.class ); + TestSetReportEntry reportEntry = mock( TestSetReportEntry.class ); when( reportEntry.getElapsed() ).thenReturn( elapsed ); when( reportEntry.getGroup() ).thenReturn( "this group" ); when( reportEntry.getMessage() ).thenReturn( reportedMessage ); @@ -1009,12 +930,14 @@ public void testReportEntryOperations( @FromDataPoints( "operation" ) String[] o when( reportEntry.getSourceName() ).thenReturn( "pkg.MyTest" ); when( reportEntry.getSourceText() ).thenReturn( "test class display name" ); when( reportEntry.getStackTraceWriter() ).thenReturn( stackTraceWriter ); + when( reportEntry.getSystemProperties() ).thenReturn( systemProps() ); final Stream out = Stream.newStream(); EventChannelEncoder encoder = new EventChannelEncoder( newBufferedChannel( out ) ); - EventChannelEncoder.class.getMethod( operation[0], ReportEntry.class, boolean.class ) + Class reportType = operation[0].startsWith( "testSet" ) ? TestSetReportEntry.class : ReportEntry.class; + EventChannelEncoder.class.getMethod( operation[0], reportType, boolean.class ) .invoke( encoder, reportEntry, trim ); ForkedProcessEventNotifier notifier = new ForkedProcessEventNotifier(); diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java b/surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java index a67643a491..70391ad147 100644 --- a/surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java +++ b/surefire-api/src/main/java/org/apache/maven/surefire/api/booter/ForkingRunListener.java @@ -68,7 +68,6 @@ public void testSetStarting( TestSetReportEntry report ) @Override public void testSetCompleted( TestSetReportEntry report ) { - target.systemProperties( report.getSystemProperties() ); target.testSetCompleted( report, trim ); } diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/api/booter/MasterProcessChannelEncoder.java b/surefire-api/src/main/java/org/apache/maven/surefire/api/booter/MasterProcessChannelEncoder.java index 231bb46c7b..50e8c96a05 100644 --- a/surefire-api/src/main/java/org/apache/maven/surefire/api/booter/MasterProcessChannelEncoder.java +++ b/surefire-api/src/main/java/org/apache/maven/surefire/api/booter/MasterProcessChannelEncoder.java @@ -22,8 +22,7 @@ import org.apache.maven.surefire.api.report.ReportEntry; import org.apache.maven.surefire.api.report.StackTraceWriter; import org.apache.maven.surefire.api.report.TestOutputReportEntry; - -import java.util.Map; +import org.apache.maven.surefire.api.report.TestSetReportEntry; /** * An abstraction for physical encoder of events. @@ -33,49 +32,159 @@ */ public interface MasterProcessChannelEncoder { + /** + * @return {@code true} if the encoder's stream has got an error + */ boolean checkError(); + /** + * Called on JVM exit error. + */ void onJvmExit(); - void systemProperties( Map sysProps ); - - void testSetStarting( ReportEntry reportEntry, boolean trimStackTraces ); - - void testSetCompleted( ReportEntry reportEntry, boolean trimStackTraces ); - + /** + * The test set has started. + * + * @param reportEntry test set report entry + * @param trimStackTraces {@code true} if stack trace trimming + */ + void testSetStarting( TestSetReportEntry reportEntry, boolean trimStackTraces ); + + /** + * The test set has finished. + * + * @param reportEntry test set report entry + * @param trimStackTraces {@code true} if stack trace trimming + */ + void testSetCompleted( TestSetReportEntry reportEntry, boolean trimStackTraces ); + + /** + * The test has started. + * + * @param reportEntry test set report entry + * @param trimStackTraces {@code true} if stack trace trimming + */ void testStarting( ReportEntry reportEntry, boolean trimStackTraces ); + /** + * The test has succeeded. + * + * @param reportEntry test set report entry + * @param trimStackTraces {@code true} if stack trace trimming + */ void testSucceeded( ReportEntry reportEntry, boolean trimStackTraces ); + /** + * The test has failed. + * + * @param reportEntry test set report entry + * @param trimStackTraces {@code true} if stack trace trimming + */ void testFailed( ReportEntry reportEntry, boolean trimStackTraces ); + /** + * The test is skipped. + * + * @param reportEntry test set report entry + * @param trimStackTraces {@code true} if stack trace trimming + */ void testSkipped( ReportEntry reportEntry, boolean trimStackTraces ); + /** + * The test error. + * + * @param reportEntry test set report entry + * @param trimStackTraces {@code true} if stack trace trimming + */ void testError( ReportEntry reportEntry, boolean trimStackTraces ); + /** + * The test assumption failure. + * + * @param reportEntry test set report entry + * @param trimStackTraces {@code true} if stack trace trimming + */ void testAssumptionFailure( ReportEntry reportEntry, boolean trimStackTraces ); + /** + * Test output, a line or characters. + * + * @param reportEntry std/out or std/err context + */ void testOutput( TestOutputReportEntry reportEntry ); + /** + * Info log. + * + * @param msg message of info logger + */ void consoleInfoLog( String msg ); + /** + * Error log. + * + * @param msg message of error logger + */ void consoleErrorLog( String msg ); + /** + * Error log. + * + * @param t exception + */ void consoleErrorLog( Throwable t ); + /** + * Error log. + * + * @param msg additional error message + * @param t exception + */ void consoleErrorLog( String msg, Throwable t ); + /** + * Error log. + * + * @param stackTraceWriter printable stack trace + * @param trimStackTraces {@code true} if selected trimmed stack trace to print into encoder channel/stream + */ void consoleErrorLog( StackTraceWriter stackTraceWriter, boolean trimStackTraces ); + /** + * Debug log. + * + * @param msg message of debug logger + */ void consoleDebugLog( String msg ); + /** + * Warning log. + * + * @param msg message of warning logger + */ void consoleWarningLog( String msg ); + /** + * Say BYE on exit. + * ForkBooter will consequently wait for BYE_ACK command which finally drains the (std/in) sink channel. + */ void bye(); + /** + * The provider wants to stop the progress. + */ void stopOnNextTest(); + /** + * The provider acquires a new test set to run. + */ void acquireNextTest(); + /** + * ForkedBooter tear down has failed while waiting for BYE_ACK command. + * + * @param stackTraceWriter printable stack trace + * @param trimStackTraces {@code true} if selected trimmed stack trace to print into encoder channel/stream + */ void sendExitError( StackTraceWriter stackTraceWriter, boolean trimStackTraces ); } diff --git a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java index 3fd45737ec..b3fc7d77ff 100644 --- a/surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java +++ b/surefire-booter/src/main/java/org/apache/maven/surefire/booter/spi/EventChannelEncoder.java @@ -28,6 +28,7 @@ import org.apache.maven.surefire.api.report.SafeThrowable; import org.apache.maven.surefire.api.report.StackTraceWriter; import org.apache.maven.surefire.api.report.TestOutputReportEntry; +import org.apache.maven.surefire.api.report.TestSetReportEntry; import org.apache.maven.surefire.api.util.internal.WritableBufferedByteChannel; import org.apache.maven.surefire.booter.stream.EventEncoder; @@ -127,8 +128,7 @@ public void onJvmExit() write( ByteBuffer.wrap( new byte[] {'\n'} ), true ); } - @Override - public void systemProperties( Map sysProps ) + private void encodeSystemProperties( Map sysProps, RunMode rm, Long testRunId ) { CharsetEncoder encoder = newCharsetEncoder(); ByteBuffer result = null; @@ -150,14 +150,15 @@ public void systemProperties( Map sysProps ) } @Override - public void testSetStarting( ReportEntry reportEntry, boolean trimStackTraces ) + public void testSetStarting( TestSetReportEntry reportEntry, boolean trimStackTraces ) { encode( BOOTERCODE_TESTSET_STARTING, runMode, reportEntry, trimStackTraces, true ); } @Override - public void testSetCompleted( ReportEntry reportEntry, boolean trimStackTraces ) + public void testSetCompleted( TestSetReportEntry reportEntry, boolean trimStackTraces ) { + encodeSystemProperties( reportEntry.getSystemProperties(), null, null ); // todo in next commit encode( BOOTERCODE_TESTSET_COMPLETED, runMode, reportEntry, trimStackTraces, true ); } diff --git a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/spi/EventChannelEncoderTest.java b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/spi/EventChannelEncoderTest.java index 7699d8ceec..98c4b7d6a7 100644 --- a/surefire-booter/src/test/java/org/apache/maven/surefire/booter/spi/EventChannelEncoderTest.java +++ b/surefire-booter/src/test/java/org/apache/maven/surefire/booter/spi/EventChannelEncoderTest.java @@ -23,7 +23,7 @@ import org.apache.maven.surefire.api.report.ReportEntry; import org.apache.maven.surefire.api.report.SafeThrowable; import org.apache.maven.surefire.api.report.StackTraceWriter; -import org.apache.maven.surefire.api.util.internal.ObjectUtils; +import org.apache.maven.surefire.api.report.TestSetReportEntry; import org.apache.maven.surefire.api.util.internal.WritableBufferedByteChannel; import org.junit.Test; @@ -44,6 +44,7 @@ import static org.apache.maven.surefire.api.report.TestOutputReportEntry.stdOut; import static org.apache.maven.surefire.api.report.TestOutputReportEntry.stdOutln; import static org.apache.maven.surefire.api.util.internal.Channels.newBufferedChannel; +import static org.apache.maven.surefire.api.util.internal.ObjectUtils.systemProps; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -75,7 +76,7 @@ public void reportEntry() throws IOException when( stackTraceWriter.writeTrimmedTraceToString() ).thenReturn( trimmedStackTrace ); when( stackTraceWriter.writeTraceToString() ).thenReturn( stackTrace ); - ReportEntry reportEntry = mock( ReportEntry.class ); + TestSetReportEntry reportEntry = mock( TestSetReportEntry.class ); when( reportEntry.getElapsed() ).thenReturn( ELAPSED_TIME ); when( reportEntry.getGroup() ).thenReturn( "this group" ); when( reportEntry.getMessage() ).thenReturn( "skipped test" ); @@ -317,7 +318,9 @@ public void testSetCompleted() throws IOException when( stackTraceWriter.writeTrimmedTraceToString() ).thenReturn( trimmedStackTrace ); when( stackTraceWriter.writeTraceToString() ).thenReturn( stackTrace ); - ReportEntry reportEntry = mock( ReportEntry.class ); + Map props = systemProps(); + + TestSetReportEntry reportEntry = mock( TestSetReportEntry.class ); when( reportEntry.getElapsed() ).thenReturn( ELAPSED_TIME ); when( reportEntry.getGroup() ).thenReturn( "this group" ); when( reportEntry.getMessage() ).thenReturn( "skipped test" ); @@ -325,12 +328,41 @@ public void testSetCompleted() throws IOException when( reportEntry.getNameWithGroup() ).thenReturn( "name with group" ); when( reportEntry.getSourceName() ).thenReturn( "pkg.MyTest" ); when( reportEntry.getStackTraceWriter() ).thenReturn( stackTraceWriter ); + when( reportEntry.getSystemProperties() ).thenReturn( props ); Stream out = Stream.newStream(); EventChannelEncoder encoder = new EventChannelEncoder( newBufferedChannel( out ) ); encoder.testSetCompleted( reportEntry, false ); + ByteArrayOutputStream expectedFrame = new ByteArrayOutputStream(); + for ( Entry entry : props.entrySet() ) + { + expectedFrame.write( ":maven-surefire-event:".getBytes() ); + expectedFrame.write( 8 ); + expectedFrame.write( ":sys-prop:".getBytes() ); + expectedFrame.write( 10 ); + expectedFrame.write( ":normal-run:".getBytes() ); + expectedFrame.write( 5 ); + expectedFrame.write( ":UTF-8:".getBytes() ); + int[] k = toBytes( entry.getKey().length() ); + expectedFrame.write( k[0] ); + expectedFrame.write( k[1] ); + expectedFrame.write( k[2] ); + expectedFrame.write( k[3] ); + expectedFrame.write( ':' ); + expectedFrame.write( entry.getKey().getBytes( UTF_8 ) ); + expectedFrame.write( ':' ); + int[] v = toBytes( entry.getValue() == null ? 1 : entry.getValue().getBytes( UTF_8 ).length ); + expectedFrame.write( v[0] ); + expectedFrame.write( v[1] ); + expectedFrame.write( v[2] ); + expectedFrame.write( v[3] ); + expectedFrame.write( ':' ); + expectedFrame.write( ( entry.getValue() == null ? "\u0000" : entry.getValue() ).getBytes( UTF_8 ) ); + expectedFrame.write( ':' ); + } + expectedFrame.write( ":maven-surefire-event:".getBytes( UTF_8 ) ); expectedFrame.write( (byte) 17 ); expectedFrame.write( ":testset-completed:".getBytes( UTF_8 ) ); @@ -377,6 +409,7 @@ public void testSetCompleted() throws IOException expectedFrame.write( ':' ); expectedFrame.write( stackTrace.getBytes( UTF_8 ) ); expectedFrame.write( ':' ); + assertThat( out.toByteArray() ) .isEqualTo( expectedFrame.toByteArray() ); } @@ -1142,41 +1175,6 @@ public void testStdErrStreamLn() throws IOException .isEqualTo( expected ); } - @Test - @SuppressWarnings( "checkstyle:innerassignment" ) - public void shouldCountSameNumberOfSystemProperties() throws IOException - { - Stream stream = Stream.newStream(); - WritableBufferedByteChannel channel = newBufferedChannel( stream ); - EventChannelEncoder encoder = new EventChannelEncoder( channel ); - - Map sysProps = ObjectUtils.systemProps(); - encoder.systemProperties( sysProps ); - channel.close(); - - for ( Entry entry : sysProps.entrySet() ) - { - int[] k = toBytes( entry.getKey().length() ); - int[] v = toBytes( entry.getValue() == null ? 1 : entry.getValue().getBytes( UTF_8 ).length ); - ByteArrayOutputStream expectedFrame = new ByteArrayOutputStream(); - expectedFrame.write( ":maven-surefire-event:sys-prop:normal-run:UTF-8:".getBytes( UTF_8 ) ); - expectedFrame.write( k[0] ); - expectedFrame.write( k[1] ); - expectedFrame.write( k[2] ); - expectedFrame.write( k[3] ); - expectedFrame.write( ':' ); - expectedFrame.write( v[0] ); - expectedFrame.write( v[1] ); - expectedFrame.write( v[2] ); - expectedFrame.write( v[3] ); - expectedFrame.write( ':' ); - expectedFrame.write( ( entry.getValue() == null ? "\u0000" : entry.getValue() ).getBytes( UTF_8 ) ); - expectedFrame.write( ':' ); - assertThat( stream.toByteArray() ) - .contains( expectedFrame.toByteArray() ); - } - } - @Test public void shouldHandleExit() {