Simplify test to min-repro, add comment explaining solution

Author: yuandrew

internal/internal_workflow_testsuite.go

@@ -2281,23 +2281,20 @@ func (env *testWorkflowEnvironmentImpl) RequestCancelChildWorkflow(_, workflowID
 
 func (env *testWorkflowEnvironmentImpl) RequestCancelExternalWorkflow(namespace, workflowID, runID string, callback ResultHandler) {
 	if env.workflowInfo.WorkflowExecution.ID == workflowID {
-		// cancel current workflow from within workflow context
-		if sd, ok := env.workflowDef.(*syncWorkflowDefinition); ok {
-			env.postCallback(func() {
-				sd.dispatcher.NewCoroutine(sd.rootCtx, "cancel-self", true, func(ctx Context) {
-					env.workflowCancelHandler()
-				})
-				if env.isChildWorkflow() && env.onChildWorkflowCanceledListener != nil {
-					env.onChildWorkflowCanceledListener(env.workflowInfo)
-				}
-			}, true)
+		// The way testWorkflowEnvironment is setup today, we close the child workflow dispatcher before calling
+		// the workflowCancelHandler. A larger refactor would be needed to handle this similar to non-test code.
+		// Maybe worth doing when https://github.com/temporalio/go-sdk/issues/50 is tackled.
+		if sd, ok := env.workflowDef.(*syncWorkflowDefinition); ok && env.isChildWorkflow() {
+			sd.dispatcher.NewCoroutine(sd.rootCtx, "cancel-self", true, func(ctx Context) {
+				env.workflowCancelHandler()
+			})
 		} else {
 			env.workflowCancelHandler()
-			if env.isChildWorkflow() && env.onChildWorkflowCanceledListener != nil {
-				env.postCallback(func() {
-					env.onChildWorkflowCanceledListener(env.workflowInfo)
-				}, false)
-			}
+		}
+		if env.isChildWorkflow() && env.onChildWorkflowCanceledListener != nil {
+			env.postCallback(func() {
+				env.onChildWorkflowCanceledListener(env.workflowInfo)
+			}, false)
 		}
 		return
 	} else if childHandle, ok := env.runningWorkflows[workflowID]; ok && !childHandle.handled {

internal/workflow_testsuite_test.go

@@ -1238,53 +1238,30 @@ func SleepHour(ctx Context) error {
 }
 
 func SleepThenCancel(ctx Context) error {
-	selector := NewSelector(ctx)
-	var activationWorkflow *WorkflowExecution
-	selector.AddReceive(GetSignalChannel(ctx, "activate"), func(c ReceiveChannel, more bool) {
-		c.Receive(ctx, nil)
-		GetLogger(ctx).Info("Received activation signal")
-		if activationWorkflow != nil {
-			RequestCancelExternalWorkflow(ctx, activationWorkflow.ID, activationWorkflow.RunID)
-		}
-
-		cwf := ExecuteChildWorkflow(
-			ctx,
-			SleepHour,
-		)
-
-		var res WorkflowExecution
-		if err := cwf.GetChildWorkflowExecution().Get(ctx, &res); err != nil {
-			GetLogger(ctx).Error("Failed to start child workflow", "error", err)
-			return
-		}
-		activationWorkflow = &res
-
-		selector.AddFuture(cwf, func(f Future) {
-			if err := f.Get(ctx, nil); err != nil {
-				GetLogger(ctx).Error("Child workflow failed", "error", err)
-			} else {
-				GetLogger(ctx).Info("Child workflow completed successfully")
-			}
-			activationWorkflow = nil
-		})
-	})
+	cwf := ExecuteChildWorkflow(
+		ctx,
+		SleepHour,
+	)
+	var res WorkflowExecution
+	if err := cwf.GetChildWorkflowExecution().Get(ctx, &res); err != nil {
+		return err
+	}
 
-	for selector.HasPending() || activationWorkflow != nil {
-		selector.Select(ctx)
+	// Canceling an external workflow that causes a timer to cancel used to fail due to
+	// "illegal access from outside of workflow context"
+	err := RequestCancelExternalWorkflow(ctx, res.ID, res.RunID).Get(ctx, nil)
+	if err != nil {
+		return err
 	}
-	return nil
+
+	// Give the workflow time to finish canceling the child workflow
+	return Sleep(ctx, 1*time.Second)
 }
 
 func TestRequestCancelExternalWorkflowInSelector(t *testing.T) {
 	testSuite := &WorkflowTestSuite{}
 	env := testSuite.NewTestWorkflowEnvironment()
 	env.RegisterWorkflow(SleepHour)
-	env.RegisterDelayedCallback(func() {
-		env.SignalWorkflow("activate", nil)
-	}, 0)
-	env.RegisterDelayedCallback(func() {
-		env.SignalWorkflow("activate", nil)
-	}, time.Second)
 	env.ExecuteWorkflow(SleepThenCancel)
 	require.NoError(t, env.GetWorkflowError())
 	env.IsWorkflowCompleted()