Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve resource manager test coverage #5973

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Improve resource manager test coverage #5973

Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a1065fc
GetWorkflowAttributes error test case
luckyarthur Nov 7, 2024
58edd9d
add UpdateWorkflowAttributes error with no attributes test case
luckyarthur Nov 7, 2024
7dfc14e
add DeleteWorkflowAttributes error test case
luckyarthur Nov 8, 2024
e7417ef
add DeleteProjectAttributes error test cases
luckyarthur Nov 8, 2024
ace396e
add UpdateProjectDomainAttributes error test case
luckyarthur Nov 8, 2024
b08a22b
add GetProjectDomainAttributes error test case
luckyarthur Nov 8, 2024
7417f4f
add DeleteProjectDomainAttributes error test case
luckyarthur Nov 8, 2024
a0c98bf
add ListAll error test case
luckyarthur Nov 8, 2024
fcd04bb
add check if the UpdateWorkflowAttributes return the FlyteAdminError
luckyarthur Nov 12, 2024
17dc908
add check if GetWorkflowAttributes return FlyteAdminError type
luckyarthur Nov 12, 2024
abc2fcd
add check of error type for validation and Delete Function error
luckyarthur Nov 12, 2024
b29140f
add error message assert for DeleteWorkflowAttributes validate and de…
luckyarthur Nov 12, 2024
2d0dc02
add error message assert for DeleteProjectAttributes validate and del…
luckyarthur Nov 12, 2024
719e19d
add error message assert for GetWorkflowAttributes validate and Get f…
luckyarthur Nov 12, 2024
f170599
add error message check for UpdateWorkflowAttributes domain error
luckyarthur Nov 12, 2024
3422feb
add validate and empty request attribute instance error type check an…
luckyarthur Nov 12, 2024
ef026eb
add validate and project domain error type and error message check
luckyarthur Nov 12, 2024
bd5998e
add delete function fail and validate error type and message check fo…
luckyarthur Nov 12, 2024
0372f31
add resource error type and message check for ListAll test
luckyarthur Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 207 additions & 0 deletions flyteadmin/pkg/manager/impl/resources/resource_manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ func TestUpdateWorkflowAttributes(t *testing.T) {
_, err := manager.UpdateWorkflowAttributes(context.Background(), request)
assert.Nil(t, err)
assert.True(t, createOrUpdateCalled)

request = &admin.WorkflowAttributesUpdateRequest{
Attributes: &admin.WorkflowAttributes{},
}
_, failError := manager.UpdateWorkflowAttributes(context.Background(), request)
assert.Error(t, failError)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also make sure it returns the right error?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and other similar places

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all new added error test cases have been error type and error message checked in the code

var newError errors.FlyteAdminError
assert.ErrorAs(t, failError, &newError)
assert.Equal(t, newError.Error(), "domain [] is unrecognized by system")
}

func TestUpdateWorkflowAttributes_CreateOrMerge(t *testing.T) {
Expand Down Expand Up @@ -181,6 +190,40 @@ func TestGetWorkflowAttributes(t *testing.T) {
MatchingAttributes: testutils.ExecutionQueueAttributes,
},
}, response))

db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = func(
ctx context.Context, projectID string) (models.Project, error) {
return models.Project{}, errors.NewFlyteAdminError(codes.NotFound, "validationError")
}

_, validationError := manager.GetWorkflowAttributes(context.Background(), request)
assert.Error(t, validationError)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

var newError errors.FlyteAdminError
assert.ErrorAs(t, validationError, &newError)
assert.Equal(t, newError.Error(), "failed to validate that project [project] and domain [domain] are registered, err: [validationError]")

db.ResourceRepo().(*mocks.MockResourceRepo).GetFunction = func(
ctx context.Context, ID repoInterfaces.ResourceID) (models.Resource, error) {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, workflow, ID.Workflow)
assert.Equal(t, admin.MatchableResource_EXECUTION_QUEUE.String(), ID.ResourceType)
expectedSerializedAttrs, _ := proto.Marshal(testutils.ExecutionQueueAttributes)
return models.Resource{
Project: project,
Domain: domain,
Workflow: workflow,
ResourceType: "resource",
Attributes: expectedSerializedAttrs,
}, errors.NewFlyteAdminError(codes.NotFound, "workflowAttributesModelError")
}
db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = mocks.NewMockRepository().ProjectRepo().(*mocks.MockProjectRepo).GetFunction

_, failError := manager.GetWorkflowAttributes(context.Background(), request)
assert.Error(t, failError)
var secondError errors.FlyteAdminError
assert.ErrorAs(t, failError, &secondError)
assert.Equal(t, secondError.Error(), "workflowAttributesModelError")
}

func TestDeleteWorkflowAttributes(t *testing.T) {
Expand All @@ -202,6 +245,33 @@ func TestDeleteWorkflowAttributes(t *testing.T) {
manager := NewResourceManager(db, testutils.GetApplicationConfigWithDefaultDomains())
_, err := manager.DeleteWorkflowAttributes(context.Background(), request)
assert.Nil(t, err)

db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = func(
ctx context.Context, projectID string) (models.Project, error) {
return models.Project{}, errors.NewFlyteAdminError(codes.NotFound, "validationError")
}
_, validationError := manager.DeleteWorkflowAttributes(context.Background(), request)
assert.Error(t, validationError)
var newError errors.FlyteAdminError
assert.ErrorAs(t, validationError, &newError)
assert.Equal(t, newError.Error(), "failed to validate that project [project] and domain [domain] are registered, err: [validationError]")

db.ResourceRepo().(*mocks.MockResourceRepo).DeleteFunction = func(
ctx context.Context, ID repoInterfaces.ResourceID) error {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, workflow, ID.Workflow)
assert.Equal(t, admin.MatchableResource_EXECUTION_QUEUE.String(), ID.ResourceType)
return errors.NewFlyteAdminError(codes.NotFound, "deleteError")
}
//cause we use reference of ProjectRepo GetFunction, need to reset to default GetFunction
db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = mocks.NewMockRepository().ProjectRepo().(*mocks.MockProjectRepo).GetFunction

_, failError := manager.DeleteWorkflowAttributes(context.Background(), request)
assert.Error(t, failError)
var secondError errors.FlyteAdminError
assert.ErrorAs(t, failError, &secondError)
assert.Equal(t, secondError.Error(), "deleteError")
}

func TestUpdateProjectDomainAttributes(t *testing.T) {
Expand Down Expand Up @@ -229,6 +299,27 @@ func TestUpdateProjectDomainAttributes(t *testing.T) {
_, err := manager.UpdateProjectDomainAttributes(context.Background(), request)
assert.Nil(t, err)
assert.True(t, createOrUpdateCalled)

db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = func(
ctx context.Context, projectID string) (models.Project, error) {
return models.Project{}, errors.NewFlyteAdminError(codes.NotFound, "validationError")
}
_, validationError := manager.UpdateProjectDomainAttributes(context.Background(), request)
assert.Error(t, validationError)
var secondError errors.FlyteAdminError
assert.ErrorAs(t, validationError, &secondError)
assert.Equal(t, secondError.Error(), "failed to validate that project [project] and domain [domain] are registered, err: [validationError]")

request = &admin.ProjectDomainAttributesUpdateRequest{
Attributes: &admin.ProjectDomainAttributes{},
}
db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = mocks.NewMockRepository().ProjectRepo().(*mocks.MockProjectRepo).GetFunction

_, attributesError := manager.UpdateProjectDomainAttributes(context.Background(), request)
assert.Error(t, attributesError)
var newError errors.FlyteAdminError
assert.ErrorAs(t, attributesError, &newError)
assert.Equal(t, newError.Error(), "domain [] is unrecognized by system")
}

func TestUpdateProjectDomainAttributes_CreateOrMerge(t *testing.T) {
Expand Down Expand Up @@ -349,6 +440,37 @@ func TestGetProjectDomainAttributes(t *testing.T) {
MatchingAttributes: testutils.ExecutionQueueAttributes,
},
}, response))

db.ResourceRepo().(*mocks.MockResourceRepo).GetFunction = func(
ctx context.Context, ID repoInterfaces.ResourceID) (models.Resource, error) {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, "", ID.Workflow)
assert.Equal(t, admin.MatchableResource_EXECUTION_QUEUE.String(), ID.ResourceType)
expectedSerializedAttrs, _ := proto.Marshal(testutils.ExecutionQueueAttributes)
return models.Resource{
Project: project,
Domain: domain,
ResourceType: "resource",
Attributes: expectedSerializedAttrs,
}, errors.NewFlyteAdminError(codes.NotFound, "projectDomainError")
}
_, projectDomainError := manager.GetProjectDomainAttributes(context.Background(), request)
assert.Error(t, projectDomainError)
var newError errors.FlyteAdminError
assert.ErrorAs(t, projectDomainError, &newError)
assert.Equal(t, newError.Error(), "projectDomainError")

db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = func(
ctx context.Context, projectID string) (models.Project, error) {
return models.Project{}, errors.NewFlyteAdminError(codes.NotFound, "validationError")
}
_, validationError := manager.GetProjectDomainAttributes(context.Background(), request)
assert.Error(t, validationError)
var secondError errors.FlyteAdminError
assert.ErrorAs(t, validationError, &secondError)
assert.Equal(t, secondError.Error(), "failed to validate that project [project] and domain [domain] are registered, err: [validationError]")

}

func TestDeleteProjectDomainAttributes(t *testing.T) {
Expand All @@ -368,6 +490,29 @@ func TestDeleteProjectDomainAttributes(t *testing.T) {
manager := NewResourceManager(db, testutils.GetApplicationConfigWithDefaultDomains())
_, err := manager.DeleteProjectDomainAttributes(context.Background(), request)
assert.Nil(t, err)

db.ResourceRepo().(*mocks.MockResourceRepo).DeleteFunction = func(
ctx context.Context, ID repoInterfaces.ResourceID) error {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, admin.MatchableResource_EXECUTION_QUEUE.String(), ID.ResourceType)
return errors.NewFlyteAdminError(codes.NotFound, "failError")
}
_, failError := manager.DeleteProjectDomainAttributes(context.Background(), request)
assert.Error(t, failError)
var newError errors.FlyteAdminError
assert.ErrorAs(t, failError, &newError)
assert.Equal(t, newError.Error(), "failError")

db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = func(
ctx context.Context, projectID string) (models.Project, error) {
return models.Project{}, errors.NewFlyteAdminError(codes.NotFound, "validationError")
}
_, validationError := manager.DeleteProjectDomainAttributes(context.Background(), request)
assert.Error(t, validationError)
var secondError errors.FlyteAdminError
assert.ErrorAs(t, validationError, &secondError)
assert.Equal(t, secondError.Error(), "failed to validate that project [project] and domain [domain] are registered, err: [validationError]")
}

func TestUpdateProjectAttributes(t *testing.T) {
Expand Down Expand Up @@ -679,6 +824,31 @@ func TestDeleteProjectAttributes(t *testing.T) {
manager := NewResourceManager(db, testutils.GetApplicationConfigWithDefaultDomains())
_, err := manager.DeleteProjectAttributes(context.Background(), request)
assert.Nil(t, err)

db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = func(
ctx context.Context, projectID string) (models.Project, error) {
return models.Project{}, errors.NewFlyteAdminError(codes.NotFound, "validationError")
}
_, validationError := manager.DeleteProjectAttributes(context.Background(), request)
assert.Error(t, validationError)
var newError errors.FlyteAdminError
assert.ErrorAs(t, validationError, &newError)
assert.Equal(t, newError.Error(), "failed to validate that project [project] is registered, err: [validationError]")

db.ResourceRepo().(*mocks.MockResourceRepo).DeleteFunction = func(
ctx context.Context, ID repoInterfaces.ResourceID) error {
assert.Equal(t, project, ID.Project)
assert.Equal(t, "", ID.Domain)
assert.Equal(t, admin.MatchableResource_WORKFLOW_EXECUTION_CONFIG.String(), ID.ResourceType)
return errors.NewFlyteAdminError(codes.NotFound, "deleteError")
}
db.ProjectRepo().(*mocks.MockProjectRepo).GetFunction = mocks.NewMockRepository().ProjectRepo().(*mocks.MockProjectRepo).GetFunction

_, failError := manager.DeleteProjectAttributes(context.Background(), request)
assert.Error(t, failError)
var secondError errors.FlyteAdminError
assert.ErrorAs(t, failError, &secondError)
assert.Equal(t, secondError.Error(), "deleteError")
}

func TestGetResource(t *testing.T) {
Expand Down Expand Up @@ -775,4 +945,41 @@ func TestListAllResources(t *testing.T) {
Workflow: "workflow",
Attributes: &workflowAttributes,
}, response.Configurations[1]))

db.ResourceRepo().(*mocks.MockResourceRepo).ListAllFunction = func(ctx context.Context, resourceType string) (
[]models.Resource, error) {
assert.Equal(t, admin.MatchableResource_CLUSTER_RESOURCE.String(), resourceType)
return []models.Resource{
{
Project: "projectA",
ResourceType: admin.MatchableResource_CLUSTER_RESOURCE.String(),
Attributes: marshaledProjectAttrs,
},
{
Project: "projectB",
Domain: "development",
Workflow: "workflow",
ResourceType: admin.MatchableResource_CLUSTER_RESOURCE.String(),
Attributes: marshaledWorkflowAttrs,
},
}, errors.NewFlyteAdminError(codes.NotFound, "resourceError")
}

_, resourceError := manager.ListAll(context.Background(), &admin.ListMatchableAttributesRequest{
ResourceType: admin.MatchableResource_CLUSTER_RESOURCE,
})
assert.Error(t, resourceError)
var newError errors.FlyteAdminError
assert.ErrorAs(t, resourceError, &newError)
assert.Equal(t, newError.Error(), "resourceError")

db.ResourceRepo().(*mocks.MockResourceRepo).ListAllFunction = func(ctx context.Context, resourceType string) (
[]models.Resource, error) {
assert.Equal(t, admin.MatchableResource_CLUSTER_RESOURCE.String(), resourceType)
return nil, nil
}
emptyResource, _ := manager.ListAll(context.Background(), &admin.ListMatchableAttributesRequest{
ResourceType: admin.MatchableResource_CLUSTER_RESOURCE,
})
assert.Equal(t, &admin.ListMatchableAttributesResponse{}, emptyResource, "The two values should be equal")
}