From 2e84800055c500ae82e08112f07c4eb9f5684372 Mon Sep 17 00:00:00 2001 From: Janusz Baginski Date: Fri, 14 Jun 2024 15:28:52 +0100 Subject: [PATCH 1/3] Uses separate ExecutorService to re-run tests --- .../main/java/com/shazam/fork/ForkRunner.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/fork-runner/src/main/java/com/shazam/fork/ForkRunner.java b/fork-runner/src/main/java/com/shazam/fork/ForkRunner.java index ac7b3f21..b826cb8a 100755 --- a/fork-runner/src/main/java/com/shazam/fork/ForkRunner.java +++ b/fork-runner/src/main/java/com/shazam/fork/ForkRunner.java @@ -34,6 +34,7 @@ import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; import static com.shazam.fork.Utils.namedExecutor; import static java.util.stream.Collectors.toList; @@ -66,15 +67,14 @@ public ForkRunner(PoolLoader poolLoader, } public boolean run() { - ExecutorService poolExecutor = null; try { Collection pools = poolLoader.loadPools(); - poolExecutor = namedExecutor(pools.size(), "PoolExecutor-%d"); Collection testCases = testClassLoader.loadTestSuite(); summaryGeneratorHook.registerHook(pools, testCases); - executeTests(poolExecutor, pools, testCases); + executeTests(pools, testCases); + AggregatedTestResult aggregatedTestResult = aggregator.aggregateTestResults(pools, testCases); if (!aggregatedTestResult.getFatalCrashedTests().isEmpty()) { @@ -83,7 +83,7 @@ public boolean run() { Collection fatalCrashedTestCases = findFatalCrashedTestCases(testCases, aggregatedTestResult.getFatalCrashedTests()); - executeTests(poolExecutor, pools, fatalCrashedTestCases); + executeTests(pools, fatalCrashedTestCases); aggregatedTestResult = aggregator.aggregateTestResults(pools, testCases); @@ -107,28 +107,33 @@ public boolean run() { } catch (Exception e) { logger.error("Error while Fork was executing", e); return false; - } finally { - if (poolExecutor != null) { - poolExecutor.shutdown(); - } } } - private void executeTests(ExecutorService poolExecutor, - Collection pools, - Collection testCases) throws InterruptedException { + private void executeTests( + Collection pools, + Collection testCases + ) throws InterruptedException { ProgressReporter progressReporter = progressReporterFactory.createProgressReporter(); progressReporter.start(); - CountDownLatch poolCountDownLatch = new CountDownLatch(pools.size()); + ExecutorService poolExecutor = namedExecutor(pools.size(), "PoolExecutor-%d"); + CountDownLatch poolCountDownLatch = new CountDownLatch(pools.size()); for (Pool pool : pools) { - Runnable poolTestRunner = - poolTestRunnerFactory.createPoolTestRunner(pool, testCases, poolCountDownLatch, progressReporter); + Runnable poolTestRunner = poolTestRunnerFactory.createPoolTestRunner( + pool, + testCases, + poolCountDownLatch, + progressReporter + ); poolExecutor.execute(poolTestRunner); } poolCountDownLatch.await(); + poolExecutor.shutdown(); + poolExecutor.awaitTermination(10, TimeUnit.MINUTES); + progressReporter.stop(); } From d08d644e0bb7e7fa46688113e982c31376dfdfa1 Mon Sep 17 00:00:00 2001 From: Janusz Baginski Date: Fri, 21 Jun 2024 15:00:33 +0100 Subject: [PATCH 2/3] Removes CountDownLatch --- .../main/java/com/shazam/fork/ForkRunner.java | 41 +++++++++++-------- .../shazam/fork/runner/DeviceTestRunner.java | 5 --- .../fork/runner/DeviceTestRunnerFactory.java | 3 -- .../shazam/fork/runner/PoolTestRunner.java | 29 +++++++------ .../fork/runner/PoolTestRunnerFactory.java | 3 -- .../java/com/shazam/fork/runner/TestRun.java | 3 +- 6 files changed, 40 insertions(+), 44 deletions(-) diff --git a/fork-runner/src/main/java/com/shazam/fork/ForkRunner.java b/fork-runner/src/main/java/com/shazam/fork/ForkRunner.java index b826cb8a..803be099 100755 --- a/fork-runner/src/main/java/com/shazam/fork/ForkRunner.java +++ b/fork-runner/src/main/java/com/shazam/fork/ForkRunner.java @@ -36,6 +36,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; +import static com.google.common.util.concurrent.Uninterruptibles.awaitTerminationUninterruptibly; import static com.shazam.fork.Utils.namedExecutor; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; @@ -113,28 +114,34 @@ public boolean run() { private void executeTests( Collection pools, Collection testCases - ) throws InterruptedException { + ) { ProgressReporter progressReporter = progressReporterFactory.createProgressReporter(); progressReporter.start(); - ExecutorService poolExecutor = namedExecutor(pools.size(), "PoolExecutor-%d"); - - CountDownLatch poolCountDownLatch = new CountDownLatch(pools.size()); - for (Pool pool : pools) { - Runnable poolTestRunner = poolTestRunnerFactory.createPoolTestRunner( - pool, - testCases, - poolCountDownLatch, - progressReporter - ); - poolExecutor.execute(poolTestRunner); - } - poolCountDownLatch.await(); + ExecutorService poolExecutor = null; + try { + poolExecutor = namedExecutor(pools.size(), "PoolExecutor-%d"); + + for (Pool pool : pools) { + Runnable poolTestRunner = poolTestRunnerFactory.createPoolTestRunner( + pool, + testCases, + progressReporter + ); + poolExecutor.submit(poolTestRunner); + } - poolExecutor.shutdown(); - poolExecutor.awaitTermination(10, TimeUnit.MINUTES); + poolExecutor.shutdown(); + awaitTerminationUninterruptibly(poolExecutor); + } finally { + progressReporter.stop(); + + if (poolExecutor != null && !poolExecutor.isTerminated()) { + poolExecutor.shutdownNow(); + awaitTerminationUninterruptibly(poolExecutor); + } + } - progressReporter.stop(); } private static void reportMissingTests(AggregatedTestResult aggregatedTestResult) { diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunner.java b/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunner.java index f4ca0ae6..2b53e852 100755 --- a/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunner.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunner.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.util.Queue; -import java.util.concurrent.CountDownLatch; import static com.shazam.fork.system.io.RemoteFileManager.*; @@ -34,7 +33,6 @@ public class DeviceTestRunner implements Runnable { private final Pool pool; private final Device device; private final Queue queueOfTestsInPool; - private final CountDownLatch deviceCountDownLatch; private final ProgressReporter progressReporter; private final ScreenRecorder screenRecorder; private final TestRunFactory testRunFactory; @@ -43,7 +41,6 @@ public DeviceTestRunner(Installer installer, Pool pool, Device device, Queue queueOfTestsInPool, - CountDownLatch deviceCountDownLatch, ProgressReporter progressReporter, ScreenRecorder screenRecorder, TestRunFactory testRunFactory) { @@ -51,7 +48,6 @@ public DeviceTestRunner(Installer installer, this.pool = pool; this.device = device; this.queueOfTestsInPool = queueOfTestsInPool; - this.deviceCountDownLatch = deviceCountDownLatch; this.progressReporter = progressReporter; this.screenRecorder = screenRecorder; this.testRunFactory = testRunFactory; @@ -81,7 +77,6 @@ public void run() { } } finally { logger.info("Device {} from pool {} finished", device.getSerial(), pool.getName()); - deviceCountDownLatch.countDown(); } } diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunnerFactory.java b/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunnerFactory.java index 78979812..e140bf47 100644 --- a/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunnerFactory.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunnerFactory.java @@ -17,7 +17,6 @@ import com.shazam.fork.system.adb.Installer; import java.util.Queue; -import java.util.concurrent.CountDownLatch; public class DeviceTestRunnerFactory { @@ -31,7 +30,6 @@ public DeviceTestRunnerFactory(Installer installer, TestRunFactory testRunFactor public Runnable createDeviceTestRunner(Pool pool, Queue testClassQueue, - CountDownLatch deviceInPoolCountDownLatch, Device device, ProgressReporter progressReporter ) { @@ -40,7 +38,6 @@ public Runnable createDeviceTestRunner(Pool pool, pool, device, testClassQueue, - deviceInPoolCountDownLatch, progressReporter, new ScreenRecorderImpl(device), testRunFactory); diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunner.java b/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunner.java index 36e95a6c..15a0880b 100755 --- a/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunner.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunner.java @@ -20,9 +20,9 @@ import org.slf4j.LoggerFactory; import java.util.Queue; -import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; +import static com.google.common.util.concurrent.Uninterruptibles.awaitTerminationUninterruptibly; import static com.shazam.fork.Utils.namedExecutor; public class PoolTestRunner implements Runnable { @@ -31,17 +31,14 @@ public class PoolTestRunner implements Runnable { private final Pool pool; private final Queue testCases; - private final CountDownLatch poolCountDownLatch; private final DeviceTestRunnerFactory deviceTestRunnerFactory; private final ProgressReporter progressReporter; public PoolTestRunner(DeviceTestRunnerFactory deviceTestRunnerFactory, Pool pool, Queue testCases, - CountDownLatch poolCountDownLatch, ProgressReporter progressReporter) { this.pool = pool; this.testCases = testCases; - this.poolCountDownLatch = poolCountDownLatch; this.deviceTestRunnerFactory = deviceTestRunnerFactory; this.progressReporter = progressReporter; } @@ -52,23 +49,25 @@ public void run() { try { int devicesInPool = pool.size(); concurrentDeviceExecutor = namedExecutor(devicesInPool, "DeviceExecutor-%d"); - CountDownLatch deviceCountDownLatch = new CountDownLatch(devicesInPool); logger.info("Pool {} started", poolName); for (Device device : pool.getDevices()) { - Runnable deviceTestRunner = deviceTestRunnerFactory.createDeviceTestRunner(pool, testCases, - deviceCountDownLatch, device, progressReporter); - concurrentDeviceExecutor.execute(deviceTestRunner); + Runnable deviceTestRunner = deviceTestRunnerFactory.createDeviceTestRunner( + pool, + testCases, + device, + progressReporter + ); + concurrentDeviceExecutor.submit(deviceTestRunner); } - deviceCountDownLatch.await(); - } catch (InterruptedException e) { - logger.warn("Pool {} was interrupted while running", poolName); + + concurrentDeviceExecutor.shutdown(); + awaitTerminationUninterruptibly(concurrentDeviceExecutor); } finally { - if (concurrentDeviceExecutor != null) { - concurrentDeviceExecutor.shutdown(); + if (concurrentDeviceExecutor != null && !concurrentDeviceExecutor.isTerminated()) { + concurrentDeviceExecutor.shutdownNow(); + awaitTerminationUninterruptibly(concurrentDeviceExecutor); } logger.info("Pool {} finished", poolName); - poolCountDownLatch.countDown(); - logger.info("Pools remaining: {}", poolCountDownLatch.getCount()); } } } diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunnerFactory.java b/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunnerFactory.java index 6d7dcf21..ca3e4b1b 100644 --- a/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunnerFactory.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunnerFactory.java @@ -15,7 +15,6 @@ import java.util.Collection; import java.util.LinkedList; -import java.util.concurrent.CountDownLatch; public class PoolTestRunnerFactory { private final DeviceTestRunnerFactory deviceTestRunnerFactory; @@ -26,7 +25,6 @@ public PoolTestRunnerFactory(DeviceTestRunnerFactory deviceTestRunnerFactory) { public Runnable createPoolTestRunner(Pool pool, Collection testCases, - CountDownLatch poolCountDownLatch, ProgressReporter progressReporter) { int totalTests = testCases.size(); @@ -36,7 +34,6 @@ public Runnable createPoolTestRunner(Pool pool, deviceTestRunnerFactory, pool, new LinkedList<>(testCases), - poolCountDownLatch, progressReporter); } } diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/TestRun.java b/fork-runner/src/main/java/com/shazam/fork/runner/TestRun.java index 1972ed16..fa9d0455 100755 --- a/fork-runner/src/main/java/com/shazam/fork/runner/TestRun.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/TestRun.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.List; +import java.util.concurrent.TimeUnit; import static java.lang.String.format; @@ -64,7 +65,7 @@ public void execute() { } runner.setRunName(poolName); runner.setMethodName(testClassName, testMethodName); - runner.setMaxtimeToOutputResponse(testRunParameters.getTestOutputTimeout()); + runner.setMaxTimeToOutputResponse(testRunParameters.getTestOutputTimeout(), TimeUnit.MILLISECONDS); if (testRunParameters.isCoverageEnabled()) { runner.setCoverage(true); From a68ba85f92f4e24756aa2d5667b0180caa71812f Mon Sep 17 00:00:00 2001 From: Janusz Baginski Date: Fri, 21 Jun 2024 15:04:09 +0100 Subject: [PATCH 3/3] Uses ConcurrentLinkedDeque as a thread safe queue --- .../main/java/com/shazam/fork/runner/DeviceTestRunner.java | 6 +++--- .../com/shazam/fork/runner/DeviceTestRunnerFactory.java | 4 ++-- .../main/java/com/shazam/fork/runner/PoolTestRunner.java | 6 +++--- .../java/com/shazam/fork/runner/PoolTestRunnerFactory.java | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunner.java b/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunner.java index 2b53e852..168ca649 100755 --- a/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunner.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunner.java @@ -22,7 +22,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; -import java.util.Queue; +import java.util.Deque; import static com.shazam.fork.system.io.RemoteFileManager.*; @@ -32,7 +32,7 @@ public class DeviceTestRunner implements Runnable { private final Installer installer; private final Pool pool; private final Device device; - private final Queue queueOfTestsInPool; + private final Deque queueOfTestsInPool; private final ProgressReporter progressReporter; private final ScreenRecorder screenRecorder; private final TestRunFactory testRunFactory; @@ -40,7 +40,7 @@ public class DeviceTestRunner implements Runnable { public DeviceTestRunner(Installer installer, Pool pool, Device device, - Queue queueOfTestsInPool, + Deque queueOfTestsInPool, ProgressReporter progressReporter, ScreenRecorder screenRecorder, TestRunFactory testRunFactory) { diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunnerFactory.java b/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunnerFactory.java index e140bf47..51996315 100644 --- a/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunnerFactory.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/DeviceTestRunnerFactory.java @@ -16,7 +16,7 @@ import com.shazam.fork.model.TestCaseEvent; import com.shazam.fork.system.adb.Installer; -import java.util.Queue; +import java.util.Deque; public class DeviceTestRunnerFactory { @@ -29,7 +29,7 @@ public DeviceTestRunnerFactory(Installer installer, TestRunFactory testRunFactor } public Runnable createDeviceTestRunner(Pool pool, - Queue testClassQueue, + Deque testClassQueue, Device device, ProgressReporter progressReporter ) { diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunner.java b/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunner.java index 15a0880b..1227f158 100755 --- a/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunner.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunner.java @@ -19,7 +19,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.Queue; +import java.util.Deque; import java.util.concurrent.ExecutorService; import static com.google.common.util.concurrent.Uninterruptibles.awaitTerminationUninterruptibly; @@ -30,12 +30,12 @@ public class PoolTestRunner implements Runnable { public static final String DROPPED_BY = "DroppedBy-"; private final Pool pool; - private final Queue testCases; + private final Deque testCases; private final DeviceTestRunnerFactory deviceTestRunnerFactory; private final ProgressReporter progressReporter; public PoolTestRunner(DeviceTestRunnerFactory deviceTestRunnerFactory, Pool pool, - Queue testCases, + Deque testCases, ProgressReporter progressReporter) { this.pool = pool; this.testCases = testCases; diff --git a/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunnerFactory.java b/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunnerFactory.java index ca3e4b1b..ed52be49 100644 --- a/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunnerFactory.java +++ b/fork-runner/src/main/java/com/shazam/fork/runner/PoolTestRunnerFactory.java @@ -14,7 +14,7 @@ import com.shazam.fork.model.TestCaseEvent; import java.util.Collection; -import java.util.LinkedList; +import java.util.concurrent.ConcurrentLinkedDeque; public class PoolTestRunnerFactory { private final DeviceTestRunnerFactory deviceTestRunnerFactory; @@ -33,7 +33,7 @@ public Runnable createPoolTestRunner(Pool pool, return new PoolTestRunner( deviceTestRunnerFactory, pool, - new LinkedList<>(testCases), + new ConcurrentLinkedDeque<>(testCases), progressReporter); } }