Add unit test for add tag (#4185)

Signed-off-by: Da-Yi Wu <[email protected]>

flyteadmin/pkg/manager/impl/execution_manager.go

@@ -1346,7 +1346,7 @@ func (m *ExecutionManager) UpdateExecution(ctx context.Context, request admin.Ex
 		return nil, err
 	}
 
-	if err = transformers.UpdateExecutionModelTag(executionModel, request.Tags); err != nil {
+	if err = transformers.UpdateExecutionModelAddTag(executionModel, request.AddTags); err != nil {
 		return nil, err
 	}
 

flyteadmin/pkg/manager/impl/execution_manager_test.go

@@ -2937,7 +2937,7 @@ func TestUpdateExecution(t *testing.T) {
 		execManager := NewExecutionManager(repository, r, getMockExecutionsConfigProvider(), getMockStorageForExecTest(context.Background()), mockScope.NewTestScope(), mockScope.NewTestScope(), &mockPublisher, mockExecutionRemoteURL, nil, nil, nil, nil, &eventWriterMocks.WorkflowExecutionEventWriter{})
 		updateResponse, err := execManager.UpdateExecution(context.Background(), admin.ExecutionUpdateRequest{
 			Id:   &executionIdentifier,
-			Tags: []string{"tag1", "tag2"},
+			AddTags: []string{"tag1", "tag2"},
 		}, time.Now())
 		assert.NoError(t, err)
 		assert.NotNil(t, updateResponse)

flyteadmin/pkg/repositories/transformers/execution.go

@@ -293,7 +293,7 @@ func UpdateExecutionModelStateChangeDetails(executionModel *models.Execution, st
 }
 
 // Update tag information of existing execution model.
-func UpdateExecutionModelTag(executionModel *models.Execution, tagsUpdatedTo []string) error {
+func UpdateExecutionModelAddTag(executionModel *models.Execution, addTags []string) error {
 
 	var spec admin.ExecutionSpec
 	var err error
@@ -310,7 +310,11 @@ func UpdateExecutionModelTag(executionModel *models.Execution, tagsUpdatedTo []s
 	for _, tag := range spec.Tags {
 		tagSet.Insert(tag)
 	}
-	for _, tag := range tagsUpdatedTo {
+	for _, tag := range addTags {
+		// if len(tag) == 0, skip
+		if len(tag) == 0 {
+			continue
+		}
 		// if tag not in tagSet, append into executionModel.Tags
 		if !tagSet.Has(tag) {
 			executionModel.Tags = append(executionModel.Tags, models.AdminTag{Name: tag})

flyteadmin/pkg/repositories/transformers/execution_test.go

@@ -901,6 +901,73 @@ func TestUpdateExecutionModelStateChangeDetails(t *testing.T) {
 	})
 }
 
+func TestUpdateExecutionModelAddTags(t *testing.T) {
+	t.Run("empty closure", func(t *testing.T) {
+		execModel := &models.Execution{}
+		tags := []string{"tags1", "tags2"}
+
+		err := UpdateExecutionModelAddTag(execModel, tags)
+		assert.Nil(t, err)
+		execTags := execModel.Tags
+		// Extract string of execTags
+		var execTagsString []string
+		for _, tag := range execTags {
+			execTagsString = append(execTagsString, tag.Name)
+		}
+		assert.Equal(t, tags, execTagsString)
+
+		var spec admin.ExecutionSpec
+		err = proto.Unmarshal(execModel.Spec, &spec)
+		assert.Nil(t, err)
+		assert.NotNil(t, spec)
+		assert.Equal(t, tags, spec.Tags)
+	})
+
+	t.Run("empty add tags", func(t *testing.T) {
+		execModel := &models.Execution{}
+		execModel.Tags = []models.AdminTag{
+			{Name: "tags1",},
+			{Name: "tags2",},
+		}
+		ref_tags := []string{"tags1", "tags2"}
+		tags := []string{""}
+
+		err := UpdateExecutionModelAddTag(execModel, tags)
+		assert.Nil(t, err)
+		var execTagsString []string
+		for _, tag := range execModel.Tags {
+			execTagsString = append(execTagsString, tag.Name)
+		}
+		assert.Nil(t, err)
+		assert.Equal(t, ref_tags, execTagsString)
+	})
+	t.Run("duplicate tags", func(t *testing.T) {
+		spec := testutils.GetExecutionRequest().Spec
+		spec.Tags = []string{"tags1", "tags2"}
+		specBytes, _ := proto.Marshal(spec)
+
+		execModel := &models.Execution{
+			Spec:    specBytes,
+		}
+
+		execModel.Tags = []models.AdminTag{
+			{Name: "tags1",},
+			{Name: "tags2",},
+		}
+
+		tags := []string{"tags2", "tags3"}
+		err := UpdateExecutionModelAddTag(execModel, tags)
+		assert.Nil(t, err)
+
+		var execTagsString []string
+		for _, tag := range execModel.Tags {
+			execTagsString = append(execTagsString, tag.Name)
+		}
+		assert.Nil(t, err)
+		assert.Equal(t, []string{"tags1", "tags2", "tags3"}, execTagsString)
+	})
+}
+
 func TestTrimErrorMessage(t *testing.T) {
 	errMsg := "[1/1] currentAttempt done. Last Error: USER::                   │\n│ ❱  760 │   │   │   │   return __callback(*args, **kwargs)                    │\n││"
 	trimmedErrMessage := TrimErrorMessage(errMsg)