You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2020/03/17 13:05:52 UTC

[sling-org-apache-sling-junit-core] branch master updated: SLING-9200 - simplify needToWait(...), use more constants and minor cleanup

This is an automated email from the ASF dual-hosted git repository.

bdelacretaz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-junit-core.git


The following commit(s) were added to refs/heads/master by this push:
     new 7e7b214  SLING-9200 - simplify needToWait(...), use more constants and minor cleanup
7e7b214 is described below

commit 7e7b2144753d861a6f70beec2674690a724b5ddc
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Tue Mar 17 14:05:36 2020 +0100

    SLING-9200 - simplify needToWait(...), use more constants and minor cleanup
---
 .../apache/sling/junit/impl/TestsManagerImpl.java  | 36 +++++++-------
 .../sling/junit/impl/TestsManagerImplTest.java     | 56 ++++++++++------------
 2 files changed, 44 insertions(+), 48 deletions(-)

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.Map;
 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 @@ public class TestsManagerImpl implements TestsManager {
     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 class TestsManagerImpl implements TestsManager {
     }
 
 
-    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 @@ public class TestsManagerImpl implements TestsManager {
             }
 
             // 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 @@ public class TestsManagerImpl implements TestsManager {
                     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 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;
@@ -44,66 +43,59 @@ import org.powermock.reflect.Whitebox;
 @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"));
   }