Merge pull request #2 from tyge68/issue/SLING-9200
SLING-9200 Adding a global timeout for waitForSystemStartup
diff --git a/pom.xml b/pom.xml
index 0f4da65..adb0d15 100644
--- a/pom.xml
+++ b/pom.xml
@@ -160,7 +160,7 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
- <version>1.5.11</version>
+ <version>1.7.30</version>
<scope>provided</scope>
</dependency>
<dependency>
@@ -173,7 +173,7 @@
<dependency>
<groupId>ch.qos.logback</groupId>
<artifactId>logback-classic</artifactId>
- <version>1.0.13</version>
+ <version>1.2.3</version>
<scope>provided</scope>
<optional>true</optional>
</dependency>
@@ -203,5 +203,17 @@
<version>${hamcrest.version}</version>
<scope>compile</scope>
</dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-module-junit4</artifactId>
+ <version>2.0.5</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-api-mockito2</artifactId>
+ <version>2.0.5</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
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 57abf84..ed7c6f2 100644
--- a/src/main/java/org/apache/sling/junit/impl/TestsManagerImpl.java
+++ b/src/main/java/org/apache/sling/junit/impl/TestsManagerImpl.java
@@ -53,12 +53,15 @@
private static final Logger log = LoggerFactory.getLogger(TestsManagerImpl.class);
- // the inactivity timeout is the maximum time after the last bundle became active
- // before waiting for more bundles to become active should be aborted
- private static final int DEFAULT_SYSTEM_STARTUP_INACTIVITY_TIMEOUT_SECONDS = 10;
+ // 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";
+
+ private static volatile int globalTimeoutSeconds = Integer.parseInt(System.getProperty(PROPERTY_SYSTEM_STARTUP_GLOBAL_TIMEOUT_SECONDS, "40"));
private static volatile boolean waitForSystemStartup = true;
+ private static volatile long timeWaitForSystemStartup = 0;
+
private ServiceTracker tracker;
private int lastTrackingCount = -1;
@@ -66,7 +69,7 @@
private BundleContext bundleContext;
// List of providers
- private List<TestsProvider> providers = new ArrayList<TestsProvider>();
+ private final List<TestsProvider> providers = new ArrayList<TestsProvider>();
// Map of test names to their provider's PID
private Map<String, String> tests = new ConcurrentHashMap<String, String>();
@@ -126,7 +129,7 @@
boolean reload = false;
for(TestsProvider p : providers) {
final Long lastMod = lastModified.get(p.getServicePid());
- if(lastMod == null || lastMod.longValue() != p.lastModified()) {
+ if(lastMod == null || lastMod != p.lastModified()) {
reload = true;
log.debug("{} updated, will reload test names from all providers", p);
break;
@@ -141,7 +144,7 @@
log.warn("{} has null PID, ignored", p);
continue;
}
- lastModified.put(pid, new Long(p.lastModified()));
+ lastModified.put(pid, p.lastModified());
final List<String> names = p.getTestNames();
for(String name : names) {
tests.put(name, pid);
@@ -162,7 +165,7 @@
result.add(test);
}
}
- log.debug("{} selected {} tests out of {}", new Object[] { selector, result.size(), allTests.size() });
+ log.debug("{} selected {} tests out of {}", selector, result.size(), allTests.size());
return result;
}
}
@@ -222,7 +225,7 @@
}
}
- public void listTests(Collection<String> testNames, Renderer renderer) throws Exception {
+ public void listTests(Collection<String> testNames, Renderer renderer) {
renderer.title(2, "Test classes");
final String note = "The test set can be restricted using partial test names"
+ " as a suffix to this URL"
@@ -244,9 +247,9 @@
}
// wait max inactivityTimeout after the last bundle became active before giving up
- long inactivityTimeout = TimeUnit.SECONDS.toMillis(DEFAULT_SYSTEM_STARTUP_INACTIVITY_TIMEOUT_SECONDS);
- long lastChange = System.currentTimeMillis();
- while (!bundlesToWaitFor.isEmpty() || (lastChange + inactivityTimeout < System.currentTimeMillis())) {
+ 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);
try {
TimeUnit.SECONDS.sleep(1);
@@ -259,20 +262,27 @@
if (bundle.getState() == Bundle.ACTIVE) {
bundles.remove();
log.debug("Bundle {} has become active", bundle.getSymbolicName());
- lastChange = System.currentTimeMillis();
}
}
}
+ timeWaitForSystemStartup = System.currentTimeMillis() - startTime;
+
if (!bundlesToWaitFor.isEmpty()) {
- log.warn("Waited {} seconds but the following bundles are not yet started: {}",
- DEFAULT_SYSTEM_STARTUP_INACTIVITY_TIMEOUT_SECONDS, bundlesToWaitFor);
+ log.warn("Waited {} milliseconds but the following bundles are not yet started: {}",
+ timeWaitForSystemStartup, bundlesToWaitFor);
} else {
log.info("All bundles are active, starting to run tests.");
}
}
}
+ 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 isFragment(final Bundle bundle) {
return bundle.getHeaders().get(Constants.FRAGMENT_HOST) != null;
}
diff --git a/src/test/java/org/apache/sling/junit/impl/TestsManagerImplTest.java b/src/test/java/org/apache/sling/junit/impl/TestsManagerImplTest.java
new file mode 100644
index 0000000..3367ba1
--- /dev/null
+++ b/src/test/java/org/apache/sling/junit/impl/TestsManagerImplTest.java
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.junit.impl;
+
+import static junit.framework.TestCase.assertFalse;
+import static junit.framework.TestCase.assertTrue;
+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;
+import java.util.concurrent.TimeUnit;
+import org.apache.sling.junit.Activator;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mockito;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.powermock.reflect.Whitebox;
+
+/**
+ * Validate waitForSystemStartup method, along with private some implementations.
+ */
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ Activator.class, TestsManagerImpl.class })
+public class TestsManagerImplTest {
+
+ static {
+ // Set the system properties for this test as the default would wait longer.
+ System.setProperty(TestsManagerImpl.PROPERTY_SYSTEM_STARTUP_GLOBAL_TIMEOUT_SECONDS, "2");
+ }
+
+ /**
+ * case if isWaitNeeded 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);
+ final Set<Bundle> bundlesToWaitFor = new HashSet<Bundle>();
+ bundlesToWaitFor.add(Mockito.mock(Bundle.class));
+ boolean isWaitNeeded = Whitebox.invokeMethod(TestsManagerImpl.class, "isWaitNeeded", globalTimeout, bundlesToWaitFor);
+ assertTrue(isWaitNeeded);
+ }
+
+ /**
+ * case if isWaitNeeded 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);
+ }
+
+ /**
+ * case if isWaitNeeded 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);
+ }
+
+ @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));
+ 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));
+ assertFalse((Boolean) Whitebox.getInternalState(TestsManagerImpl.class, "waitForSystemStartup"));
+ }
+
+ private void setupBundleContextMock(final int bundleState) {
+ PowerMockito.mockStatic(Activator.class);
+ BundleContext mockedBundleContext = mock(BundleContext.class);
+ Bundle mockedBundle = mock(Bundle.class);
+ Hashtable<String, String> bundleHeaders = new Hashtable<String, String>();
+ when(mockedBundle.getState()).thenReturn(bundleState);
+ when(mockedBundle.getHeaders()).thenReturn(bundleHeaders);
+ when(mockedBundleContext.getBundles()).thenReturn(new Bundle[] { mockedBundle });
+ when(Activator.getBundleContext()).thenReturn(mockedBundleContext);
+ Whitebox.setInternalState(TestsManagerImpl.class, "waitForSystemStartup", true);
+ }
+}
\ No newline at end of file
