Corrected error messages in getValidatedActivityOptions (#1224)

* Improved error messages in getValidatedActivityOptions, also removed redundant call to getActivityOptions

* Added tests for the error messages in getValidatedActivityOptions

* Rolled back to the previous error messages and transformed the three new tests into a table test

Author: jakobht

internal/internal_activity.go

@@ -187,7 +187,7 @@ func getValidatedActivityOptions(ctx Context) (*activityOptions, error) {
 		return nil, errors.New("missing or negative StartToCloseTimeoutSeconds")
 	}
 	if p.ScheduleToCloseTimeoutSeconds < 0 {
-		return nil, errors.New("missing or negative ScheduleToCloseTimeoutSeconds")
+		return nil, errors.New("invalid negative ScheduleToCloseTimeoutSeconds")
 	}
 	if p.ScheduleToCloseTimeoutSeconds == 0 {
 		// This is a optional parameter, we default to sum of the other two timeouts.

internal/internal_workflow_test.go

@@ -75,36 +75,88 @@ func helloWorldAct(ctx context.Context) (string, error) {
 	return "test", nil
 }
 
-func helloWorldActivityWorkflow(ctx Context, input string) (result string, err error) {
-	ao := ActivityOptions{
-		ScheduleToStartTimeout: 10 * time.Second,
-		StartToCloseTimeout:    5 * time.Second,
-		HeartbeatTimeout:       2 * time.Second,
-		ActivityID:             "id1",
-		TaskList:               tasklist,
-	}
-	ctx1 := WithActivityOptions(ctx, ao)
-	f := ExecuteActivity(ctx1, helloWorldAct)
-	var r1 string
-	err = f.Get(ctx, &r1)
-	if err != nil {
-		return "", err
-	}
-	return r1, nil
-}
-
 type key int
 
 const unitTestKey key = 1
 
-func (s *WorkflowUnitTest) Test_SingleActivityWorkflow() {
+func singleActivityWorkflowWithOptions(s *WorkflowUnitTest, ao ActivityOptions) error {
+	helloWorldActivityWorkflow := func(ctx Context, input string) (result string, err error) {
+		ctx1 := WithActivityOptions(ctx, ao)
+		f := ExecuteActivity(ctx1, helloWorldAct)
+		var r1 string
+		err = f.Get(ctx, &r1)
+		if err != nil {
+			return "", err
+		}
+		return r1, nil
+	}
+
 	env := newTestWorkflowEnv(s.T())
 	ctx := context.WithValue(context.Background(), unitTestKey, s)
 	env.SetWorkerOptions(WorkerOptions{BackgroundActivityContext: ctx})
 	env.RegisterActivity(helloWorldAct)
 	env.ExecuteWorkflow(helloWorldActivityWorkflow, "Hello")
 	s.True(env.IsWorkflowCompleted())
-	s.NoError(env.GetWorkflowError())
+	return env.GetWorkflowError()
+}
+
+func (s *WorkflowUnitTest) Test_SingleActivityWorkflow() {
+	ao := ActivityOptions{
+		ScheduleToStartTimeout: 10 * time.Second,
+		StartToCloseTimeout:    5 * time.Second,
+		HeartbeatTimeout:       2 * time.Second,
+		ActivityID:             "id1",
+		TaskList:               tasklist,
+	}
+	err := singleActivityWorkflowWithOptions(s, ao)
+	s.NoError(err)
+}
+
+func (s *WorkflowUnitTest) Test_SingleActivityWorkflowIsErrorMessagesMatched() {
+	testCases := []struct {
+		name                   string
+		ScheduleToStartTimeout time.Duration
+		StartToCloseTimeout    time.Duration
+		ScheduleToCloseTimeout time.Duration
+		expectedErrorMessage   string
+	}{
+		{
+			name:                   "ZeroScheduleToStartTimeout",
+			ScheduleToStartTimeout: 0 * time.Second,
+			StartToCloseTimeout:    5 * time.Second,
+			ScheduleToCloseTimeout: 0 * time.Second,
+			expectedErrorMessage:   "missing or negative ScheduleToStartTimeoutSeconds",
+		},
+		{
+			name:                   "ZeroStartToCloseTimeout",
+			ScheduleToStartTimeout: 10 * time.Second,
+			StartToCloseTimeout:    0 * time.Second,
+			ScheduleToCloseTimeout: 0 * time.Second,
+			expectedErrorMessage:   "missing or negative StartToCloseTimeoutSeconds",
+		},
+		{
+			name:                   "NegativeScheduleToCloseTimeout",
+			ScheduleToStartTimeout: 10 * time.Second,
+			StartToCloseTimeout:    5 * time.Second,
+			ScheduleToCloseTimeout: -1 * time.Second,
+			expectedErrorMessage:   "invalid negative ScheduleToCloseTimeoutSeconds",
+		},
+	}
+
+	for _, testCase := range testCases {
+		ao := ActivityOptions{
+			ScheduleToStartTimeout: testCase.ScheduleToStartTimeout,
+			StartToCloseTimeout:    testCase.StartToCloseTimeout,
+			ScheduleToCloseTimeout: testCase.ScheduleToCloseTimeout,
+			HeartbeatTimeout:       2 * time.Second,
+			ActivityID:             "id1",
+			TaskList:               tasklist,
+		}
+		s.Run(testCase.name, func() {
+			err := singleActivityWorkflowWithOptions(s, ao)
+			s.ErrorContains(err, testCase.expectedErrorMessage)
+		})
+	}
 }
 
 func splitJoinActivityWorkflow(ctx Context, testPanic bool) (result string, err error) {

internal/workflow.go

@@ -743,8 +743,7 @@ func (wc *workflowEnvironmentInterceptor) ExecuteActivity(ctx Context, typeName
 		return future
 	}
 	// Validate context options.
-	options := getActivityOptions(ctx)
-	options, err = getValidatedActivityOptions(ctx)
+	options, err := getValidatedActivityOptions(ctx)
 	if err != nil {
 		settable.Set(nil, err)
 		return future