Fix retry option without expiration in test env (#315)

We allow user to specify either expiration or maxAttempt in retry options.
Currently if only maxAttempt is set, test Env state machine will throw exception.
Moreover, if only maxAttempt is set, expiration should be treated as infinite when deciding whether to retry or not.

Author: meiliang86

src/main/java/com/uber/cadence/internal/testservice/RetryState.java

@@ -34,7 +34,8 @@ final class RetryState {
 
   private RetryState(RetryPolicy retryPolicy, long expirationTime, int attempt) {
     this.retryPolicy = retryPolicy;
-    this.expirationTime = expirationTime;
+    this.expirationTime =
+        retryPolicy.getExpirationIntervalInSeconds() == 0 ? Long.MAX_VALUE : expirationTime;
     this.attempt = attempt;
   }
 
@@ -121,10 +122,6 @@ static RetryPolicy validateRetryPolicy(RetryPolicy policy) throws BadRequestErro
     if (policy.getMaximumAttempts() < 0) {
       throw new BadRequestError("MaximumAttempts cannot be less than 0 on retry policy.");
     }
-    if (policy.getExpirationIntervalInSeconds() <= 0) {
-      throw new BadRequestError(
-          "ExpirationIntervalInSeconds must be greater than 0 on retry policy.");
-    }
     if (policy.getMaximumAttempts() == 0 && policy.getExpirationIntervalInSeconds() == 0) {
       throw new BadRequestError(
           "MaximumAttempts and ExpirationIntervalInSeconds are both 0. At least one of them must be specified.");

src/test/java/com/uber/cadence/workflow/WorkflowTest.java

@@ -399,7 +399,7 @@ public void testSync() {
         "executeActivity TestActivities::activity2");
   }
 
-  public static class TestActivityRetry implements TestWorkflow1 {
+  public static class TestActivityRetryWithMaxAttempts implements TestWorkflow1 {
 
     @Override
     @SuppressWarnings("Finally")
@@ -408,12 +408,9 @@ public String execute(String taskList) {
           new ActivityOptions.Builder()
               .setTaskList(taskList)
               .setHeartbeatTimeout(Duration.ofSeconds(5))
-              .setScheduleToCloseTimeout(Duration.ofSeconds(5))
-              .setScheduleToStartTimeout(Duration.ofSeconds(5))
-              .setStartToCloseTimeout(Duration.ofSeconds(10))
+              .setScheduleToCloseTimeout(Duration.ofSeconds(3))
               .setRetryOptions(
                   new RetryOptions.Builder()
-                      .setExpiration(Duration.ofSeconds(100))
                       .setMaximumInterval(Duration.ofSeconds(1))
                       .setInitialInterval(Duration.ofSeconds(1))
                       .setMaximumAttempts(3)
@@ -426,16 +423,62 @@ public String execute(String taskList) {
         activities.heartbeatAndThrowIO();
       } finally {
         if (Workflow.currentTimeMillis() - start < 2000) {
-          throw new RuntimeException("Activity retried without delay");
+          fail("Activity retried without delay");
         }
       }
       return "ignored";
     }
   }
 
   @Test
-  public void testActivityRetry() {
-    startWorkerFor(TestActivityRetry.class);
+  public void testActivityRetryWithMaxAttempts() {
+    startWorkerFor(TestActivityRetryWithMaxAttempts.class);
+    TestWorkflow1 workflowStub =
+        workflowClient.newWorkflowStub(
+            TestWorkflow1.class, newWorkflowOptionsBuilder(taskList).build());
+    try {
+      workflowStub.execute(taskList);
+      fail("unreachable");
+    } catch (WorkflowException e) {
+      assertTrue(e.getCause().getCause() instanceof IOException);
+    }
+    assertEquals(activitiesImpl.toString(), 3, activitiesImpl.invocations.size());
+  }
+
+  public static class TestActivityRetryWithExpiration implements TestWorkflow1 {
+
+    @Override
+    @SuppressWarnings("Finally")
+    public String execute(String taskList) {
+      ActivityOptions options =
+          new ActivityOptions.Builder()
+              .setTaskList(taskList)
+              .setHeartbeatTimeout(Duration.ofSeconds(5))
+              .setScheduleToCloseTimeout(Duration.ofSeconds(3))
+              .setRetryOptions(
+                  new RetryOptions.Builder()
+                      .setExpiration(Duration.ofSeconds(3))
+                      .setMaximumInterval(Duration.ofSeconds(1))
+                      .setInitialInterval(Duration.ofSeconds(1))
+                      .setDoNotRetry(AssertionError.class)
+                      .build())
+              .build();
+      TestActivities activities = Workflow.newActivityStub(TestActivities.class, options);
+      long start = Workflow.currentTimeMillis();
+      try {
+        activities.heartbeatAndThrowIO();
+      } finally {
+        if (Workflow.currentTimeMillis() - start < 2000) {
+          fail("Activity retried without delay");
+        }
+      }
+      return "ignored";
+    }
+  }
+
+  @Test
+  public void testActivityRetryWithExiration() {
+    startWorkerFor(TestActivityRetryWithExpiration.class);
     TestWorkflow1 workflowStub =
         workflowClient.newWorkflowStub(
             TestWorkflow1.class, newWorkflowOptionsBuilder(taskList).build());
@@ -1241,17 +1284,17 @@ public void testMemo() {
       startWorkerFor(TestMultiargsWorkflowsImpl.class);
       WorkflowOptions workflowOptions = newWorkflowOptionsBuilder(taskList).setMemo(memo).build();
       TestMultiargsWorkflowsFunc stubF =
-              workflowClient.newWorkflowStub(TestMultiargsWorkflowsFunc.class, workflowOptions);
+          workflowClient.newWorkflowStub(TestMultiargsWorkflowsFunc.class, workflowOptions);
       WorkflowExecution executionF = WorkflowClient.start(stubF::func);
 
       GetWorkflowExecutionHistoryResponse historyResp =
-              WorkflowExecutionUtils.getHistoryPage(
-                      new byte[] {}, testEnvironment.getWorkflowService(), DOMAIN, executionF);
+          WorkflowExecutionUtils.getHistoryPage(
+              new byte[] {}, testEnvironment.getWorkflowService(), DOMAIN, executionF);
       HistoryEvent startEvent = historyResp.history.getEvents().get(0);
       Memo memoFromEvent = startEvent.workflowExecutionStartedEventAttributes.getMemo();
       byte[] memoBytes = memoFromEvent.getFields().get(testMemoKey).array();
       String memoRetrieved =
-              JsonDataConverter.getInstance().fromData(memoBytes, String.class, String.class);
+          JsonDataConverter.getInstance().fromData(memoBytes, String.class, String.class);
       assertEquals(testMemoValue, memoRetrieved);
     }
   }
@@ -3761,7 +3804,9 @@ public void testGetVersion() {
   }
 
   static CompletableFuture<Boolean> executionStarted = new CompletableFuture<>();
-  public static class TestGetVersionWithoutDecisionEventWorkflowImpl implements TestWorkflowSignaled {
+
+  public static class TestGetVersionWithoutDecisionEventWorkflowImpl
+      implements TestWorkflowSignaled {
 
     CompletablePromise<Boolean> signalReceived = Workflow.newPromise();
 
@@ -3774,7 +3819,8 @@ public String execute() {
           executionStarted.complete(true);
           signalReceived.get();
         } else {
-          // Execute getVersion in replay mode. In this case we have no decision event, only a signal.
+          // Execute getVersion in replay mode. In this case we have no decision event, only a
+          // signal.
           int version = Workflow.getVersion("test_change", Workflow.DEFAULT_VERSION, 1);
           if (version == Workflow.DEFAULT_VERSION) {
             signalReceived.get();