kirklund commented on a change in pull request #5014:
URL: https://github.com/apache/geode/pull/5014#discussion_r420945761



##########
File path: 
geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplTest.java
##########
@@ -620,6 +625,40 @@ public void getCacheServers_isCanonical() {
         .isSameAs(gemFireCacheImpl.getCacheServers());
   }
 
+  @Test
+  public void testLockDiskStore() throws InterruptedException {
+    int nThread = 10;
+    String diskStoreName = "MyDiskStore";
+    AtomicInteger nTrue = new AtomicInteger();
+    AtomicInteger nFalse = new AtomicInteger();
+    ExecutorService executorService = Executors.newFixedThreadPool(nThread);

Review comment:
       I recommend replacing ExecutorService with ExecutorServiceRule. You can 
limit the rule to a specific number of threads if you need to otherwise it 
defaults to as many threads as tasks that you submit:
   ```
   @Rule
   public ExecutorServiceRule executorServiceRule = new ExecutorServiceRule();
   ```
   The Rule will automatically do shutdown etc during tearDown().
   
   Also, you might want to capture the `Future<Void>` return value from 
`executorService/executorServiceRule.submit` to await on. You could even have a 
`Collection<Future<Void>>` if you wanted. 
   
   If you await on the Futures, then any assertion failures will be thrown 
causing the test to fail:
   ```
   Future<Void> doLockUnlock = executorService.submit(() -> {
     try {
       assertThat(gemFireCacheImpl.doLockDiskStore(diskStoreName)).isTrue();
     } finally {
       assertThat(gemFireCacheImpl. doUnlockDiskStore(diskStoreName)).isTrue();
     }
   }
   
   doLockUnlock.get(GeodeAwaitility.getTimeout().toMillis(), 
TimeUnit.MILLISECONDS);
   ```
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to