SLING-9200 - simplify needToWait(...), use more constants and minor cleanup
diff --git a/src/main/java/org/apache/sling/junit/impl/TestsManagerImpl.java b/src/main/java/org/apache/sling/junit/impl/TestsManagerImpl.java
index ed7c6f2..f939c5f 100644
--- a/src/main/java/org/apache/sling/junit/impl/TestsManagerImpl.java
+++ b/src/main/java/org/apache/sling/junit/impl/TestsManagerImpl.java
@@ -27,6 +27,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
+import java.util.function.LongUnaryOperator;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Service;
@@ -54,14 +55,12 @@
private static final Logger log = LoggerFactory.getLogger(TestsManagerImpl.class);
// Global Timeout up to which it stop waiting for bundles to be all active, default to 40 seconds.
- public static final String PROPERTY_SYSTEM_STARTUP_GLOBAL_TIMEOUT_SECONDS = "sling.junit.core.system_startup_global_timeout";
+ public static final String PROP_STARTUP_TIMEOUT_SECONDS = "sling.junit.core.SystemStartupTimeoutSeconds";
- private static volatile int globalTimeoutSeconds = Integer.parseInt(System.getProperty(PROPERTY_SYSTEM_STARTUP_GLOBAL_TIMEOUT_SECONDS, "40"));
+ private static volatile int startupTimeoutSeconds = Integer.parseInt(System.getProperty(PROP_STARTUP_TIMEOUT_SECONDS, "40"));
private static volatile boolean waitForSystemStartup = true;
- private static volatile long timeWaitForSystemStartup = 0;
-
private ServiceTracker tracker;
private int lastTrackingCount = -1;
@@ -235,7 +234,11 @@
}
- public static void waitForSystemStartup() {
+ /** Wait for all bundles to be started
+ * @return number of msec taken by this method to execute
+ */
+ public static long waitForSystemStartup() {
+ long elapsedMsec = -1;
if (waitForSystemStartup) {
waitForSystemStartup = false;
final BundleContext bundleContext = Activator.getBundleContext();
@@ -247,10 +250,10 @@
}
// wait max inactivityTimeout after the last bundle became active before giving up
- long startTime = System.currentTimeMillis();
- long globalTimeout = startTime + TimeUnit.SECONDS.toMillis(globalTimeoutSeconds);
- while (isWaitNeeded(globalTimeout, bundlesToWaitFor)) {
- log.info("Waiting for the following bundles to start: {}", bundlesToWaitFor);
+ final long startTime = System.currentTimeMillis();
+ final long startupTimeout = startTime + TimeUnit.SECONDS.toMillis(startupTimeoutSeconds);
+ while (needToWait(startupTimeout, bundlesToWaitFor)) {
+ log.info("Waiting for bundles to start: {}", bundlesToWaitFor);
try {
TimeUnit.SECONDS.sleep(1);
} catch (InterruptedException e) {
@@ -261,26 +264,27 @@
Bundle bundle = bundles.next();
if (bundle.getState() == Bundle.ACTIVE) {
bundles.remove();
- log.debug("Bundle {} has become active", bundle.getSymbolicName());
+ log.debug("Bundle {} is now active", bundle.getSymbolicName());
}
}
}
- timeWaitForSystemStartup = System.currentTimeMillis() - startTime;
+ elapsedMsec = System.currentTimeMillis() - startTime;
if (!bundlesToWaitFor.isEmpty()) {
log.warn("Waited {} milliseconds but the following bundles are not yet started: {}",
- timeWaitForSystemStartup, bundlesToWaitFor);
+ elapsedMsec, bundlesToWaitFor);
} else {
log.info("All bundles are active, starting to run tests.");
}
+
}
+
+ return elapsedMsec;
}
- private static boolean isWaitNeeded(final long globalTimeout, final Set<Bundle> bundlesToWaitFor) {
- long currentTime = System.currentTimeMillis();
- boolean globallyTimeout = globalTimeout < currentTime;
- return !globallyTimeout && !bundlesToWaitFor.isEmpty();
+ private static boolean needToWait(final long startupTimeout, final Collection<Bundle> bundlesToWaitFor) {
+ return startupTimeout > System.currentTimeMillis() && !bundlesToWaitFor.isEmpty();
}
private static boolean isFragment(final Bundle bundle) {
diff --git a/src/test/java/org/apache/sling/junit/impl/TestsManagerImplTest.java b/src/test/java/org/apache/sling/junit/impl/TestsManagerImplTest.java
index 3367ba1..8b394f4 100644
--- a/src/test/java/org/apache/sling/junit/impl/TestsManagerImplTest.java
+++ b/src/test/java/org/apache/sling/junit/impl/TestsManagerImplTest.java
@@ -21,7 +21,6 @@
import static org.powermock.api.mockito.PowerMockito.mock;
import static org.powermock.api.mockito.PowerMockito.when;
-import java.util.Dictionary;
import java.util.HashSet;
import java.util.Hashtable;
import java.util.Set;
@@ -44,66 +43,59 @@
@PrepareForTest({ Activator.class, TestsManagerImpl.class })
public class TestsManagerImplTest {
+ private static final String WAIT_METHOD_NAME = "needToWait";
+ private static final int SYSTEM_STARTUP_SECONDS = 2;
+
static {
- // Set the system properties for this test as the default would wait longer.
- System.setProperty(TestsManagerImpl.PROPERTY_SYSTEM_STARTUP_GLOBAL_TIMEOUT_SECONDS, "2");
+ // Set a short timeout so our tests can run faster
+ System.setProperty("sling.junit.core.SystemStartupTimeoutSeconds", String.valueOf(SYSTEM_STARTUP_SECONDS));
}
/**
- * case if isWaitNeeded should return true, mainly it still have some bundles in the list to wait, and global timeout didn't pass.
+ * case if needToWait should return true, mainly it still have some bundles in the list to wait, and global timeout didn't pass.
*/
@Test
- public void isWaitNeededPositiveNotEmptyListNotGloballyTimeout() throws Exception {
- final TestsManagerImpl testsManager = new TestsManagerImpl();
- long globalTimeout = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(10);
+ public void needToWaitPositiveNotEmptyListNotGloballyTimeout() throws Exception {
+ long startupTimeout = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(5 * SYSTEM_STARTUP_SECONDS);
final Set<Bundle> bundlesToWaitFor = new HashSet<Bundle>();
bundlesToWaitFor.add(Mockito.mock(Bundle.class));
- boolean isWaitNeeded = Whitebox.invokeMethod(TestsManagerImpl.class, "isWaitNeeded", globalTimeout, bundlesToWaitFor);
- assertTrue(isWaitNeeded);
+ assertTrue((Boolean)Whitebox.invokeMethod(TestsManagerImpl.class, WAIT_METHOD_NAME, startupTimeout, bundlesToWaitFor));
}
/**
- * case if isWaitNeeded should return false, when for example it reached the global timeout limit.
+ * case if needToWait should return false, when for example it reached the global timeout limit.
*/
@Test
- public void isWaitNeededNegativeForGlobalTimeout() throws Exception {
- final TestsManagerImpl testsManager = new TestsManagerImpl();
- long lastChange = System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(1);
- long globalTimeout = lastChange - TimeUnit.SECONDS.toMillis(1);
- final Set<Bundle> bundlesToWaitFor = new HashSet<Bundle>();
- boolean isWaitNeeded = Whitebox.invokeMethod(TestsManagerImpl.class, "isWaitNeeded", globalTimeout, bundlesToWaitFor);
- assertFalse(isWaitNeeded);
+ public void needToWaitNegativeForstartupTimeout() throws Exception {
+ long lastChange = System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(SYSTEM_STARTUP_SECONDS / 2);
+ long startupTimeout = lastChange - TimeUnit.SECONDS.toMillis(1);
+ assertFalse((Boolean)Whitebox.invokeMethod(TestsManagerImpl.class, WAIT_METHOD_NAME, startupTimeout, new HashSet<Bundle>()));
}
/**
- * case if isWaitNeeded should return false, when for example it reached the global timeout limit.
+ * case if needToWait should return false, when for example it reached the global timeout limit.
*/
@Test
- public void isWaitNeededNegativeForEmptyList() throws Exception {
- final TestsManagerImpl testsManager = new TestsManagerImpl();
- long lastChange = System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(1);
- long globalTimeout = lastChange + TimeUnit.SECONDS.toMillis(10);
- final Set<Bundle> bundlesToWaitFor = new HashSet<Bundle>();
- boolean isWaitNeeded = Whitebox.invokeMethod(TestsManagerImpl.class, "isWaitNeeded", globalTimeout, bundlesToWaitFor);
- assertFalse(isWaitNeeded);
+ public void needToWaitNegativeForEmptyList() throws Exception {
+ long lastChange = System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(SYSTEM_STARTUP_SECONDS / 2);
+ long startupTimeout = lastChange + TimeUnit.SECONDS.toMillis(10);
+ assertFalse((Boolean)Whitebox.invokeMethod(TestsManagerImpl.class, WAIT_METHOD_NAME, startupTimeout, new HashSet<Bundle>()));
}
@Test
public void waitForSystemStartupTimeout() {
setupBundleContextMock(Bundle.INSTALLED);
- TestsManagerImpl.waitForSystemStartup();
- long timeWaitForSystemStartup = Whitebox.getInternalState(TestsManagerImpl.class, "timeWaitForSystemStartup");
- assertTrue(timeWaitForSystemStartup > TimeUnit.SECONDS.toMillis(2));
- assertTrue(timeWaitForSystemStartup < TimeUnit.SECONDS.toMillis(3));
+ final long elapsed = TestsManagerImpl.waitForSystemStartup();
+ assertTrue(elapsed > TimeUnit.SECONDS.toMillis(SYSTEM_STARTUP_SECONDS));
+ assertTrue(elapsed < TimeUnit.SECONDS.toMillis(SYSTEM_STARTUP_SECONDS + 1));
assertFalse((Boolean) Whitebox.getInternalState(TestsManagerImpl.class, "waitForSystemStartup"));
}
@Test
public void waitForSystemStartupAllActiveBundles() {
setupBundleContextMock(Bundle.ACTIVE);
- TestsManagerImpl.waitForSystemStartup();
- long timeWaitForSystemStartup = Whitebox.getInternalState(TestsManagerImpl.class, "timeWaitForSystemStartup");
- assertTrue(timeWaitForSystemStartup < TimeUnit.SECONDS.toMillis(2));
+ final long elapsed = TestsManagerImpl.waitForSystemStartup();
+ assertTrue(elapsed < TimeUnit.SECONDS.toMillis(SYSTEM_STARTUP_SECONDS));
assertFalse((Boolean) Whitebox.getInternalState(TestsManagerImpl.class, "waitForSystemStartup"));
}