Last bits of review comments

temporalio · May 25, 2023 · 4781e6e · 4781e6e
1 parent f3d24e2
commit 4781e6e
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 9 deletions.
diff --git a/internal/worker_version_sets.go b/internal/worker_version_sets.go
@@ -42,9 +42,9 @@ const (
 	// version if possible. It may not be possible if the target task queue does not also have
 	// knowledge of the current worker's build ID.
 	VersioningIntentCompatible
-	// VersioningIntentUseDefault indicates that the command should run on the target task queue's
+	// VersioningIntentDefault indicates that the command should run on the target task queue's
 	// current overall-default build ID.
-	VersioningIntentUseDefault
+	VersioningIntentDefault
 )
 
 type (
@@ -185,7 +185,7 @@ func (v *BuildIDOpPromoteIDWithinSet) targetedBuildId() string      { return v.B
 // the user's intent and whether the target task queue matches this worker's task queue.
 func determineUseCompatibleFlagForCommand(intent VersioningIntent, workerTq, TargetTq string) bool {
 	useCompat := true
-	if intent == VersioningIntentUseDefault {
+	if intent == VersioningIntentDefault {
 		useCompat = false
 	} else if intent == VersioningIntentUnspecified {
 		// If the target task queue doesn't match ours, use the default version

diff --git a/internal/worker_version_sets_test.go b/internal/worker_version_sets_test.go
@@ -63,3 +63,60 @@ func Test_WorkerVersionSets_fromProtoResponse(t *testing.T) {
 		})
 	}
 }
+
+func Test_VersioningIntent(t *testing.T) {
+	tests := []struct {
+		name                string
+		intent              VersioningIntent
+		tqSame              bool
+		shouldUseCompatible bool
+	}{
+		{
+			name:                "Unspecified same TQ",
+			intent:              VersioningIntentUnspecified,
+			tqSame:              true,
+			shouldUseCompatible: true,
+		},
+		{
+			name:                "Unspecified different TQ",
+			intent:              VersioningIntentUnspecified,
+			tqSame:              false,
+			shouldUseCompatible: false,
+		},
+		{
+			name:                "Default same TQ",
+			intent:              VersioningIntentDefault,
+			tqSame:              true,
+			shouldUseCompatible: false,
+		},
+		{
+			name:                "Default different TQ",
+			intent:              VersioningIntentDefault,
+			tqSame:              false,
+			shouldUseCompatible: false,
+		},
+		{
+			name:                "Compatible same TQ",
+			intent:              VersioningIntentCompatible,
+			tqSame:              true,
+			shouldUseCompatible: true,
+		},
+		{
+			name:                "Compatible different TQ",
+			intent:              VersioningIntentCompatible,
+			tqSame:              false,
+			shouldUseCompatible: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tqA := "a"
+			tqB := "b"
+			if tt.tqSame {
+				tqB = tqA
+			}
+			assert.Equal(t,
+				tt.shouldUseCompatible, determineUseCompatibleFlagForCommand(tt.intent, tqA, tqB))
+		})
+	}
+}
diff --git a/internal/workflow_test.go b/internal/workflow_test.go
@@ -56,8 +56,7 @@ func TestGetChildWorkflowOptions(t *testing.T) {
 			"foo": "bar",
 		},
 		ParentClosePolicy: enums.PARENT_CLOSE_POLICY_REQUEST_CANCEL,
-		// TODO(bergundy): test the defaults here with caller and destination same task queue / different task queue
-		VersioningIntent: VersioningIntentUseDefault,
+		VersioningIntent:  VersioningIntentDefault,
 	}
 
 	// Require test options to have non-zero value for each field. This ensures that we update tests (and the
@@ -78,8 +77,7 @@ func TestGetActivityOptions(t *testing.T) {
 		ActivityID:             "bar",
 		RetryPolicy:            newTestRetryPolicy(),
 		DisableEagerExecution:  true,
-		// TODO(bergundy): test the defaults here with caller and destination same task queue / different task queue
-		VersioningIntent: VersioningIntentUseDefault,
+		VersioningIntent:       VersioningIntentDefault,
 	}
 
 	assertNonZero(t, opts)

diff --git a/temporal/build_id_versioning.go b/temporal/build_id_versioning.go
@@ -40,8 +40,8 @@ const (
 	// knowledge of the current worker's build ID.
 	// WARNING: Worker versioning is currently experimental
 	VersioningIntentCompatible = internal.VersioningIntentCompatible
-	// VersioningIntentUseDefault indicates that the command should run on the target task queue's
+	// VersioningIntentDefault indicates that the command should run on the target task queue's
 	// current overall-default build ID.
 	// WARNING: Worker versioning is currently experimental
-	VersioningIntentUseDefault = internal.VersioningIntentUseDefault
+	VersioningIntentDefault = internal.VersioningIntentDefault
 )