You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ud...@apache.org on 2017/08/22 16:39:28 UTC
[09/14] geode git commit: GEODE-3469: prevent zero pid from
AvailablePid for tests
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-3416
Commit: 28616a27ec79c0693fb2fbb1a135cf3df3ad4150
Parents: c4def6b
Author: Kirk Lund <kl...@apache.org>
Authored: Fri Aug 18 14:33:25 2017 -0700
Committer: Kirk Lund <kl...@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);
}