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

Improve resource manager test coverage #5973

Merged
merged 20 commits into from
Dec 26, 2024
Merged

Improve resource manager test coverage #5973

Changes from 8 commits
Commits
Show all changes
20 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
a8b3441
Merge master
eapolinario Dec 26, 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
159 changes: 159 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,13 @@ 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
Contributor 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


}

func TestUpdateWorkflowAttributes_CreateOrMerge(t *testing.T) {
Expand Down Expand Up @@ -181,6 +188,33 @@ 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


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")
}

_, failError := manager.GetWorkflowAttributes(context.Background(), request)
assert.Error(t, failError)
}

func TestDeleteWorkflowAttributes(t *testing.T) {
Expand All @@ -202,6 +236,25 @@ 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)

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")
}

_, failError := manager.DeleteWorkflowAttributes(context.Background(), request)
assert.Error(t, failError)
}

func TestUpdateProjectDomainAttributes(t *testing.T) {
Expand Down Expand Up @@ -229,6 +282,19 @@ 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)

request = &admin.ProjectDomainAttributesUpdateRequest{
Attributes: &admin.ProjectDomainAttributes{},
}
_, attributesError := manager.UpdateProjectDomainAttributes(context.Background(), request)
assert.Error(t, attributesError)
}

func TestUpdateProjectDomainAttributes_CreateOrMerge(t *testing.T) {
Expand Down Expand Up @@ -349,6 +415,30 @@ 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)

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)
}

func TestDeleteProjectDomainAttributes(t *testing.T) {
Expand All @@ -368,6 +458,24 @@ 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)

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)

}

func TestUpdateProjectAttributes(t *testing.T) {
Expand Down Expand Up @@ -679,6 +787,23 @@ 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)

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")
}
_, failError := manager.DeleteProjectAttributes(context.Background(), request)
assert.Error(t, failError)
}

func TestGetResource(t *testing.T) {
Expand Down Expand Up @@ -775,4 +900,38 @@ 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)

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")
}
Loading