thomasmueller commented on code in PR #1664:
URL: https://github.com/apache/jackrabbit-oak/pull/1664#discussion_r1732541135


##########
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/AsyncCheckpointCreatorTest.java:
##########
@@ -47,14 +49,36 @@ public void testAsync() {
         }
         // Task run post the minConcurrentCheckpoints should not result in 
additional
         // checkpoints since the oldest checkpoint should get deleted
-        for (int j = 0; j < 2 ; j++) {
+        for (int j = 0; j < 2; j++) {
             List<String> checkpointList = new ArrayList<>();
+            sleepOneMillisecond();
             task.run();
-            for(String checkpoint : store.checkpoints()) {
+            for (String checkpoint : store.checkpoints()) {
                 checkpointList.add(checkpoint);
             }
             Assert.assertFalse(checkpointList.contains(checkpointMap.get(j + 
1)));
             Assert.assertEquals(minConcurrentCheckpoints, 
checkpointList.size());
         }
     }
+    
+    /**
+     * The "oldest" checkpoint is removed, but for this to work reliably, the
+     * checkpoints need to be at least 1 ms apart. So here we wait at least 1 
ms,
+     * such that the checkpoints are not on the same millisecond. This is a 
bit a
+     * hack, but I think it's safer to change the test case than to change the 
code.
+     */
+    private static void sleepOneMillisecond() {
+        long start = System.currentTimeMillis();
+        while (true) {
+            try {
+                Thread.sleep(1);
+            } catch (InterruptedException e) {
+                // ignore
+            }
+            long time = System.currentTimeMillis();
+            if (time - start >= 1) {
+                break;
+            }
+        }
+    }

Review Comment:
   There are two cases: the thread could be interrupted, and the milliseconds 
could change (adjusting the system clock). The second case is probably more 
likely. In which case, the test still fails, because the millisecond is still 
the same. (The real code uses the System.currentTimeMillis).
   
   > which can block an attempt of terminating the thread/JVM.
   
   Yes. But this is a test case. I'm not worried about this.
   
   I'm fixing a broken test case here. 



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to