This is an automated email from the ASF dual-hosted git repository.
khowe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 2868c3c GEODE-5585: Check that threads have been run (#2332)
2868c3c is described below
commit 2868c3c0ece3ecab658d1a431bfd753b52537ea5
Author: Helena Bales <[email protected]>
AuthorDate: Tue Aug 21 12:38:42 2018 -0700
GEODE-5585: Check that threads have been run (#2332)
* GEODE-5585: Check that threads have been run
In the ConcurrencyRule's after(), check that all threads that have been
added have been run, to protect from tests having false passes due to
threads never being executed and results gathered. If tests really need
to be added and not run, clear() must be used before after() is invoked.
---
.../geode/test/junit/rules/ConcurrencyRule.java | 14 ++++++++++++-
.../test/junit/rules/ConcurrencyRuleTest.java | 24 ++++++++++++++++++++++
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git
a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/ConcurrencyRule.java
b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/ConcurrencyRule.java
index b45c8e2..d17096b 100644
---
a/geode-junit/src/main/java/org/apache/geode/test/junit/rules/ConcurrencyRule.java
+++
b/geode-junit/src/main/java/org/apache/geode/test/junit/rules/ConcurrencyRule.java
@@ -28,6 +28,7 @@ import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import com.google.common.base.Stopwatch;
import org.junit.rules.ErrorCollector;
@@ -81,6 +82,8 @@ public class ConcurrencyRule extends
SerializableExternalResource {
private ProtectedErrorCollector errorCollector;
private Duration timeout;
+ private final AtomicBoolean allThreadsExecuted = new AtomicBoolean(false);
+
/**
* A default constructor that sets the timeout to a default of 30 seconds
*/
@@ -89,6 +92,7 @@ public class ConcurrencyRule extends
SerializableExternalResource {
futures = new ArrayList<>();
timeout = Duration.ofSeconds(300);
errorCollector = new ProtectedErrorCollector();
+ allThreadsExecuted.set(false);
}
/**
@@ -102,10 +106,14 @@ public class ConcurrencyRule extends
SerializableExternalResource {
futures = new ArrayList<>();
this.timeout = timeout;
errorCollector = new ProtectedErrorCollector();
+ allThreadsExecuted.set(false);
}
@Override
- protected void after() {
+ protected void after() throws IllegalStateException {
+ if (allThreadsExecuted.get() == Boolean.FALSE) {
+ throw new IllegalStateException("Threads have been added that have not
been executed.");
+ }
clear();
stopThreadPool();
}
@@ -122,6 +130,7 @@ public class ConcurrencyRule extends
SerializableExternalResource {
public <T> ConcurrentOperation<T> add(Callable<T> callable) {
ConcurrentOperation<T> concurrentOperation = new
ConcurrentOperation(callable);
toInvoke.add(concurrentOperation);
+ allThreadsExecuted.set(false);
return concurrentOperation;
}
@@ -143,6 +152,7 @@ public class ConcurrencyRule extends
SerializableExternalResource {
for (ConcurrentOperation op : toInvoke) {
futures.add(threadPool.submit(op));
}
+ allThreadsExecuted.set(true);
awaitFutures();
errorCollector.verify();
@@ -164,6 +174,7 @@ public class ConcurrencyRule extends
SerializableExternalResource {
for (ConcurrentOperation op : toInvoke) {
awaitFuture(threadPool.submit(op));
}
+ allThreadsExecuted.set(true);
errorCollector.verify();
}
@@ -176,6 +187,7 @@ public class ConcurrencyRule extends
SerializableExternalResource {
toInvoke.clear();
futures.clear();
errorCollector = new ProtectedErrorCollector();
+ allThreadsExecuted.set(true);
}
/**
diff --git
a/geode-junit/src/test/java/org/apache/geode/test/junit/rules/ConcurrencyRuleTest.java
b/geode-junit/src/test/java/org/apache/geode/test/junit/rules/ConcurrencyRuleTest.java
index 282cdb5..9607bff 100644
---
a/geode-junit/src/test/java/org/apache/geode/test/junit/rules/ConcurrencyRuleTest.java
+++
b/geode-junit/src/test/java/org/apache/geode/test/junit/rules/ConcurrencyRuleTest.java
@@ -418,6 +418,30 @@ public class ConcurrencyRuleTest {
});
}
+ @Test
+ public void afterFailsIfThreadsWereNotRun() {
+ Callable<Integer> c1 = () -> {
+ return 2;
+ };
+
+ Callable<String> c2 = () -> {
+ return "some string";
+ };
+
+ concurrencyRule.add(c1).expectValue(2);
+ concurrencyRule.add(c1).expectValue(2).repeatForIterations(5);
+ concurrencyRule.executeInParallel();
+
+ concurrencyRule.add(c1).expectValue(3);
+ concurrencyRule.add(c2).expectValue("some string");
+
+ assertThatThrownBy(() -> concurrencyRule.after())
+ .isInstanceOf(IllegalStateException.class)
+ .withFailMessage("exception should have been thrown");
+
+ concurrencyRule.clear(); // so that this test's after succeeds
+ }
+
@SuppressWarnings("unused")
private enum Execution {
EXECUTE_IN_SERIES(concurrencyRule -> {
