Skip to content

[TEST] Improve unit test coverage for apiserver pkg/model #3495

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

Merged
kevin85421 merged 10 commits into from
May 5, 2025
Merged

[TEST] Improve unit test coverage for apiserver pkg/model #3495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4696df2
test: Add unit test for FromCrdToAPIService
JiangJiaWei1103 Apr 26, 2025
8b77188
test: Add test for PopulateServeApplicationStatus
JiangJiaWei1103 Apr 27, 2025
1c803ec
refactor: Rename app name to avoid confusion
JiangJiaWei1103 Apr 27, 2025
76a241b
refactor: Align var name
JiangJiaWei1103 Apr 27, 2025
f7985e1
test: Add test for PopulateServeDeploymentStatus
JiangJiaWei1103 Apr 27, 2025
9326638
test: Add test for PopulateRayServiceEvent
JiangJiaWei1103 Apr 27, 2025
2c2f06f
refactor: Move var into test functions
JiangJiaWei1103 Apr 27, 2025
14dde50
fix: Lint
JiangJiaWei1103 Apr 27, 2025
45dd2e3
refactor: Define expected var to avoid duplication
JiangJiaWei1103 Apr 28, 2025
0adf21a
refactor: Reuse field values of the predefined object for tests
JiangJiaWei1103 May 3, 2025
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
8 changes: 4 additions & 4 deletions apiserver/pkg/model/converter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ func FromCrdToAPIServices(
services []*rayv1api.RayService,
serviceEventsMap map[string][]corev1.Event,
) []*api.RayService {
apiServices := make([]*api.RayService, 0)
apiServices := make([]*api.RayService, 0, len(services))
for _, service := range services {
apiServices = append(apiServices, FromCrdToAPIService(service, serviceEventsMap[service.Name]))
}
Expand Down Expand Up @@ -593,7 +593,7 @@ func PoplulateRayServiceStatus(
func PopulateServeApplicationStatus(
serveApplicationStatuses map[string]rayv1api.AppStatus,
) []*api.ServeApplicationStatus {
appStatuses := make([]*api.ServeApplicationStatus, 0)
appStatuses := make([]*api.ServeApplicationStatus, 0, len(serveApplicationStatuses))
for appName, appStatus := range serveApplicationStatuses {
ds := &api.ServeApplicationStatus{
Name: appName,
Expand All @@ -609,7 +609,7 @@ func PopulateServeApplicationStatus(
func PopulateServeDeploymentStatus(
serveDeploymentStatuses map[string]rayv1api.ServeDeploymentStatus,
) []*api.ServeDeploymentStatus {
deploymentStatuses := make([]*api.ServeDeploymentStatus, 0)
deploymentStatuses := make([]*api.ServeDeploymentStatus, 0, len(serveDeploymentStatuses))
for deploymentName, deploymentStatus := range serveDeploymentStatuses {
ds := &api.ServeDeploymentStatus{
DeploymentName: deploymentName,
Expand All @@ -622,7 +622,7 @@ func PopulateServeDeploymentStatus(
}

func PopulateRayServiceEvent(serviceName string, events []corev1.Event) []*api.RayServiceEvent {
serviceEvents := make([]*api.RayServiceEvent, 0)
serviceEvents := make([]*api.RayServiceEvent, 0, len(events))
for _, event := range events {
serviceEvent := &api.RayServiceEvent{
Id: event.Name,
Expand Down
80 changes: 80 additions & 0 deletions apiserver/pkg/model/converter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,3 +728,83 @@ func TestPopulateJob(t *testing.T) {
assert.Equal(t, "Job is currently running", job.Message)
assert.Equal(t, "raycluster-sample-xxxxx", job.RayClusterName)
}

func TestFromCrdToAPIServices(t *testing.T) {
services := []*rayv1api.RayService{&ServiceV2Test}
serviceEventsMap := map[string][]corev1.Event{}
apiServices := FromCrdToAPIServices(services, serviceEventsMap)
assert.Len(t, apiServices, 1)

apiService := apiServices[0]
assert.Equal(t, ServiceV2Test.ObjectMeta.Name, apiService.Name)
assert.Equal(t, ServiceV2Test.ObjectMeta.Namespace, apiService.Namespace)
assert.Equal(t, ServiceV2Test.ObjectMeta.Labels["ray.io/user"], apiService.User)
}

func TestPopulateService(t *testing.T) {
service := FromCrdToAPIService(&ServiceV2Test, []corev1.Event{})
assert.Equal(t, ServiceV2Test.ObjectMeta.Name, service.Name)
assert.Equal(t, ServiceV2Test.ObjectMeta.Namespace, service.Namespace)
assert.Equal(t, ServiceV2Test.ObjectMeta.Labels["ray.io/user"], service.User)
assert.Equal(t, ServiceV2Test.Spec.ServeConfigV2, service.ServeConfig_V2)
assert.Equal(t, int64(-1), service.DeleteAt.Seconds)
}

func TestPopulateServeApplicationStatus(t *testing.T) {
expectedAppName := "app0"
expectedAppStatus := rayv1api.AppStatus{
Deployments: map[string]rayv1api.ServeDeploymentStatus{},
Status: rayv1api.ApplicationStatusEnum.DEPLOYING,
Message: "Deploying...",
}
serveApplicationStatuses := map[string]rayv1api.AppStatus{
expectedAppName: expectedAppStatus,
}
appStatuses := PopulateServeApplicationStatus(serveApplicationStatuses)
assert.Len(t, appStatuses, 1)

appStatus := appStatuses[0]
assert.Equal(t, expectedAppName, appStatus.Name)
assert.Equal(t, expectedAppStatus.Status, appStatus.Status)
assert.Equal(t, expectedAppStatus.Message, appStatus.Message)
}

func TestPopulateServeDeploymentStatus(t *testing.T) {
expectedDeploymentStatus := rayv1api.ServeDeploymentStatus{
Status: rayv1api.DeploymentStatusEnum.UPDATING,
Message: "Updating...",
}
serveDeploymentStatuses := map[string]rayv1api.ServeDeploymentStatus{
"deployment0": expectedDeploymentStatus,
}
deploymentStatuses := PopulateServeDeploymentStatus(serveDeploymentStatuses)
assert.Len(t, deploymentStatuses, 1)

deploymentStatus := deploymentStatuses[0]
assert.Equal(t, expectedDeploymentStatus.Status, deploymentStatus.Status)
assert.Equal(t, expectedDeploymentStatus.Message, deploymentStatus.Message)
}

func TestPopulateRayServiceEvent(t *testing.T) {
expectedServiceName := "svc0"
expectedEvent := corev1.Event{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Reason: "test",
Message: "test",
Type: "Normal",
Count: 2,
}
events := []corev1.Event{expectedEvent}
serviceEvents := PopulateRayServiceEvent(expectedServiceName, events)
assert.Len(t, serviceEvents, 1)

serviceEvent := serviceEvents[0]
Copy link
Contributor

@dentiny dentiny May 1, 2025

Choose a reason for hiding this comment

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

not sure if I suggested before, I would prefer to have an event variable.

event := &corev1.Event{
		{
			ObjectMeta: metav1.ObjectMeta{
				Name: "test",
			},
			Reason:  "test",
			Message: "test",
			Type:    "Normal",
			Count:   2,
		}

so we could reuse the fields inside of it, rather than hand-write ourselves.
The benefit is, if we want to update field value in the future, we only need to update in one place.

Copy link
Contributor Author

@JiangJiaWei1103 JiangJiaWei1103 May 3, 2025

Choose a reason for hiding this comment

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

Fixed! All checks now consistently reuse field values from the pre-defined objects. Thanks for catching that.

assert.Equal(t, expectedEvent.Name, serviceEvent.Id)
assert.Equal(t, expectedServiceName+"-"+expectedEvent.ObjectMeta.Name, serviceEvent.Name)
assert.Equal(t, expectedEvent.Reason, serviceEvent.Reason)
assert.Equal(t, expectedEvent.Message, serviceEvent.Message)
assert.Equal(t, expectedEvent.Type, serviceEvent.Type)
assert.Equal(t, expectedEvent.Count, serviceEvent.Count)
}
Loading