This is an automated email from the ASF dual-hosted git repository. tibordigana pushed a commit to branch it-946 in repository https://gitbox.apache.org/repos/asf/maven-surefire.git
commit d19f79ffb422011b225f2a44120c41d9a4a547ce Author: tibor.digana <[email protected]> AuthorDate: Tue Feb 22 23:49:06 2022 +0100 [SUREFIRE-2023] The integration test Surefire946KillMainProcessInReusableForkIT hanged and timed out because SIGTERM happened before the first test has started. The plugin should be able to terminate itself whenever after SIGTERM. --- .../plugin/surefire/AbstractSurefireMojo.java | 50 +++-- .../plugin/surefire/InPluginVMSurefireStarter.java | 9 +- .../plugin/surefire/booterclient/ForkStarter.java | 201 +++++++++------------ .../plugin/surefire/booterclient/Platform.java | 17 ++ .../surefire/booterclient/output/ForkClient.java | 45 ++--- .../booterclient/output/ForkClientTest.java | 11 +- .../api/util/internal/ConcurrencyUtils.java | 13 +- .../api/util/internal/ConcurrencyUtilsTest.java | 41 +++-- ...Surefire946KillMainProcessInReusableForkIT.java | 2 +- .../maven/surefire/common/junit4/Notifier.java | 8 +- .../maven/surefire/testng/TestNGExecutor.java | 18 +- 11 files changed, 203 insertions(+), 212 deletions(-) diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java index a2d76c8..89f13aa 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java @@ -154,6 +154,8 @@ import static org.apache.maven.surefire.shared.utils.StringUtils.isEmpty; import static org.apache.maven.surefire.shared.utils.StringUtils.isNotBlank; import static org.apache.maven.surefire.shared.utils.StringUtils.isNotEmpty; import static org.apache.maven.surefire.shared.utils.StringUtils.split; +import static org.apache.maven.surefire.shared.utils.cli.ShutdownHookUtils.addShutDownHook; +import static org.apache.maven.surefire.shared.utils.cli.ShutdownHookUtils.removeShutdownHook; /** * Abstract base class for running tests using Surefire. @@ -922,29 +924,39 @@ public abstract class AbstractSurefireMojo // Stuff that should have been final setupStuff(); Platform platform = PLATFORM.withJdkExecAttributesForTests( getEffectiveJvm() ); - - if ( verifyParameters() && !hasExecutedBefore() ) + Thread shutdownThread = new Thread( platform::setShutdownState ); + addShutDownHook( shutdownThread ); + try { - DefaultScanResult scan = scanForTestClasses(); - if ( !hasSuiteXmlFiles() && scan.isEmpty() ) + if ( verifyParameters() && !hasExecutedBefore() ) { - switch ( getEffectiveFailIfNoTests() ) + DefaultScanResult scan = scanForTestClasses(); + if ( !hasSuiteXmlFiles() && scan.isEmpty() ) { - case COULD_NOT_RUN_DEFAULT_TESTS: - throw new MojoFailureException( - "No tests were executed! (Set -DfailIfNoTests=false to ignore this error.)" ); - case COULD_NOT_RUN_SPECIFIED_TESTS: - throw new MojoFailureException( "No tests matching pattern \"" - + getSpecificTests().toString() - + "\" were executed! (Set " - + "-D" + getPluginName() + ".failIfNoSpecifiedTests=false to ignore this error.)" ); - default: - handleSummary( noTestsRun(), null ); - return; + switch ( getEffectiveFailIfNoTests() ) + { + case COULD_NOT_RUN_DEFAULT_TESTS: + throw new MojoFailureException( + "No tests were executed! (Set -DfailIfNoTests=false to ignore this error.)" ); + case COULD_NOT_RUN_SPECIFIED_TESTS: + throw new MojoFailureException( "No tests matching pattern \"" + + getSpecificTests().toString() + + "\" were executed! (Set " + + "-D" + getPluginName() + + ".failIfNoSpecifiedTests=false to ignore this error.)" ); + default: + handleSummary( noTestsRun(), null ); + return; + } } + logReportsDirectory(); + executeAfterPreconditionsChecked( scan, platform ); } - logReportsDirectory(); - executeAfterPreconditionsChecked( scan, platform ); + } + finally + { + platform.clearShutdownState(); + removeShutdownHook( shutdownThread ); } } @@ -2446,7 +2458,7 @@ public abstract class AbstractSurefireMojo StartupReportConfiguration startupReportConfiguration = getStartupReportConfiguration( configChecksum, false ); ProviderConfiguration providerConfiguration = createProviderConfiguration( runOrderParameters ); return new InPluginVMSurefireStarter( startupConfiguration, providerConfiguration, startupReportConfiguration, - getConsoleLogger() ); + getConsoleLogger(), platform ); } // todo this is in separate method and can be better tested than whole method createForkConfiguration() diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/InPluginVMSurefireStarter.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/InPluginVMSurefireStarter.java index 2e54e0d..78bc920 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/InPluginVMSurefireStarter.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/InPluginVMSurefireStarter.java @@ -19,6 +19,7 @@ package org.apache.maven.plugin.surefire; * under the License. */ +import org.apache.maven.plugin.surefire.booterclient.Platform; import org.apache.maven.plugin.surefire.log.api.ConsoleLogger; import org.apache.maven.surefire.booter.ClasspathConfiguration; import org.apache.maven.surefire.booter.ProviderConfiguration; @@ -52,16 +53,18 @@ public class InPluginVMSurefireStarter private final StartupReportConfiguration startupReportConfig; private final ProviderConfiguration providerConfig; private final ConsoleLogger consoleLogger; + private final Platform platform; public InPluginVMSurefireStarter( @Nonnull StartupConfiguration startupConfig, @Nonnull ProviderConfiguration providerConfig, @Nonnull StartupReportConfiguration startupReportConfig, - @Nonnull ConsoleLogger consoleLogger ) + @Nonnull ConsoleLogger consoleLogger, @Nonnull Platform platform ) { this.startupConfig = startupConfig; this.startupReportConfig = startupReportConfig; this.providerConfig = providerConfig; this.consoleLogger = consoleLogger; + this.platform = platform; } public RunResult runSuitesInProcess( @Nonnull DefaultScanResult scanResult ) @@ -84,7 +87,9 @@ public class InPluginVMSurefireStarter try { - return invokeProvider( null, testClassLoader, factory, providerConfig, false, startupConfig, true ); + return platform.isShutdown() + ? new RunResult( 0, 0, 0, 0 ) + : invokeProvider( null, testClassLoader, factory, providerConfig, false, startupConfig, true ); } catch ( InvocationTargetException e ) { 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 2a2c820..17b0e54 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 @@ -61,7 +61,6 @@ import javax.annotation.Nonnull; import java.io.Closeable; import java.io.File; import java.io.IOException; -import java.util.ArrayList; import java.util.Collection; import java.util.Map; import java.util.Queue; @@ -88,6 +87,8 @@ import static java.util.UUID.randomUUID; import static java.util.concurrent.Executors.newScheduledThreadPool; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; +import static java.util.stream.Collectors.toList; +import static java.util.stream.StreamSupport.stream; import static org.apache.maven.plugin.surefire.AbstractSurefireMojo.createCopyAndReplaceForkNumPlaceholder; import static org.apache.maven.plugin.surefire.SurefireHelper.DUMP_FILE_PREFIX; import static org.apache.maven.plugin.surefire.SurefireHelper.replaceForkThreadsInPath; @@ -101,7 +102,7 @@ import static org.apache.maven.surefire.shared.utils.cli.ShutdownHookUtils.remov import static org.apache.maven.surefire.api.suite.RunResult.SUCCESS; import static org.apache.maven.surefire.api.suite.RunResult.failure; import static org.apache.maven.surefire.api.suite.RunResult.timeout; -import static org.apache.maven.surefire.api.util.internal.ConcurrencyUtils.countDownToZero; +import static org.apache.maven.surefire.api.util.internal.ConcurrencyUtils.runIfZeroCountDown; import static org.apache.maven.surefire.api.util.internal.DaemonThreadFactory.newDaemonThread; import static org.apache.maven.surefire.api.util.internal.DaemonThreadFactory.newDaemonThreadFactory; import static org.apache.maven.surefire.api.util.internal.StringUtils.NL; @@ -288,15 +289,15 @@ public class ForkStarter } } - private RunResult run( SurefireProperties effectiveSystemProperties, Map<String, String> providerProperties ) + private RunResult run( SurefireProperties effectiveSystemProps, Map<String, String> providerProperties ) throws SurefireBooterForkException { TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder(); - PropertiesWrapper props = new PropertiesWrapper( providerProperties ); TestLessInputStream stream = builder.build(); Thread shutdown = createImmediateShutdownHookThread( builder, providerConfiguration.getShutdown() ); ScheduledFuture<?> ping = triggerPingTimerForShutdown( builder ); int forkNumber = drawNumber(); + PropertiesWrapper props = new PropertiesWrapper( providerProperties ); try { addShutDownHook( shutdown ); @@ -304,8 +305,10 @@ public class ForkStarter new DefaultReporterFactory( startupReportConfiguration, log, forkNumber ); defaultReporterFactories.add( forkedReporterFactory ); ForkClient forkClient = new ForkClient( forkedReporterFactory, stream, forkNumber ); - return fork( null, props, forkClient, effectiveSystemProperties, forkNumber, stream, - forkConfiguration.getForkNodeFactory(), false ); + ForkNodeFactory node = forkConfiguration.getForkNodeFactory(); + return forkConfiguration.getPluginPlatform().isShutdown() + ? new RunResult( 0, 0, 0, 0 ) + : fork( null, props, forkClient, effectiveSystemProps, forkNumber, stream, node, false ); } finally { @@ -336,81 +339,68 @@ public class ForkStarter } @SuppressWarnings( "checkstyle:magicnumber" ) - private RunResult runSuitesForkOnceMultiple( final SurefireProperties effectiveSystemProperties, int forkCount ) + private RunResult runSuitesForkOnceMultiple( SurefireProperties effectiveSystemProps, int forkCount ) throws SurefireBooterForkException { - ThreadPoolExecutor executorService = new ThreadPoolExecutor( forkCount, forkCount, 60, SECONDS, - new ArrayBlockingQueue<Runnable>( forkCount ) ); - executorService.setThreadFactory( FORKED_JVM_DAEMON_THREAD_FACTORY ); + ThreadPoolExecutor executor + = new ThreadPoolExecutor( forkCount, forkCount, 60L, SECONDS, new ArrayBlockingQueue<>( forkCount ) ); + executor.setThreadFactory( FORKED_JVM_DAEMON_THREAD_FACTORY ); - final Queue<String> tests = new ConcurrentLinkedQueue<>(); + Queue<String> tests = new ConcurrentLinkedQueue<>(); for ( Class<?> clazz : getSuitesIterator() ) { tests.add( clazz.getName() ); } - final Queue<TestProvidingInputStream> testStreams = new ConcurrentLinkedQueue<>(); + Queue<TestProvidingInputStream> forks = new ConcurrentLinkedQueue<>(); for ( int forkNum = 0, total = min( forkCount, tests.size() ); forkNum < total; forkNum++ ) { - testStreams.add( new TestProvidingInputStream( tests ) ); + forks.add( new TestProvidingInputStream( tests ) ); } - ScheduledFuture<?> ping = triggerPingTimerForShutdown( testStreams ); - Thread shutdown = createShutdownHookThread( testStreams, providerConfiguration.getShutdown() ); + Thread shutdown = createShutdownHookThread( forks, providerConfiguration.getShutdown() ); + addShutDownHook( shutdown ); + ScheduledFuture<?> ping = triggerPingTimerForShutdown( forks ); try { - addShutDownHook( shutdown ); int failFastCount = providerConfiguration.getSkipAfterFailureCount(); - final AtomicInteger notifyStreamsToSkipTestsJustNow = new AtomicInteger( failFastCount ); - final Collection<Future<RunResult>> results = new ArrayList<>( forkCount ); - for ( final TestProvidingInputStream testProvidingInputStream : testStreams ) - { - Callable<RunResult> pf = new Callable<RunResult>() + AtomicInteger notifyForksToSkipTestsNow = new AtomicInteger( failFastCount ); + Collection<Future<RunResult>> results = + forks.stream() + .filter( fork -> !forkConfiguration.getPluginPlatform().isShutdown() ) + .map( fork -> (Callable<RunResult>) () -> { - @Override - public RunResult call() - throws Exception + int forkNumber = drawNumber(); + DefaultReporterFactory reporter = + new DefaultReporterFactory( startupReportConfiguration, log, forkNumber ); + defaultReporterFactories.add( reporter ); + ForkClient client = new ForkClient( reporter, fork, forkNumber ); + client.setStopOnNextTestListener( () -> + runIfZeroCountDown( () -> notifyStreamsToSkipTests( forks ), notifyForksToSkipTestsNow ) ); + Map<String, String> providerProperties = providerConfiguration.getProviderProperties(); + PropertiesWrapper keyValues = new PropertiesWrapper( providerProperties ); + ForkNodeFactory node = forkConfiguration.getForkNodeFactory(); + try { - int forkNumber = drawNumber(); - DefaultReporterFactory reporter = - new DefaultReporterFactory( startupReportConfiguration, log, forkNumber ); - defaultReporterFactories.add( reporter ); - ForkClient forkClient = new ForkClient( reporter, testProvidingInputStream, forkNumber ) - { - @Override - protected void stopOnNextTest() - { - if ( countDownToZero( notifyStreamsToSkipTestsJustNow ) ) - { - notifyStreamsToSkipTests( testStreams ); - } - } - }; - Map<String, String> providerProperties = providerConfiguration.getProviderProperties(); - try - { - return fork( null, new PropertiesWrapper( providerProperties ), forkClient, - effectiveSystemProperties, forkNumber, testProvidingInputStream, - forkConfiguration.getForkNodeFactory(), true ); - } - finally - { - returnNumber( forkNumber ); - } + return fork( null, keyValues, client, effectiveSystemProps, forkNumber, fork, node, true ); } - }; - results.add( executorService.submit( pf ) ); - } - return awaitResultsDone( results, executorService ); + finally + { + returnNumber( forkNumber ); + } + } ).map( executor::submit ) + .collect( toList() ); + + return awaitResultsDone( results, executor ); } finally { removeShutdownHook( shutdown ); ping.cancel( true ); - closeExecutor( executorService ); + closeExecutor( executor ); } } @@ -423,68 +413,57 @@ public class ForkStarter } @SuppressWarnings( "checkstyle:magicnumber" ) - private RunResult runSuitesForkPerTestSet( final SurefireProperties effectiveSystemProperties, int forkCount ) + private RunResult runSuitesForkPerTestSet( SurefireProperties effectiveSystemProps, int forkCount ) throws SurefireBooterForkException { - ArrayList<Future<RunResult>> results = new ArrayList<>( 500 ); - ThreadPoolExecutor executorService = - new ThreadPoolExecutor( forkCount, forkCount, 60, SECONDS, new LinkedBlockingQueue<Runnable>() ); - executorService.setThreadFactory( FORKED_JVM_DAEMON_THREAD_FACTORY ); - final TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder(); - ScheduledFuture<?> ping = triggerPingTimerForShutdown( builder ); + TestLessInputStreamBuilder builder = new TestLessInputStreamBuilder(); Thread shutdown = createCachableShutdownHookThread( builder, providerConfiguration.getShutdown() ); + ThreadPoolExecutor executor = + new ThreadPoolExecutor( forkCount, forkCount, 60, SECONDS, new LinkedBlockingQueue<>() ); + executor.setThreadFactory( FORKED_JVM_DAEMON_THREAD_FACTORY ); + ScheduledFuture<?> ping = triggerPingTimerForShutdown( builder ); try { addShutDownHook( shutdown ); int failFastCount = providerConfiguration.getSkipAfterFailureCount(); - final AtomicInteger notifyStreamsToSkipTestsJustNow = new AtomicInteger( failFastCount ); - for ( final Object testSet : getSuitesIterator() ) + AtomicInteger notifyForksToSkipTestsNow = new AtomicInteger( failFastCount ); + Collection<Future<RunResult>> results = + stream( ( (Iterable<?>) getSuitesIterator() ).spliterator(), false ) + .filter( fork -> !forkConfiguration.getPluginPlatform().isShutdown() ) + .map( testSet -> (Callable<RunResult>) () -> { - Callable<RunResult> pf = new Callable<RunResult>() + int forkNumber = drawNumber(); + DefaultReporterFactory forkedReporterFactory = + new DefaultReporterFactory( startupReportConfiguration, log, forkNumber ); + defaultReporterFactories.add( forkedReporterFactory ); + TestLessInputStream stream = builder.build(); + ForkClient forkClient = new ForkClient( forkedReporterFactory, stream, forkNumber ); + NotifiableTestStream notifiable = builder.getCachableCommands(); + forkClient.setStopOnNextTestListener( () -> + runIfZeroCountDown( notifiable::skipSinceNextTest, notifyForksToSkipTestsNow ) ); + Map<String, String> providerProperties = providerConfiguration.getProviderProperties(); + PropertiesWrapper keyValues = new PropertiesWrapper( providerProperties ); + ForkNodeFactory node = forkConfiguration.getForkNodeFactory(); + try { - @Override - public RunResult call() - throws Exception - { - int forkNumber = drawNumber(); - DefaultReporterFactory forkedReporterFactory = - new DefaultReporterFactory( startupReportConfiguration, log, forkNumber ); - defaultReporterFactories.add( forkedReporterFactory ); - TestLessInputStream stream = builder.build(); - ForkClient forkClient = new ForkClient( forkedReporterFactory, stream, forkNumber ) - { - @Override - protected void stopOnNextTest() - { - if ( countDownToZero( notifyStreamsToSkipTestsJustNow ) ) - { - builder.getCachableCommands().skipSinceNextTest(); - } - } - }; - try - { - return fork( testSet, - new PropertiesWrapper( providerConfiguration.getProviderProperties() ), - forkClient, effectiveSystemProperties, forkNumber, stream, - forkConfiguration.getForkNodeFactory(), false ); - } - finally - { - returnNumber( forkNumber ); - builder.removeStream( stream ); - } - } - }; - results.add( executorService.submit( pf ) ); - } - return awaitResultsDone( results, executorService ); + return fork( testSet, keyValues, forkClient, effectiveSystemProps, forkNumber, stream, node, + false ); + } + finally + { + returnNumber( forkNumber ); + builder.removeStream( stream ); + } + } ).map( executor::submit ) + .collect( toList() ); + + return awaitResultsDone( results, executor ); } finally { removeShutdownHook( shutdown ); ping.cancel( true ); - closeExecutor( executorService ); + closeExecutor( executor ); } } @@ -662,7 +641,10 @@ public class ForkStarter log.error( "Closing the streams after (InterruptedException) '" + e.getLocalizedMessage() + "'" ); // maybe implement it in the Future.cancel() of the extension or similar forkChannel.disable(); - err.disable(); + if ( err != null ) + { + err.disable(); + } } catch ( Exception e ) { @@ -678,16 +660,7 @@ public class ForkStarter log.debug( "Closing the fork " + forkNumber + " after " + ( forkClient.isSaidGoodBye() ? "saying GoodBye." : "not saying Good Bye." ) ); currentForkClients.remove( forkClient ); - try - { - Closeable c = forkClient.isSaidGoodBye() ? closer : commandReader; - c.close(); - } - catch ( IOException e ) - { - InPluginProcessDumpSingleton.getSingleton() - .dumpException( e, e.getLocalizedMessage(), dumpLogDir, forkNumber ); - } + closer.close(); if ( runResult == null ) { diff --git a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/Platform.java b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/Platform.java index 97a3121..852b274 100644 --- a/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/Platform.java +++ b/maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/booterclient/Platform.java @@ -40,6 +40,8 @@ public final class Platform private final JdkAttributes jdk; + private volatile boolean shutdown; + public Platform() { // the job may take 50 or 80 ms @@ -53,6 +55,21 @@ public final class Platform this.jdk = jdk; } + public boolean isShutdown() + { + return shutdown; + } + + public void setShutdownState() + { + this.shutdown = true; + } + + public void clearShutdownState() + { + this.shutdown = false; + } + public Long getPluginPid() { try 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 c8eb672..37e50f5 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 @@ -56,7 +56,7 @@ import static org.apache.maven.surefire.api.report.CategorizedReportEntry.report * * @author Kristian Rosenvold */ -public class ForkClient +public final class ForkClient implements EventHandler<Event> { private static final long START_TIME_ZERO = 0L; @@ -89,7 +89,8 @@ public class ForkClient private volatile StackTraceWriter errorInFork; - public ForkClient( DefaultReporterFactory defaultReporterFactory, NotifiableTestStream notifiableTestStream, + public ForkClient( DefaultReporterFactory defaultReporterFactory, + NotifiableTestStream notifiableTestStream, int forkNumber ) { this.defaultReporterFactory = defaultReporterFactory; @@ -110,12 +111,16 @@ public class ForkClient notifier.setAcquireNextTestListener( new AcquireNextTestListener() ); notifier.setConsoleErrorListener( new ErrorListener() ); notifier.setByeListener( new ByeListener() ); - notifier.setStopOnNextTestListener( new StopOnNextTestListener() ); notifier.setConsoleDebugListener( new DebugListener() ); notifier.setConsoleWarningListener( new WarningListener() ); notifier.setExitErrorEventListener( new ExitErrorEventListener() ); } + public void setStopOnNextTestListener( ForkedProcessEventListener listener ) + { + notifier.setStopOnNextTestListener( listener ); + } + private final class TestSetStartingListener implements ForkedProcessReportEventListener<TestSetReportEntry> { @@ -277,15 +282,6 @@ public class ForkClient } } - private final class StopOnNextTestListener implements ForkedProcessEventListener - { - @Override - public void handle() - { - stopOnNextTest(); - } - } - private final class DebugListener implements ForkedProcessStringEventListener { @Override @@ -316,13 +312,6 @@ public class ForkClient } } - /** - * Overridden by a subclass, see {@link org.apache.maven.plugin.surefire.booterclient.ForkStarter}. - */ - protected void stopOnNextTest() - { - } - public void kill() { if ( !saidGoodBye ) @@ -338,7 +327,7 @@ public class ForkClient * @param currentTimeMillis current time in millis seconds * @param forkedProcessTimeoutInSeconds timeout in seconds given by MOJO */ - public final void tryToTimeout( long currentTimeMillis, int forkedProcessTimeoutInSeconds ) + public void tryToTimeout( long currentTimeMillis, int forkedProcessTimeoutInSeconds ) { if ( forkedProcessTimeoutInSeconds > 0 ) { @@ -352,13 +341,13 @@ public class ForkClient } } - public final DefaultReporterFactory getDefaultReporterFactory() + public DefaultReporterFactory getDefaultReporterFactory() { return defaultReporterFactory; } @Override - public final void handleEvent( @Nonnull Event event ) + public void handleEvent( @Nonnull Event event ) { notifier.notifyEvent( event ); } @@ -373,7 +362,7 @@ public class ForkClient } } - public final boolean hadTimeout() + public boolean hadTimeout() { return testSetStartedAt.get() == START_TIME_NEGATIVE_TIMEOUT; } @@ -409,7 +398,7 @@ public class ForkClient .writeTestOutput( new TestOutputReportEntry( output, isStdout, newLine, /*todo*/ null, null ) ); } - public final Map<String, String> getTestVmSystemProperties() + public Map<String, String> getTestVmSystemProperties() { return unmodifiableMap( testVmSystemProperties ); } @@ -420,7 +409,7 @@ public class ForkClient * * @return A mock provider reporter */ - public final RunListener getReporter() + public RunListener getReporter() { return getTestSetReporter(); } @@ -440,17 +429,17 @@ public class ForkClient // no op } - public final boolean isSaidGoodBye() + public boolean isSaidGoodBye() { return saidGoodBye; } - public final StackTraceWriter getErrorInFork() + public StackTraceWriter getErrorInFork() { return errorInFork; } - public final boolean isErrorInFork() + public boolean isErrorInFork() { return errorInFork != null; } diff --git a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClientTest.java b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClientTest.java index 7822300..c440114 100644 --- a/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClientTest.java +++ b/maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/booterclient/output/ForkClientTest.java @@ -438,15 +438,8 @@ public class ForkClientTest .thenReturn( new File( target, "surefire-reports" ) ); NotifiableTestStream notifiableTestStream = mock( NotifiableTestStream.class ); final boolean[] verified = {false}; - ForkClient client = new ForkClient( factory, notifiableTestStream, 0 ) - { - @Override - protected void stopOnNextTest() - { - super.stopOnNextTest(); - verified[0] = true; - } - }; + ForkClient client = new ForkClient( factory, notifiableTestStream, 0 ); + client.setStopOnNextTestListener( () -> verified[0] = true ); client.handleEvent( new ControlStopOnNextTestEvent() ); verifyZeroInteractions( notifiableTestStream ); verifyZeroInteractions( factory ); diff --git a/surefire-api/src/main/java/org/apache/maven/surefire/api/util/internal/ConcurrencyUtils.java b/surefire-api/src/main/java/org/apache/maven/surefire/api/util/internal/ConcurrencyUtils.java index a43d8f3..fd2c189 100644 --- a/surefire-api/src/main/java/org/apache/maven/surefire/api/util/internal/ConcurrencyUtils.java +++ b/surefire-api/src/main/java/org/apache/maven/surefire/api/util/internal/ConcurrencyUtils.java @@ -38,11 +38,11 @@ public final class ConcurrencyUtils * Decreases {@code counter} to zero, or does not change the counter if negative. * This method pretends been atomic. Only one thread can succeed setting the counter to zero. * + * @param runner run if this Thread has concurrently decremented the counter down to zero * @param counter atomic counter - * @return {@code true} if this Thread modified concurrent counter from any positive number down to zero. */ @SuppressWarnings( "checkstyle:emptyforiteratorpad" ) - public static boolean countDownToZero( AtomicInteger counter ) + public static void runIfZeroCountDown( Runnable runner, AtomicInteger counter ) { for (;;) { @@ -52,12 +52,17 @@ public final class ConcurrencyUtils int newCounter = c - 1; if ( counter.compareAndSet( c, newCounter ) ) { - return newCounter == 0; + boolean isZero = newCounter == 0; + if ( isZero ) + { + runner.run(); + } + break; } } else { - return false; + break; } } } diff --git a/surefire-api/src/test/java/org/apache/maven/surefire/api/util/internal/ConcurrencyUtilsTest.java b/surefire-api/src/test/java/org/apache/maven/surefire/api/util/internal/ConcurrencyUtilsTest.java index a2181c1..10c03e3 100644 --- a/surefire-api/src/test/java/org/apache/maven/surefire/api/util/internal/ConcurrencyUtilsTest.java +++ b/surefire-api/src/test/java/org/apache/maven/surefire/api/util/internal/ConcurrencyUtilsTest.java @@ -21,12 +21,12 @@ package org.apache.maven.surefire.api.util.internal; import org.junit.Test; -import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.FutureTask; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import static org.apache.maven.surefire.api.util.internal.ConcurrencyUtils.countDownToZero; +import static org.apache.maven.surefire.api.util.internal.ConcurrencyUtils.runIfZeroCountDown; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertTrue; @@ -45,7 +45,9 @@ public class ConcurrencyUtilsTest public void countDownShouldBeUnchangedAsZeroNegativeTest() { AtomicInteger atomicCounter = new AtomicInteger( 0 ); - assertFalse( countDownToZero( atomicCounter ) ); + AtomicBoolean runner = new AtomicBoolean(); + runIfZeroCountDown( () -> runner.set( true ), atomicCounter ); + assertFalse( runner.get() ); assertThat( atomicCounter.get(), is( 0 ) ); } @@ -53,7 +55,9 @@ public class ConcurrencyUtilsTest public void countDownShouldBeUnchangedAsNegativeNegativeTest() { AtomicInteger atomicCounter = new AtomicInteger( -1 ); - assertFalse( countDownToZero( atomicCounter ) ); + AtomicBoolean runner = new AtomicBoolean(); + runIfZeroCountDown( () -> runner.set( true ), atomicCounter ); + assertFalse( runner.get() ); assertThat( atomicCounter.get(), is( -1 ) ); } @@ -61,7 +65,9 @@ public class ConcurrencyUtilsTest public void countDownShouldBeDecreasedByOneThreadModification() { AtomicInteger atomicCounter = new AtomicInteger( 10 ); - assertFalse( countDownToZero( atomicCounter ) ); + AtomicBoolean runner = new AtomicBoolean(); + runIfZeroCountDown( () -> runner.set( true ), atomicCounter ); + assertFalse( runner.get() ); assertThat( atomicCounter.get(), is( 9 ) ); } @@ -69,7 +75,9 @@ public class ConcurrencyUtilsTest public void countDownToZeroShouldBeDecreasedByOneThreadModification() { AtomicInteger atomicCounter = new AtomicInteger( 1 ); - assertTrue( countDownToZero( atomicCounter ) ); + AtomicBoolean runner = new AtomicBoolean(); + runIfZeroCountDown( () -> runner.set( true ), atomicCounter ); + assertTrue( runner.get() ); assertThat( atomicCounter.get(), is( 0 ) ); } @@ -77,27 +85,28 @@ public class ConcurrencyUtilsTest public void countDownShouldBeDecreasedByTwoThreadsModification() throws ExecutionException, InterruptedException { - final AtomicInteger atomicCounter = new AtomicInteger( 3 ); + AtomicInteger atomicCounter = new AtomicInteger( 3 ); - FutureTask<Boolean> task = new FutureTask<Boolean>( new Callable<Boolean>() + FutureTask<Boolean> task = new FutureTask<>( () -> { - @Override - public Boolean call() - throws Exception - { - return countDownToZero( atomicCounter ); - } + AtomicBoolean runner = new AtomicBoolean(); + runIfZeroCountDown( () -> runner.set( true ), atomicCounter ); + return runner.get(); } ); Thread t = new Thread( task ); t.start(); - assertFalse( countDownToZero( atomicCounter ) ); + AtomicBoolean runner = new AtomicBoolean(); + runIfZeroCountDown( () -> runner.set( true ), atomicCounter ); + assertFalse( runner.get() ); assertFalse( task.get() ); assertThat( atomicCounter.get(), is( 1 ) ); - assertTrue( countDownToZero( atomicCounter ) ); + runner.set( false ); + runIfZeroCountDown( () -> runner.set( true ), atomicCounter ); + assertTrue( runner.get() ); assertThat( atomicCounter.get(), is( 0 ) ); } diff --git a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire946KillMainProcessInReusableForkIT.java b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire946KillMainProcessInReusableForkIT.java index 730786d..cdd3236 100644 --- a/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire946KillMainProcessInReusableForkIT.java +++ b/surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire946KillMainProcessInReusableForkIT.java @@ -95,7 +95,7 @@ public class Surefire946KillMainProcessInReusableForkIT "-" + shutdownMavenMethod + "-" + shutdownSurefireMethod ) .sysProp( "distinct.classifier", classifierOfDummyDependency ) .sysProp( "surefire.shutdown", shutdownSurefireMethod ) - .sysProp( "selfdestruct.timeoutInMillis", "10000" ) + .sysProp( "selfdestruct.timeoutInMillis", "20000" ) .sysProp( "selfdestruct.method", shutdownMavenMethod ) .sysProp( "testSleepTime", String.valueOf( TEST_SLEEP_TIME ) ) .addGoal( "org.apache.maven.plugins.surefire:maven-selfdestruct-plugin:selfdestruct" ) diff --git a/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/Notifier.java b/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/Notifier.java index ab70395..927df00 100644 --- a/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/Notifier.java +++ b/surefire-providers/common-junit4/src/main/java/org/apache/maven/surefire/common/junit4/Notifier.java @@ -33,7 +33,7 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; import static org.apache.maven.surefire.common.junit4.JUnit4ProviderUtil.toClassMethod; -import static org.apache.maven.surefire.api.util.internal.ConcurrencyUtils.countDownToZero; +import static org.apache.maven.surefire.api.util.internal.ConcurrencyUtils.runIfZeroCountDown; /** * Extends {@link RunNotifier JUnit notifier}, @@ -172,11 +172,7 @@ public class Notifier */ private void fireStopEvent() { - if ( countDownToZero( skipAfterFailureCount ) ) - { - pleaseStop(); - } - + runIfZeroCountDown( this::pleaseStop, skipAfterFailureCount ); reporter.testExecutionSkippedByUser(); } } diff --git a/surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java b/surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java index c0799f7..54fea3d 100644 --- a/surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java +++ b/surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java @@ -57,7 +57,7 @@ import static org.apache.maven.surefire.api.util.ReflectionUtils.newInstance; import static org.apache.maven.surefire.api.util.ReflectionUtils.tryGetConstructor; import static org.apache.maven.surefire.api.util.ReflectionUtils.tryGetMethod; import static org.apache.maven.surefire.api.util.ReflectionUtils.tryLoadClass; -import static org.apache.maven.surefire.api.util.internal.ConcurrencyUtils.countDownToZero; +import static org.apache.maven.surefire.api.util.internal.ConcurrencyUtils.runIfZeroCountDown; /** * Contains utility methods for executing TestNG. @@ -137,7 +137,7 @@ final class TestNGExecutor xmlTest.setName( metadata.testName ); addSelector( xmlTest, groupMatchingSelector ); addSelector( xmlTest, methodNameFilteringSelector ); - xmlTest.setXmlClasses( new ArrayList<XmlClass>() ); + xmlTest.setXmlClasses( new ArrayList<>() ); suiteAndNamedTests.testNameToTest.put( metadata.testName, xmlTest ); } @@ -366,18 +366,10 @@ final class TestNGExecutor { final AtomicInteger currentFaultCount = new AtomicInteger( skipAfterFailureCount ); - return new Stoppable() + return () -> { - @Override - public void fireStopEvent() - { - if ( countDownToZero( currentFaultCount ) ) - { - FailFastEventsSingleton.getInstance().setSkipOnNextTest(); - } - - reportManager.testExecutionSkippedByUser(); - } + runIfZeroCountDown( () -> FailFastEventsSingleton.getInstance().setSkipOnNextTest(), currentFaultCount ); + reportManager.testExecutionSkippedByUser(); }; }
