Apply suggestions from code review

Co-authored-by: Steven L <[email protected]>

Author: arzonus

internal/internal_workflow_testsuite_test.go

@@ -1414,10 +1414,8 @@ func (s *WorkflowTestSuiteUnitTest) Test_MockGetVersion_ExecuteWithVersion() {
 	env.RegisterActivity(oldActivity)
 	env.RegisterActivity(newActivity)
 
-	env.OnGetVersion("change_1", DefaultVersion, 2).Return(func(string, Version, Version) Version {
-		return DefaultVersion
-	})
-	env.OnGetVersion(mock.Anything, DefaultVersion, 2).Return(Version(2))
+	env.OnGetVersion("change_1", DefaultVersion, 2).Return(DefaultVersion)
+	env.OnGetVersion("change_2", DefaultVersion, 2).Return(Version(2))
 	env.ExecuteWorkflow(workflowFn)
 
 	s.True(env.IsWorkflowCompleted())

internal/workflow_testsuite.go

@@ -366,7 +366,6 @@ func (t *TestWorkflowEnvironment) OnRequestCancelExternalWorkflow(domainName, wo
 
 // OnGetVersion setup a mock for workflow.GetVersion() call. By default, if mock is not setup, the GetVersion call from
 // workflow code will always return the maxSupported version or version specified by GetVersionOption.
-// Make it not possible to test old version branch. With this mock support, it is possible to test code branch for different versions.
 //
 // Note: mock can be setup for a specific changeID. Or if mock.Anything is used as changeID then all calls to GetVersion
 // will be mocked. Mock for a specific changeID has higher priority over mock.Anything.

workflow/workflow.go

@@ -368,7 +368,7 @@ const DefaultVersion Version = internal.DefaultVersion
 // Following the steps below, your changes will be forward and backward compatible, keeping possible a safe rollback
 // to the previous version of the workflow code:
 //
-// 1. Keep execution of foo activity, add a support of bar activity
+// 1. Keep execution of foo activity, add support for bar activity
 //
 //	v := GetVersion(ctx, "fooChange", DefaultVersion, 1,  ExecuteWithVersion(DefaultVersion))
 //	if v == DefaultVersion {
@@ -378,8 +378,9 @@ const DefaultVersion Version = internal.DefaultVersion
 //	}
 //
 // The code above supports replaying of workflow execution with both versions DefaultVersion and 1.
-// All new workflow executions will execute foo activity, because GetVersion with ExecuteWithMinVersion option
-// returns DefaultVersion, that will be recorded into the workflow history.
+// All new workflow executions will execute foo activity, because ExecuteWithVersion(DefaultVersion)
+// causes GetVersion to return DefaultVersion rather than 1.
+// In this example, this is also exactly the same as using ExecuteWithMinVersion().
 //
 // 2. Enable execution of bar activity
 //
@@ -392,16 +393,14 @@ const DefaultVersion Version = internal.DefaultVersion
 //
 // The code above supports replaying of workflow execution with both versions DefaultVersion and 1.
 // All new workflow executions will execute bar activity, because
-// GetVersion returns the maximum supported version - 1, that will be recorded into the workflow history.
+// GetVersion returns the maximum supported version (1), which will be recorded into the workflow history.
 //
 // 3. Remove a support of foo activity:
 //
 //	GetVersion(ctx, "fooChange", 1, 1)
 //	err = workflow.ExecuteActivity(ctx, bar).Get(ctx, nil)
 //
 // When there are no workflow executions running DefaultVersion the support of foo activity can be removed.
-//
-// ExecuteWithVersion option is useful when you want to ensure that your changes can be safely rolled back if needed.
 func ExecuteWithVersion(version Version) internal.GetVersionOptions {
 	return internal.ExecuteWithVersion(version)
 }