[geode] branch develop updated: GEODE-5585: Check that threads have been run (#2332)

[kh...@apache.org]

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 <hb...@pivotal.io>
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 -> {