adjust tests

Signed-off-by: Iaroslav Ciupin <[email protected]>
flyteorg · Aug 23, 2023 · 235e18e · 235e18e
1 parent 193a8ad
commit 235e18e
Show file tree

Hide file tree

Showing 35 changed files with 211 additions and 280 deletions.
diff --git a/pkg/clusterresource/impl/db_admin_data_provider.go b/pkg/clusterresource/impl/db_admin_data_provider.go
@@ -8,6 +8,7 @@ import (
 	"github.com/flyteorg/flyteadmin/pkg/clusterresource/interfaces"
 	"github.com/flyteorg/flyteadmin/pkg/common"
 	managerInterfaces "github.com/flyteorg/flyteadmin/pkg/manager/interfaces"
+	"github.com/flyteorg/flyteadmin/pkg/repositories/gormimpl"
 	repositoryInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces"
 	"github.com/flyteorg/flyteadmin/pkg/repositories/transformers"
 	runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces"
@@ -47,6 +48,13 @@ func (p dbAdminProvider) getDomains() []*admin.Domain {
 	return domains
 }
 
+var descCreatedAtSortParam = admin.Sort{
+	Direction: admin.Sort_DESCENDING,
+	Key:       "created_at",
+}
+
+var descCreatedAtSortDBParam, _ = common.NewSortParameter(&descCreatedAtSortParam, gormimpl.ProjectColumns)
+
 func (p dbAdminProvider) GetProjects(ctx context.Context) (*admin.Projects, error) {
 	filter, err := common.NewSingleValueFilter(common.Project, common.NotEqual, "state", int32(admin.Project_ARCHIVED))
 	if err != nil {

diff --git a/pkg/clusterresource/impl/db_admin_data_provider_test.go b/pkg/clusterresource/impl/db_admin_data_provider_test.go
@@ -3,13 +3,15 @@ package impl
 import (
 	"context"
 	"errors"
+	"strings"
 	"testing"
 
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
+	"github.com/flyteorg/flyteadmin/pkg/common"
 	"github.com/flyteorg/flyteadmin/pkg/manager/interfaces"
 	"github.com/flyteorg/flyteadmin/pkg/manager/mocks"
 	repoInterfaces "github.com/flyteorg/flyteadmin/pkg/repositories/interfaces"
@@ -105,7 +107,7 @@ func TestGetProjects(t *testing.T) {
 		mockRepo.(*repoMocks.MockRepository).ProjectRepoIface = &repoMocks.MockProjectRepo{
 			ListProjectsFunction: func(ctx context.Context, input repoInterfaces.ListResourceInput) ([]models.Project, error) {
 				assert.Len(t, input.InlineFilters, 1)
-				assert.Equal(t, input.SortParameters.GetGormOrderExpr(), "created_at desc")
+				assert.Equal(t, "created_at desc", sortParamsSQL(input.SortParameters))
 				return []models.Project{
 					{
 						Identifier: "flytesnacks",
@@ -142,3 +144,11 @@ func TestGetProjects(t *testing.T) {
 		assert.EqualError(t, err, errFoo.Error())
 	})
 }
+
+func sortParamsSQL(params []common.SortParameter) string {
+	sqls := make([]string, len(params))
+	for i, param := range params {
+		sqls[i] = param.GetGormOrderExpr()
+	}
+	return strings.Join(sqls, ", ")
+}
diff --git a/pkg/clusterresource/impl/shared.go b/pkg/clusterresource/impl/shared.go
@@ -1,20 +1,11 @@
 package impl
 
 import (
-	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
 	"google.golang.org/grpc/codes"
 
-	"github.com/flyteorg/flyteadmin/pkg/common"
 	"github.com/flyteorg/flyteadmin/pkg/errors"
 )
 
 func NewMissingEntityError(entity string) error {
 	return errors.NewFlyteAdminErrorf(codes.NotFound, "Failed to find [%s]", entity)
 }
-
-var descCreatedAtSortParam = admin.Sort{
-	Direction: admin.Sort_DESCENDING,
-	Key:       "created_at",
-}
-
-var descCreatedAtSortDBParam, _ = common.NewSortParameter(descCreatedAtSortParam)
diff --git a/pkg/common/sorting.go b/pkg/common/sorting.go
@@ -27,16 +27,21 @@ func (s *sortParamImpl) GetGormOrderExpr() string {
 }
 
 func NewSortParameter(sort *admin.Sort, allowed sets.String) ([]SortParameter, error) {
-	if !allowed.Has(sort.Key) {
-		return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid sort_key: %s", sort.Key)
+	if sort == nil {
+		return nil, nil
+	}
+
+	key := sort.GetKey()
+	if !allowed.Has(key) {
+		return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid sort_key: %s", key)
 	}
 
 	var gormOrderExpression string
-	switch sort.Direction {
+	switch sort.GetDirection() {
 	case admin.Sort_DESCENDING:
-		gormOrderExpression = fmt.Sprintf(gormDescending, sort.Key)
+		gormOrderExpression = fmt.Sprintf(gormDescending, key)
 	case admin.Sort_ASCENDING:
-		gormOrderExpression = fmt.Sprintf(gormAscending, sort.Key)
+		gormOrderExpression = fmt.Sprintf(gormAscending, key)
 	default:
 		return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid sort order specified: %v", sort)
 	}

diff --git a/pkg/common/sorting_test.go b/pkg/common/sorting_test.go
@@ -3,24 +3,26 @@ package common
 import (
 	"testing"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestSortParameter_Ascending(t *testing.T) {
-	sortParameter, err := NewSortParameter(admin.Sort{
+	sortParameter, err := NewSortParameter(&admin.Sort{
 		Direction: admin.Sort_ASCENDING,
 		Key:       "name",
-	})
+	}, sets.NewString("name"))
 	assert.Nil(t, err)
-	assert.Equal(t, "name asc", sortParameter.GetGormOrderExpr())
+	assert.Equal(t, "name asc", sortParameter[0].GetGormOrderExpr())
 }
 
 func TestSortParameter_Descending(t *testing.T) {
-	sortParameter, err := NewSortParameter(admin.Sort{
+	sortParameter, err := NewSortParameter(&admin.Sort{
 		Direction: admin.Sort_DESCENDING,
 		Key:       "project",
-	})
+	}, sets.NewString("project"))
 	assert.Nil(t, err)
-	assert.Equal(t, "project desc", sortParameter.GetGormOrderExpr())
+	assert.Equal(t, "project desc", sortParameter[0].GetGormOrderExpr())
 }
diff --git a/pkg/manager/impl/description_entity_manager_test.go b/pkg/manager/impl/description_entity_manager_test.go
@@ -4,15 +4,16 @@ import (
 	"context"
 	"testing"
 
+	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
+	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
+	mockScope "github.com/flyteorg/flytestdlib/promutils"
+	"github.com/stretchr/testify/assert"
+
 	"github.com/flyteorg/flyteadmin/pkg/manager/impl/testutils"
 	"github.com/flyteorg/flyteadmin/pkg/repositories/interfaces"
 	repositoryMocks "github.com/flyteorg/flyteadmin/pkg/repositories/mocks"
 	runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces"
 	runtimeMocks "github.com/flyteorg/flyteadmin/pkg/runtime/mocks"
-	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
-	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
-	mockScope "github.com/flyteorg/flytestdlib/promutils"
-	"github.com/stretchr/testify/assert"
 )
 
 var descriptionEntityIdentifier = core.Identifier{

diff --git a/pkg/manager/impl/execution_manager_test.go b/pkg/manager/impl/execution_manager_test.go
@@ -3,67 +3,54 @@ package impl
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"testing"
-
-	"github.com/flyteorg/flyteadmin/plugins"
-
-	"google.golang.org/grpc/status"
-
-	"google.golang.org/protobuf/types/known/timestamppb"
+	"time"
 
 	"github.com/benbjohnson/clock"
 	"github.com/flyteorg/flyteidl/clients/go/coreutils"
+	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
+	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
 	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/event"
+	mockScope "github.com/flyteorg/flytestdlib/promutils"
+	"github.com/flyteorg/flytestdlib/storage"
 	"github.com/gogo/protobuf/jsonpb"
+	"github.com/golang/protobuf/proto"
 	"github.com/golang/protobuf/ptypes"
+	"github.com/golang/protobuf/ptypes/wrappers"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/types/known/timestamppb"
+	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/apimachinery/pkg/util/sets"
 
+	"github.com/flyteorg/flyteadmin/auth"
+	eventWriterMocks "github.com/flyteorg/flyteadmin/pkg/async/events/mocks"
+	notificationMocks "github.com/flyteorg/flyteadmin/pkg/async/notifications/mocks"
 	"github.com/flyteorg/flyteadmin/pkg/common"
+	commonMocks "github.com/flyteorg/flyteadmin/pkg/common/mocks"
 	commonTestUtils "github.com/flyteorg/flyteadmin/pkg/common/testutils"
+	dataMocks "github.com/flyteorg/flyteadmin/pkg/data/mocks"
 	flyteAdminErrors "github.com/flyteorg/flyteadmin/pkg/errors"
 	"github.com/flyteorg/flyteadmin/pkg/manager/impl/executions"
 	"github.com/flyteorg/flyteadmin/pkg/manager/impl/shared"
+	"github.com/flyteorg/flyteadmin/pkg/manager/impl/testutils"
 	managerInterfaces "github.com/flyteorg/flyteadmin/pkg/manager/interfaces"
 	managerMocks "github.com/flyteorg/flyteadmin/pkg/manager/mocks"
-	"github.com/flyteorg/flyteadmin/pkg/runtime"
-
-	"k8s.io/apimachinery/pkg/api/resource"
-
-	"k8s.io/apimachinery/pkg/util/sets"
-
-	eventWriterMocks "github.com/flyteorg/flyteadmin/pkg/async/events/mocks"
-
-	"github.com/flyteorg/flyteadmin/auth"
-
-	commonMocks "github.com/flyteorg/flyteadmin/pkg/common/mocks"
-
-	"github.com/flyteorg/flytestdlib/storage"
-
-	"time"
-
-	"fmt"
-
-	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/admin"
-	"github.com/flyteorg/flyteidl/gen/pb-go/flyteidl/core"
-	mockScope "github.com/flyteorg/flytestdlib/promutils"
-	"github.com/golang/protobuf/proto"
-	"github.com/golang/protobuf/ptypes/wrappers"
-	"github.com/stretchr/testify/assert"
-
-	notificationMocks "github.com/flyteorg/flyteadmin/pkg/async/notifications/mocks"
-	dataMocks "github.com/flyteorg/flyteadmin/pkg/data/mocks"
-	"github.com/flyteorg/flyteadmin/pkg/manager/impl/testutils"
 	"github.com/flyteorg/flyteadmin/pkg/repositories/interfaces"
 	repositoryMocks "github.com/flyteorg/flyteadmin/pkg/repositories/mocks"
 	"github.com/flyteorg/flyteadmin/pkg/repositories/models"
 	"github.com/flyteorg/flyteadmin/pkg/repositories/transformers"
+	"github.com/flyteorg/flyteadmin/pkg/runtime"
 	runtimeInterfaces "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces"
 	runtimeIFaceMocks "github.com/flyteorg/flyteadmin/pkg/runtime/interfaces/mocks"
 	runtimeMocks "github.com/flyteorg/flyteadmin/pkg/runtime/mocks"
 	workflowengineInterfaces "github.com/flyteorg/flyteadmin/pkg/workflowengine/interfaces"
 	workflowengineMocks "github.com/flyteorg/flyteadmin/pkg/workflowengine/mocks"
+	"github.com/flyteorg/flyteadmin/plugins"
 )
 
 var spec = testutils.GetExecutionRequest().Spec
@@ -2982,7 +2969,7 @@ func TestListExecutions(t *testing.T) {
 		assert.True(t, domainFilter, "Missing domain equality filter")
 		assert.False(t, nameFilter, "Included name equality filter")
 		assert.Equal(t, limit, input.Limit)
-		assert.Equal(t, "domain asc", input.SortParameters.GetGormOrderExpr())
+		assert.Equal(t, "execution_domain asc", sortParamsSQL(input.SortParameters))
 		assert.Equal(t, 2, input.Offset)
 		assert.EqualValues(t, map[common.Entity]bool{
 			common.Execution: true,
@@ -3030,7 +3017,7 @@ func TestListExecutions(t *testing.T) {
 		Limit: limit,
 		SortBy: &admin.Sort{
 			Direction: admin.Sort_ASCENDING,
-			Key:       "domain",
+			Key:       "execution_domain",
 		},
 		Token: "2",
 	})
@@ -3968,7 +3955,7 @@ func TestListExecutions_LegacyModel(t *testing.T) {
 		assert.True(t, domainFilter, "Missing domain equality filter")
 		assert.False(t, nameFilter, "Included name equality filter")
 		assert.Equal(t, limit, input.Limit)
-		assert.Equal(t, "domain asc", input.SortParameters.GetGormOrderExpr())
+		assert.Equal(t, "execution_domain asc", sortParamsSQL(input.SortParameters))
 		assert.Equal(t, 2, input.Offset)
 		return interfaces.ExecutionCollectionOutput{
 			Executions: []models.Execution{
@@ -4013,7 +4000,7 @@ func TestListExecutions_LegacyModel(t *testing.T) {
 		Limit: limit,
 		SortBy: &admin.Sort{
 			Direction: admin.Sort_ASCENDING,
-			Key:       "domain",
+			Key:       "execution_domain",
 		},
 		Token: "2",
 	})

diff --git a/pkg/manager/impl/launch_plan_manager_test.go b/pkg/manager/impl/launch_plan_manager_test.go
@@ -1098,7 +1098,7 @@ func TestLaunchPlanManager_ListLaunchPlans(t *testing.T) {
 		assert.True(t, nameFilter, "Missing name equality filter")
 		assert.Equal(t, 10, input.Limit)
 		assert.Equal(t, 2, input.Offset)
-		assert.Equal(t, "domain asc", input.SortParameters.GetGormOrderExpr())
+		assert.Equal(t, "domain asc", sortParamsSQL(input.SortParameters))
 
 		return interfaces.LaunchPlanCollectionOutput{
 			LaunchPlans: []models.LaunchPlan{
@@ -1195,7 +1195,7 @@ func TestLaunchPlanManager_ListLaunchPlanIds(t *testing.T) {
 		assert.True(t, projectFilter, "Missing project equality filter")
 		assert.True(t, domainFilter, "Missing domain equality filter")
 		assert.Equal(t, 10, input.Limit)
-		assert.Equal(t, "domain asc", input.SortParameters.GetGormOrderExpr())
+		assert.Equal(t, "domain asc", sortParamsSQL(input.SortParameters))
 
 		return interfaces.LaunchPlanCollectionOutput{
 			LaunchPlans: []models.LaunchPlan{
@@ -1282,7 +1282,7 @@ func TestLaunchPlanManager_ListActiveLaunchPlans(t *testing.T) {
 		assert.True(t, domainFilter, "Missing domain equality filter")
 		assert.True(t, activeFilter, "Missing active filter")
 		assert.Equal(t, 10, input.Limit)
-		assert.Equal(t, "domain asc", input.SortParameters.GetGormOrderExpr())
+		assert.Equal(t, "domain asc", sortParamsSQL(input.SortParameters))
 
 		return interfaces.LaunchPlanCollectionOutput{
 			LaunchPlans: []models.LaunchPlan{

diff --git a/pkg/manager/impl/node_execution_manager_test.go b/pkg/manager/impl/node_execution_manager_test.go
@@ -809,7 +809,7 @@ func TestListNodeExecutionsLevelZero(t *testing.T) {
 				"parent_task_execution_id": nil,
 			}, filter)
 
-			assert.Equal(t, "domain asc", input.SortParameters.GetGormOrderExpr())
+			assert.Equal(t, "execution_domain asc", sortParamsSQL(input.SortParameters))
 			return interfaces.NodeExecutionCollectionOutput{
 				NodeExecutions: []models.NodeExecution{
 					{
@@ -860,10 +860,10 @@ func TestListNodeExecutionsLevelZero(t *testing.T) {
 		Token: "2",
 		SortBy: &admin.Sort{
 			Direction: admin.Sort_ASCENDING,
-			Key:       "domain",
+			Key:       "execution_domain",
 		},
 	})
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Len(t, nodeExecutions.NodeExecutions, 1)
 	assert.True(t, proto.Equal(&admin.NodeExecution{
 		Id: &core.NodeExecutionIdentifier{
@@ -927,7 +927,7 @@ func TestListNodeExecutionsWithParent(t *testing.T) {
 			assert.Equal(t, parentID, queryExpr.Args)
 			assert.Equal(t, "parent_id = ?", queryExpr.Query)
 
-			assert.Equal(t, "domain asc", input.SortParameters.GetGormOrderExpr())
+			assert.Equal(t, "execution_domain asc", sortParamsSQL(input.SortParameters))
 			return interfaces.NodeExecutionCollectionOutput{
 				NodeExecutions: []models.NodeExecution{
 					{
@@ -960,7 +960,7 @@ func TestListNodeExecutionsWithParent(t *testing.T) {
 		Token: "2",
 		SortBy: &admin.Sort{
 			Direction: admin.Sort_ASCENDING,
-			Key:       "domain",
+			Key:       "execution_domain",
 		},
 		UniqueParentId: "parent_1",
 	})
@@ -1087,7 +1087,7 @@ func TestListNodeExecutions_NothingToReturn(t *testing.T) {
 		Token: "2",
 		SortBy: &admin.Sort{
 			Direction: admin.Sort_ASCENDING,
-			Key:       "domain",
+			Key:       "execution_domain",
 		},
 	})
 	assert.Nil(t, err)
@@ -1141,7 +1141,7 @@ func TestListNodeExecutionsForTask(t *testing.T) {
 			assert.Equal(t, uint(8), queryExpr.Args)
 			assert.Equal(t, "parent_task_execution_id = ?", queryExpr.Query)
 
-			assert.Equal(t, "domain asc", input.SortParameters.GetGormOrderExpr())
+			assert.Equal(t, "execution_domain asc", sortParamsSQL(input.SortParameters))
 			return interfaces.NodeExecutionCollectionOutput{
 				NodeExecutions: []models.NodeExecution{
 					{
@@ -1186,7 +1186,7 @@ func TestListNodeExecutionsForTask(t *testing.T) {
 		Token: "2",
 		SortBy: &admin.Sort{
 			Direction: admin.Sort_ASCENDING,
-			Key:       "domain",
+			Key:       "execution_domain",
 		},
 	})
 	assert.Nil(t, err)

diff --git a/pkg/manager/impl/project_manager_test.go b/pkg/manager/impl/project_manager_test.go
@@ -56,7 +56,7 @@ func testListProjects(request admin.ProjectListRequest, token string, orderExpr
 			q, _ := input.InlineFilters[0].GetGormQueryExpr()
 			assert.Equal(t, *queryExpr, q)
 		}
-		assert.Equal(t, orderExpr, input.SortParameters.GetGormOrderExpr())
+		assert.Equal(t, orderExpr, sortParamsSQL(input.SortParameters))
 		activeState := int32(admin.Project_ACTIVE)
 		return []models.Project{
 			{