From 5d2c619ff1927abb3001067a9a2bf0ad50c62914 Mon Sep 17 00:00:00 2001
From: Shereen Haj <shajmakh@redhat.com>
Date: Mon, 21 Oct 2024 11:02:56 +0300
Subject: [PATCH] api: add ToString for NodeGroup

Provide a method to return consistent string representation of the structs and add usit tests for both ToString()'s.

Signed-off-by: Shereen Haj <shajmakh@redhat.com>
---
 .../v1/numaresourcesoperator_types.go         | 11 ++-
 .../v1/numaresourcesoperator_types_test.go    | 96 +++++++++++++++++++
 test/e2e/serial/tests/configuration.go        |  4 +-
 3 files changed, 104 insertions(+), 7 deletions(-)
 create mode 100644 api/numaresourcesoperator/v1/numaresourcesoperator_types_test.go

diff --git a/api/numaresourcesoperator/v1/numaresourcesoperator_types.go b/api/numaresourcesoperator/v1/numaresourcesoperator_types.go
index 1f26bab5a..c17056c84 100644
--- a/api/numaresourcesoperator/v1/numaresourcesoperator_types.go
+++ b/api/numaresourcesoperator/v1/numaresourcesoperator_types.go
@@ -210,9 +210,10 @@ func init() {
 }
 
 func (ngc *NodeGroupConfig) ToString() string {
-	if ngc != nil {
-		ngc.SetDefaults()
-		return fmt.Sprintf("PodsFingerprinting mode: %s InfoRefreshMode: %s InfoRefreshPeriod: %s InfoRefreshPause: %s", *ngc.PodsFingerprinting, *ngc.InfoRefreshMode, *ngc.InfoRefreshPeriod, *ngc.InfoRefreshPause)
-	}
-	return ""
+	ngc.SetDefaults()
+	return fmt.Sprintf("PodsFingerprinting mode: %s InfoRefreshMode: %s InfoRefreshPeriod: %s InfoRefreshPause: %s Tolerations: %+v", *ngc.PodsFingerprinting, *ngc.InfoRefreshMode, *ngc.InfoRefreshPeriod, *ngc.InfoRefreshPause, ngc.Tolerations)
+}
+
+func (ng *NodeGroup) ToString() string {
+	return fmt.Sprintf("PoolName: %s MachineConfigPoolSelector: %s Config: %s", *ng.PoolName, ng.MachineConfigPoolSelector.String(), ng.Config.ToString())
 }
diff --git a/api/numaresourcesoperator/v1/numaresourcesoperator_types_test.go b/api/numaresourcesoperator/v1/numaresourcesoperator_types_test.go
new file mode 100644
index 000000000..8cc7e54fc
--- /dev/null
+++ b/api/numaresourcesoperator/v1/numaresourcesoperator_types_test.go
@@ -0,0 +1,96 @@
+package v1
+
+import (
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"testing"
+	"time"
+)
+
+func TestNodeGroupConfigToString(t *testing.T) {
+	defaultConfig := DefaultNodeGroupConfig()
+	//nilValue := NodeGroup{}
+	//non default config fields values
+	d, err := time.ParseDuration("33s")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	period := metav1.Duration{
+		Duration: d,
+	}
+	pfpMode := PodsFingerprintingDisabled
+	refMode := InfoRefreshEvents
+	rteMode := InfoRefreshPauseEnabled
+
+	testcases := []struct {
+		name     string
+		input    NodeGroupConfig
+		expected string
+	}{
+		{
+			name:     "empty fields should reflect default values",
+			input:    NodeGroupConfig{},
+			expected: defaultConfig.ToString(),
+		},
+		{
+			name: "full",
+			input: NodeGroupConfig{
+				PodsFingerprinting: &pfpMode,
+				InfoRefreshMode:    &refMode,
+				InfoRefreshPeriod:  &period,
+				InfoRefreshPause:   &rteMode,
+				Tolerations: []corev1.Toleration{
+					{
+						Key:    "foo",
+						Value:  "1",
+						Effect: corev1.TaintEffectNoSchedule,
+					},
+				},
+			},
+			expected: "PodsFingerprinting mode: Disabled InfoRefreshMode: Events InfoRefreshPeriod: {33s} InfoRefreshPause: Enabled Tolerations: [{Key:foo Operator: Value:1 Effect:NoSchedule TolerationSeconds:<nil>}]",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := tc.input.ToString()
+			if actual != tc.expected {
+				t.Errorf("expected: %s, actual: %s", tc.expected, actual)
+			}
+		})
+	}
+}
+
+func TestNodeGroupToString(t *testing.T) {
+	pfpMode := PodsFingerprintingDisabled
+	refMode := InfoRefreshEvents
+	pn := "pn"
+
+	testcases := []struct {
+		name     string
+		input    NodeGroup
+		expected string
+	}{
+		{
+			name: "empty fields should reflect default values",
+			input: NodeGroup{
+				MachineConfigPoolSelector: nil,
+				Config: &NodeGroupConfig{
+					PodsFingerprinting: &pfpMode,
+					InfoRefreshMode:    &refMode,
+				},
+				PoolName: &pn, // although not allowed more than a specifier but we still need to display and here is not the right place to perform validations
+			},
+			expected: "PoolName: pn MachineConfigPoolSelector: nil Config: PodsFingerprinting mode: Disabled InfoRefreshMode: Events InfoRefreshPeriod: {10s} InfoRefreshPause: Disabled Tolerations: []",
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := tc.input.ToString()
+			if actual != tc.expected {
+				t.Errorf("expected: %s, actual: %s", tc.expected, actual)
+			}
+		})
+	}
+}
diff --git a/test/e2e/serial/tests/configuration.go b/test/e2e/serial/tests/configuration.go
index 0c79c19e0..820bd503c 100644
--- a/test/e2e/serial/tests/configuration.go
+++ b/test/e2e/serial/tests/configuration.go
@@ -1218,7 +1218,7 @@ var _ = Describe("[serial][disruptive] numaresources configuration management",
 					PoolName: &mcp.Name,
 					Config:   conf, // intentionally set the shorter config to ensure the status was normalized when published
 				}
-				klog.Infof("the new node group is with values:\npool name %q\nconfig %s\nMCPselector %s\n", *ng.PoolName, ng.Config.ToString(), ng.MachineConfigPoolSelector.String())
+				klog.Infof("the updated node group: %s", ng.ToString())
 				newNodeGroups := append(initialOperObj.Spec.NodeGroups, ng)
 				var updatedNRO nropv1.NUMAResourcesOperator
 				Eventually(func(g Gomega) {
@@ -1265,7 +1265,7 @@ var _ = Describe("[serial][disruptive] numaresources configuration management",
 					PoolName: &mcp.Name,
 					Config:   conf,
 				}
-				klog.Infof("the updated node group is with values:\npool name %q\nconfig %s\nMCPselector %s\n", *ng.PoolName, ng.Config.ToString(), ng.MachineConfigPoolSelector.String())
+				klog.Infof("the updated node group: %s", ng.ToString())
 				newNodeGroups = append(initialOperObj.Spec.NodeGroups, ng)
 
 				Eventually(func(g Gomega) {