Merge pull request #1089 from openshift-kni/test-coverage-20241121

tests: add coverage report and increase coverage

.gitignore

@@ -23,3 +23,5 @@ testbin/*
 *.swp
 *.swo
 *~
+
+coverage.out

Makefile

@@ -123,6 +123,9 @@ test-unit: test-unit-pkgs test-controllers
 test-unit-pkgs:
 	go test ./api/... ./pkg/... ./rte/pkg/... ./internal/... ./nrovalidate/validator/...
 
+test-unit-pkgs-cover:
+	go test -coverprofile=coverage.out ./api/numaresourcesoperator/v1/helper/... ./api/numaresourcesoperator/v1alpha1/helper/... ./pkg/... ./rte/pkg/... ./internal/... ./nrovalidate/validator/...
+
 test-controllers: envtest
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" go test ./controllers/...
 
@@ -135,6 +138,12 @@ test-install-e2e: build-e2e-all
 test-must-gather-e2e: build-must-gather-e2e
 	hack/run-test-must-gather-e2e.sh
 
+cover-view:
+	go tool cover -html=coverage.out
+
+cover-summary:
+	go tool cover -func=coverage.out
+
 ##@ Build
 
 binary: build-tools

go.mod

@@ -6,7 +6,6 @@ toolchain go1.22.8
 
 require (
 	github.com/aquasecurity/go-version v0.0.0-20210121072130-637058cfe492
-	github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d
 	github.com/drone/envsubst v1.0.3
 	github.com/go-logr/logr v1.4.2
 	github.com/google/go-cmp v0.6.0

go.sum

@@ -1370,8 +1370,6 @@ github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
 github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
-github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ=
-github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=

internal/reconcile/step_test.go

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2024 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package reconcile
+
+import (
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestStepSuccess(t *testing.T) {
+	st := StepSuccess()
+	assert.True(t, st.Done())
+	assert.False(t, st.EarlyStop())
+}
+
+func TestStepOngoing(t *testing.T) {
+	st := StepOngoing(5 * time.Second)
+	assert.False(t, st.Done())
+	assert.True(t, st.EarlyStop())
+}
+
+func TestStepFailed(t *testing.T) {
+	st := StepFailed(errors.New("fake error"))
+	assert.False(t, st.Done())
+	assert.True(t, st.EarlyStop())
+}

pkg/hash/configmap.go

@@ -34,7 +34,7 @@ import (
 
 const ConfigMapAnnotation = "configmap.hash"
 
-func ComputeCurrentConfigMap(ctx context.Context, cli client.Client, cm *corev1.ConfigMap) (string, error) {
+func ComputeCurrentConfigMap(ctx context.Context, cli client.Reader, cm *corev1.ConfigMap) (string, error) {
 	updatedConfigMap := &corev1.ConfigMap{}
 	key := client.ObjectKeyFromObject(cm)
 

pkg/hash/configmap_test.go

@@ -17,15 +17,50 @@
 package hash
 
 import (
+	"context"
+	"regexp"
 	"strings"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"github.com/asaskevich/govalidator"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	"github.com/openshift-kni/numaresources-operator/internal/objects"
 )
 
+func TestComputeCurrentConfigMap(t *testing.T) {
+	testCases := []struct {
+		name        string
+		cli         client.Client
+		cm          *corev1.ConfigMap
+		expectedOut string
+		expectedErr bool
+	}{
+		{
+			name: "map not created",
+			cli:  fake.NewFakeClient(),
+			cm:   objects.NewKubeletConfigConfigMap("test", map[string]string{}, objects.NewKubeletConfigAutoresizeControlPlane()),
+			// verified manually
+			expectedOut: "SHA256:93909e569a15b6e4a5eefdcac4153f2c8179bf155143d10dac589f62ddcdf742",
+			expectedErr: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		out, err := ComputeCurrentConfigMap(context.TODO(), tc.cli, tc.cm.DeepCopy())
+		gotErr := (err != nil)
+		if gotErr != tc.expectedErr {
+			t.Fatalf("got error %v expected error=%v", err, tc.expectedErr)
+		}
+		if out != tc.expectedOut {
+			t.Fatalf("got output %q expected %q", out, tc.expectedOut)
+		}
+	}
+}
+
 func TestConfigMapData(t *testing.T) {
 	cm := &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
@@ -64,7 +99,7 @@ func TestConfigMapData(t *testing.T) {
 			t.Errorf("test: %q hash string cannot be empty", tc.testName)
 		}
 
-		if !govalidator.IsHash(strings.TrimLeft(hash, "SHA256:"), "sha256") {
+		if !looksLikeSHA256Sum(hash) {
 			t.Errorf("test: %q ilegal hash string %q", tc.testName, hash)
 		}
 	}
@@ -183,3 +218,12 @@ func TestConfigMapDataConsistency(t *testing.T) {
 		}
 	}
 }
+
+func looksLikeSHA256Sum(hs string) bool {
+	if !strings.HasPrefix(hs, "SHA256:") {
+		return false
+	}
+	val := strings.TrimLeft(hs, "SHA256:")
+	ok, _ := regexp.MatchString("^[a-f0-9]{64}$", val)
+	return ok
+}

pkg/numaresourcesscheduler/consts_test.go

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2024 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package numaresourcesscheduler
+
+import "testing"
+
+func TestLeaderElectionResourceName(t *testing.T) {
+	// avoid conflicts with main scheduler
+	if LeaderElectionResourceName == "kube-scheduler" {
+		t.Fatalf("the secondary scheduler leader election resource name *MUST* be different than the kubernetes defaults")
+	}
+}

pkg/numaresourcesscheduler/controlplane/discover_test.go

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2024 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package controlplane
+
+import "testing"
+
+func TestDefaults(t *testing.T) {
+	info := Defaults()
+	if info.NodeCount < 1 {
+		t.Fatalf("control plane node count defaults should be at least 1")
+	}
+}

pkg/status/conditioninfo/conditioninfo_test.go

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2024 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package conditioninfo
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/openshift-kni/numaresources-operator/pkg/status"
+)
+
+func TestWithReason(t *testing.T) {
+	cond := ConditionInfo{}
+	cond2 := cond.WithReason("fizzbuzz")
+	assert.Empty(t, cond.Reason) // original object not mutated
+	assert.Equal(t, cond2.Reason, "fizzbuzz")
+}
+
+func TestWithMessage(t *testing.T) {
+	cond := ConditionInfo{}
+	cond2 := cond.WithMessage("foobar")
+	assert.Empty(t, cond.Message) // original object not mutated
+	assert.Equal(t, cond2.Message, "foobar")
+}
+
+func TestAvailable(t *testing.T) {
+	cond := Available()
+	assert.Equal(t, cond.Type, status.ConditionAvailable)
+}
+
+func TestProgressing(t *testing.T) {
+	cond := Progressing()
+	assert.Equal(t, cond.Type, status.ConditionProgressing)
+}
+
+func TestDegradedFromError(t *testing.T) {
+	cond1 := DegradedFromError(nil)
+	assert.Equal(t, cond1.Type, status.ConditionDegraded)
+	assert.Equal(t, cond1.Reason, status.ReasonAsExpected)
+	assert.Empty(t, cond1.Message)
+
+	cond2 := DegradedFromError(errors.New("test conditioninfo error"))
+	assert.Equal(t, cond2.Type, status.ConditionDegraded)
+	assert.Equal(t, cond2.Reason, status.ReasonInternalError)
+	assert.Equal(t, cond2.Message, "test conditioninfo error")
+}

pkg/status/status.go

@@ -134,14 +134,6 @@ func newBaseConditions() []metav1.Condition {
 	}
 }
 
-type ErrResourcesNotReady struct {
-	Message string
-}
-
-func (e ErrResourcesNotReady) Error() string {
-	return e.Message
-}
-
 func ReasonFromError(err error) string {
 	if err == nil {
 		return ReasonAsExpected

pkg/status/status_test.go

@@ -33,6 +33,45 @@ import (
 	testobjs "github.com/openshift-kni/numaresources-operator/internal/objects"
 )
 
+func TestFindCondition(t *testing.T) {
+	type testCase struct {
+		name          string
+		desired       string
+		conds         []metav1.Condition
+		expectedFound bool
+	}
+
+	testCases := []testCase{
+		{
+			name:          "nil conditions",
+			desired:       ConditionAvailable,
+			expectedFound: false,
+		},
+		{
+			name:          "missing condition",
+			desired:       "foobar",
+			conds:         newBaseConditions(),
+			expectedFound: false,
+		},
+		{
+			name:          "found condition",
+			desired:       ConditionProgressing,
+			conds:         newBaseConditions(),
+			expectedFound: true,
+		},
+	}
+
+	for _, tcase := range testCases {
+		t.Run(tcase.name, func(t *testing.T) {
+			cond := FindCondition(tcase.conds, tcase.desired)
+			found := (cond != nil)
+			if found != tcase.expectedFound {
+				t.Errorf("failure looking for condition %q: got=%v expected=%v", tcase.desired, found, tcase.expectedFound)
+			}
+		})
+	}
+}
+
 func TestUpdate(t *testing.T) {
 	err := nropv1.AddToScheme(scheme.Scheme)
 	if err != nil {
@@ -162,6 +201,46 @@ func TestIsUpdatedNUMAResourcesOperator(t *testing.T) {
 	}
 }
 
+func TestReasonFromError(t *testing.T) {
+	type testCase struct {
+		name     string
+		err      error
+		expected string
+	}
+
+	testCases := []testCase{
+		{
+			name:     "nil error",
+			expected: ReasonAsExpected,
+		},
+		{
+			name:     "simple error",
+			err:      errors.New("testing error with simple message"),
+			expected: ReasonInternalError,
+		},
+		{
+			name:     "simple formatted error",
+			err:      fmt.Errorf("testing error message=%s val=%d", "complex", 42),
+			expected: ReasonInternalError,
+		},
+
+		{
+			name:     "wrapped error",
+			err:      fmt.Errorf("outer error: %w", errors.New("inner error")),
+			expected: ReasonInternalError,
+		},
+	}
+
+	for _, tcase := range testCases {
+		t.Run(tcase.name, func(t *testing.T) {
+			got := ReasonFromError(tcase.err)
+			if got != tcase.expected {
+				t.Errorf("failure getting reason from error: got=%q expected=%q", got, tcase.expected)
+			}
+		})
+	}
+}
+
 func TestMessageFromError(t *testing.T) {
 	type testCase struct {
 		name     string