Flaky test: testChildWorkflowWithCronSchedule (#610)

* Added a map that records lastCompletionResult for every run to solve flakiness

Author: vkoby

temporal-sdk/src/test/java/io/temporal/workflow/WorkflowWithCronScheduleTest.java

@@ -29,6 +29,7 @@
 import io.temporal.workflow.shared.SDKTestWorkflowRule;
 import io.temporal.workflow.shared.TestWorkflowWithCronScheduleImpl;
 import java.time.Duration;
+import java.util.Map;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
@@ -70,7 +71,9 @@ public void testWorkflowWithCronSchedule() {
     }
 
     // Run 3 failed. So on run 4 we get the last completion result from run 2.
-    assertEquals("run 2", TestWorkflowWithCronScheduleImpl.lastCompletionResult);
+    Map<Integer, String> lastCompletionResults =
+        TestWorkflowWithCronScheduleImpl.lastCompletionResults.get(testName.getMethodName());
+    assertEquals("run 2", lastCompletionResults.get(4));
     // The last failure ought to be the one from run 3
     assertTrue(TestWorkflowWithCronScheduleImpl.lastFail.isPresent());
     assertTrue(

temporal-sdk/src/test/java/io/temporal/workflow/childWorkflowTests/ChildWorkflowWithCronScheduleTest.java

@@ -32,6 +32,7 @@
 import io.temporal.workflow.shared.TestWorkflows.TestWorkflow1;
 import io.temporal.workflow.shared.TestWorkflows.TestWorkflowWithCronSchedule;
 import java.time.Duration;
+import java.util.Map;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
@@ -56,11 +57,7 @@ public void testChildWorkflowWithCronSchedule() {
         testWorkflowRule
             .getWorkflowClient()
             .newUntypedWorkflowStub(
-                "TestWorkflow1",
-                newWorkflowOptionsWithTimeouts(testWorkflowRule.getTaskQueue())
-                    .toBuilder()
-                    .setWorkflowRunTimeout(Duration.ofHours(10))
-                    .build());
+                "TestWorkflow1", newWorkflowOptionsWithTimeouts(testWorkflowRule.getTaskQueue()));
     client.start(testName.getMethodName());
     testWorkflowRule.getTestEnvironment().sleep(Duration.ofHours(3));
     client.cancel();
@@ -73,16 +70,18 @@ public void testChildWorkflowWithCronSchedule() {
     }
 
     // Run 3 failed. So on run 4 we get the last completion result from run 2.
-    assertEquals("run 2", TestWorkflowWithCronScheduleImpl.lastCompletionResult);
+    Map<Integer, String> lastCompletionResults =
+        TestWorkflowWithCronScheduleImpl.lastCompletionResults.get(testName.getMethodName());
+    assertEquals("run 2", lastCompletionResults.get(4));
   }
 
   public static class TestCronParentWorkflow implements TestWorkflow1 {
     private final TestWorkflowWithCronSchedule cronChild =
         Workflow.newChildWorkflowStub(TestWorkflowWithCronSchedule.class);
 
     @Override
-    public String execute(String taskQueue) {
-      return cronChild.execute(taskQueue);
+    public String execute(String testName) {
+      return cronChild.execute(testName);
     }
   }
 }

temporal-sdk/src/test/java/io/temporal/workflow/shared/TestWorkflowWithCronScheduleImpl.java

@@ -25,6 +25,7 @@
 import io.temporal.workflow.shared.TestWorkflows.TestWorkflowWithCronSchedule;
 import java.text.SimpleDateFormat;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
@@ -34,7 +35,8 @@
 public class TestWorkflowWithCronScheduleImpl implements TestWorkflowWithCronSchedule {
 
   public static final Map<String, AtomicInteger> retryCount = new ConcurrentHashMap<>();
-  public static String lastCompletionResult;
+  public static final Map<String, Map<Integer, String>> lastCompletionResults =
+      new ConcurrentHashMap<>();
   public static Optional<Exception> lastFail;
 
   @Override
@@ -46,23 +48,19 @@ public String execute(String testName) {
       return null;
     }
 
-    lastCompletionResult = Workflow.getLastCompletionResult(String.class);
     lastFail = Workflow.getPreviousRunFailure();
+    int count = retryCount.computeIfAbsent(testName, k -> new AtomicInteger()).incrementAndGet();
+    lastCompletionResults
+        .computeIfAbsent(testName, k -> new HashMap<>())
+        .put(count, Workflow.getLastCompletionResult(String.class));
 
-    AtomicInteger count = retryCount.get(testName);
-    if (count == null) {
-      count = new AtomicInteger();
-      retryCount.put(testName, count);
-    }
-    int c = count.incrementAndGet();
-
-    if (c == 3) {
+    if (count == 3) {
       throw ApplicationFailure.newFailure("simulated error", "test");
     }
 
     SimpleDateFormat sdf = new SimpleDateFormat("MMM dd,yyyy HH:mm:ss.SSS");
     Date now = new Date(Workflow.currentTimeMillis());
     log.debug("TestWorkflowWithCronScheduleImpl run at " + sdf.format(now));
-    return "run " + c;
+    return "run " + count;
   }
 }

temporal-testing/build.gradle

@@ -12,7 +12,7 @@ dependencies {
 
     testImplementation group: 'junit', name: 'junit', version: '4.13.2'
     testImplementation group: 'ch.qos.logback', name: 'logback-classic', version: '1.2.5'
-    testRuntimeOnly group: 'org.slf4j', name: 'slf4j-simple', version: '1.7.31'
+    testRuntimeOnly group: 'org.slf4j', name: 'slf4j-simple', version: '1.7.32'
 }
 
 task testServiceServer(type: CreateStartScripts) {