geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bschucha...@apache.org
Subject [28/48] geode git commit: GEODE-3469: prevent zero pid from AvailablePid for tests
Date Tue, 22 Aug 2017 22:36:30 GMT
GEODE-3469: prevent zero pid from AvailablePid for tests

This closes #724


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/28616a27
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/28616a27
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/28616a27

Branch: refs/heads/feature/GEODE-3447
Commit: 28616a27ec79c0693fb2fbb1a135cf3df3ad4150
Parents: c4def6b
Author: Kirk Lund <klund@apache.org>
Authored: Fri Aug 18 14:33:25 2017 -0700
Committer: Kirk Lund <klund@apache.org>
Committed: Mon Aug 21 14:59:26 2017 -0700

----------------------------------------------------------------------
 .../internal/process/lang/AvailablePid.java     | 119 ++++++++++++++++---
 .../internal/process/lang/AvailablePidTest.java |  81 +++++++++++--
 2 files changed, 170 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/28616a27/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePid.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePid.java
b/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePid.java
index d26f73e..c74bd10 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePid.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePid.java
@@ -12,55 +12,107 @@
  * or implied. See the License for the specific language governing permissions and limitations
under
  * the License.
  */
-
 package org.apache.geode.internal.process.lang;
 
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
 
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Random;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
-import org.apache.geode.internal.util.StopWatch;
+import com.google.common.base.Stopwatch;
 
 /**
  * Finds unused pids for use in tests.
  */
 public class AvailablePid {
 
-  static final int LOWER_BOUND = 1;
-  static final int UPPER_BOUND = 64000;
+  static final int DEFAULT_LOWER_BOUND = 1;
+  static final int DEFAULT_UPPER_BOUND = 64000;
   static final int DEFAULT_TIMEOUT_MILLIS = 60 * 1000;
 
+  private final int lowerBound;
+  private final int upperBound;
   private final Random random;
   private final int timeoutMillis;
 
   /**
-   * Construct with no seed and default timeout of 1 minute.
+   * Construct with:
+   * <ul>
+   * <li>default {@link Bounds} of {@link #DEFAULT_LOWER_BOUND} (inclusive) and
+   * {@link #DEFAULT_UPPER_BOUND} (inclusive)
+   * <li>Random with no see
+   * <li>default timeout of 1 minute.
+   * </ul>
    */
   public AvailablePid() {
-    this(new Random(), DEFAULT_TIMEOUT_MILLIS);
+    this(new Bounds(DEFAULT_LOWER_BOUND, DEFAULT_UPPER_BOUND), new Random(),
+        DEFAULT_TIMEOUT_MILLIS);
+  }
+
+  /**
+   * Construct with:
+   * <ul>
+   * <li>default {@link Bounds} of {@link #DEFAULT_LOWER_BOUND} (inclusive) and
+   * {@link #DEFAULT_UPPER_BOUND} (inclusive)
+   * <li>Random with specified seed
+   * <li>default timeout of 1 minute
+   * </ul>
+   */
+  public AvailablePid(final long seed) {
+    this(new Bounds(DEFAULT_LOWER_BOUND, DEFAULT_UPPER_BOUND), new Random(seed),
+        DEFAULT_TIMEOUT_MILLIS);
   }
 
   /**
-   * Construct with specified seed and timeout.
+   * Construct with:
+   * <ul>
+   * <li>default {@link Bounds} of {@link #DEFAULT_LOWER_BOUND} (inclusive) and
+   * {@link #DEFAULT_UPPER_BOUND} (inclusive)
+   * <li>specified Random instance
+   * <li>default timeout of 1 minute
+   * </ul>
    */
-  public AvailablePid(final long seed, final int timeoutMillis) {
-    this(new Random(seed), timeoutMillis);
+  public AvailablePid(final Random random) {
+    this(new Bounds(DEFAULT_LOWER_BOUND, DEFAULT_UPPER_BOUND), random, DEFAULT_TIMEOUT_MILLIS);
   }
 
   /**
-   * Construct with specified Random implementation.
+   * Construct with:
+   * <ul>
+   * <li>specified {@link Bounds} of {@code lowerBound} (inclusive) and {@code upperBound}
+   * (inclusive)
+   * <li>Random with no seed
+   * <li>default timeout of 1 minute
+   * </ul>
    */
-  public AvailablePid(final Random random, final int timeoutMillis) {
+  public AvailablePid(final Bounds bounds) {
+    this(bounds, new Random(), DEFAULT_TIMEOUT_MILLIS);
+  }
+
+  /**
+   * Construct with:
+   * <ul>
+   * <li>specified {@link Bounds} of {@code lowerBound} (inclusive) and {@code upperBound}
+   * (inclusive)
+   * <li>specified Random instance
+   * <li>specified default timeout millis
+   * </ul>
+   */
+  public AvailablePid(final Bounds bounds, final Random random, final int timeoutMillis)
{
+    this.lowerBound = bounds.lowerBound;
+    this.upperBound = bounds.upperBound;
     this.random = random;
     this.timeoutMillis = timeoutMillis;
   }
 
   /**
-   * Returns specified pid if it's unused. Else returns randomly unused pid.
+   * Returns specified pid if it's unused. Else returns randomly unused pid between
+   * {@code lowerBound} (inclusive) and {@code upperBound} (inclusive).
    */
   public int findAvailablePid(final int pid) throws TimeoutException {
     if (isProcessAlive(pid)) {
@@ -70,13 +122,14 @@ public class AvailablePid {
   }
 
   /**
-   * Returns randomly unused pid.
+   * Returns randomly unused pid between {@code lowerBound} (inclusive) and {@code upperBound}
+   * (inclusive).
    */
   public int findAvailablePid() throws TimeoutException {
-    StopWatch stopWatch = new StopWatch(true);
+    Stopwatch stopwatch = Stopwatch.createStarted();
     int pid = random();
     while (isProcessAlive(pid)) {
-      if (stopWatch.elapsedTimeMillis() > timeoutMillis) {
+      if (stopwatch.elapsed(MILLISECONDS) > timeoutMillis) {
         throw new TimeoutException(
             "Failed to find available pid within " + timeoutMillis + " millis.");
       }
@@ -86,7 +139,8 @@ public class AvailablePid {
   }
 
   /**
-   * Returns specified number of randomly unused pids.
+   * Returns specified number of unique, randomly unused pids between {@code lowerBound}
(inclusive)
+   * and {@code upperBound} (inclusive).
    */
   public int[] findAvailablePids(final int number) throws TimeoutException {
     Set<Integer> pids = new HashSet<>();
@@ -99,8 +153,37 @@ public class AvailablePid {
     return Arrays.stream(pids.toArray(new Integer[0])).mapToInt(Integer::intValue).toArray();
   }
 
-  private int random() {
-    return random.nextInt(UPPER_BOUND - LOWER_BOUND);
+  /**
+   * Returns a value between {@code lowerBound} (inclusive) and {@code upperBound} (inclusive)
+   */
+  int random() {
+    return random.nextInt(upperBound + 1 - lowerBound) + lowerBound;
+  }
+
+  /**
+   * Lower and upper bounds for desired PIDs. Both are inclusive -- if you specify
+   * {@code new Bounds(1, 100)} then {@code AvailablePid} will return values of 1 through
100.
+   *
+   * <ul>
+   * <li>{@code lowerBound} must be an integer greater than zero.
+   * <li>{@code upperBound} must be an integer greater than {@code lowerBound}.
+   * </ul>
+   */
+  static class Bounds {
+    final int lowerBound;
+    final int upperBound;
+
+    Bounds(final int lowerBound, final int upperBound) {
+      if (lowerBound < 1) {
+        throw new IllegalArgumentException("lowerBound must be greater than '0'");
+      }
+      if (upperBound <= lowerBound) {
+        throw new IllegalArgumentException(
+            "upperBound must be greater than lowerBound '" + lowerBound + "'");
+      }
+      this.lowerBound = lowerBound;
+      this.upperBound = upperBound;
+    }
   }
 
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/28616a27/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
b/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
index 00beb67..48ba235 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/process/lang/AvailablePidTest.java
@@ -12,13 +12,16 @@
  * or implied. See the License for the specific language governing permissions and limitations
under
  * the License.
  */
-
 package org.apache.geode.internal.process.lang;
 
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.geode.internal.process.ProcessUtils.identifyPid;
 import static org.apache.geode.internal.process.ProcessUtils.isProcessAlive;
-import static org.apache.geode.internal.process.lang.AvailablePid.DEFAULT_TIMEOUT_MILLIS;
+import static org.apache.geode.internal.process.lang.AvailablePid.DEFAULT_LOWER_BOUND;
+import static org.apache.geode.internal.process.lang.AvailablePid.DEFAULT_UPPER_BOUND;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.awaitility.Awaitility.await;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.spy;
@@ -29,9 +32,12 @@ import java.util.Random;
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import com.google.common.base.Stopwatch;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.junit.rules.Timeout;
 
 import org.apache.geode.test.junit.categories.UnitTest;
 
@@ -43,6 +49,9 @@ public class AvailablePidTest {
 
   private AvailablePid availablePid;
 
+  @Rule
+  public Timeout timeout = Timeout.builder().withTimeout(20, SECONDS).build();
+
   @Before
   public void before() throws Exception {
     availablePid = new AvailablePid();
@@ -50,39 +59,39 @@ public class AvailablePidTest {
 
   @Test
   public void lowerBoundShouldBeLegalPid() throws Exception {
-    assertThat(isProcessAlive(AvailablePid.LOWER_BOUND)).isIn(true, false);
+    assertThat(isProcessAlive(DEFAULT_LOWER_BOUND)).isIn(true, false);
   }
 
   @Test
   public void upperBoundShouldBeLegalPid() throws Exception {
-    assertThat(isProcessAlive(AvailablePid.UPPER_BOUND)).isIn(true, false);
+    assertThat(isProcessAlive(DEFAULT_UPPER_BOUND)).isIn(true, false);
   }
 
-  @Test(timeout = DEFAULT_TIMEOUT_MILLIS)
+  @Test
   public void findAvailablePidShouldNotReturnLocalPid() throws Exception {
     int pid = availablePid.findAvailablePid();
 
     assertThat(pid).isNotEqualTo(identifyPid());
   }
 
-  @Test(timeout = DEFAULT_TIMEOUT_MILLIS)
+  @Test
   public void findAvailablePidShouldNotReturnLivePid() throws Exception {
     int pid = availablePid.findAvailablePid();
 
     assertThat(isProcessAlive(pid)).isFalse();
   }
 
-  @Test(timeout = DEFAULT_TIMEOUT_MILLIS)
-  public void findAvailablePidShouldReturnRandomPid() throws Exception {
+  @Test
+  public void findAvailablePidShouldUseRandom() throws Exception {
     Random random = spy(new Random());
-    availablePid = new AvailablePid(random, DEFAULT_TIMEOUT_MILLIS);
+    availablePid = new AvailablePid(random);
 
     availablePid.findAvailablePid();
 
     verify(random, atLeastOnce()).nextInt(anyInt());
   }
 
-  @Test(timeout = DEFAULT_TIMEOUT_MILLIS)
+  @Test
   public void findAvailablePidsShouldReturnSpecifiedNumberOfPids() throws Exception {
     assertThat(availablePid.findAvailablePids(1)).hasSize(1);
     assertThat(availablePid.findAvailablePids(2)).hasSize(2);
@@ -91,7 +100,7 @@ public class AvailablePidTest {
     assertThat(availablePid.findAvailablePids(8)).hasSize(8);
   }
 
-  @Test(timeout = DEFAULT_TIMEOUT_MILLIS)
+  @Test
   public void findAvailablePidsShouldReturnNoDuplicatedPids() throws Exception {
     assertThatNoPidIsDuplicated(availablePid.findAvailablePids(1));
     assertThatNoPidIsDuplicated(availablePid.findAvailablePids(2));
@@ -100,7 +109,55 @@ public class AvailablePidTest {
     assertThatNoPidIsDuplicated(availablePid.findAvailablePids(8));
   }
 
-  private void assertThatNoPidIsDuplicated(int[] pids) {
+  @Test
+  public void findAvailablePidShouldReturnGreaterThanOrEqualToLowerBound() throws Exception
{
+    availablePid = new AvailablePid(new AvailablePid.Bounds(1, 10));
+    Stopwatch stopwatch = Stopwatch.createStarted();
+
+    do {
+      assertThat(availablePid.findAvailablePid()).isGreaterThanOrEqualTo(1);
+    } while (stopwatch.elapsed(SECONDS) < 2);
+  }
+
+  @Test
+  public void findAvailablePidShouldReturnLessThanOrEqualToUpperBound() throws Exception
{
+    availablePid = new AvailablePid(new AvailablePid.Bounds(1, 10));
+    Stopwatch stopwatch = Stopwatch.createStarted();
+
+    do {
+      assertThat(availablePid.findAvailablePid()).isLessThanOrEqualTo(10);
+    } while (stopwatch.elapsed(SECONDS) < 2);
+  }
+
+  @Test
+  public void randomLowerBoundIsInclusive() throws Exception {
+    availablePid = new AvailablePid(new AvailablePid.Bounds(1, 3));
+
+    await().atMost(10, SECONDS).until(() -> assertThat(availablePid.random()).isEqualTo(1));
+  }
+
+  @Test
+  public void randomUpperBoundIsInclusive() throws Exception {
+    availablePid = new AvailablePid(new AvailablePid.Bounds(1, 3));
+
+    await().atMost(10, SECONDS).until(() -> assertThat(availablePid.random()).isEqualTo(3));
+  }
+
+  @Test
+  public void lowerBoundMustBeGreaterThanZero() throws Exception {
+    assertThatThrownBy(() -> new AvailablePid(new AvailablePid.Bounds(0, 1)))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("lowerBound must be greater than '0'");
+  }
+
+  @Test
+  public void upperBoundMustBeGreaterThanLowerBound() throws Exception {
+    assertThatThrownBy(() -> new AvailablePid(new AvailablePid.Bounds(1, 1)))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("upperBound must be greater than lowerBound '1'");
+  }
+
+  private void assertThatNoPidIsDuplicated(final int[] pids) {
     Set<Integer> pidSet = Arrays.stream(pids).boxed().collect(Collectors.toSet());
     assertThat(pidSet).hasSize(pids.length);
   }


Mime
View raw message