refactor: factor common code out and fix tests

halkyonio · Oct 9, 2019 · 2d0f669 · 2d0f669
1 parent 68e3b2f
commit 2d0f669
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 28 deletions.
diff --git a/pkg/controller/capability.go b/pkg/controller/capability.go
@@ -1,7 +1,6 @@
 package controller
 
 import (
-	"fmt"
 	halkyon "halkyon.io/api/capability/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
@@ -75,15 +74,7 @@ func (in *Capability) DependentStatusFieldName() string {
 }
 
 func (in *Capability) SetSuccessStatus(statuses []DependentResourceStatus, msg string) bool {
-	changed := false
-	for _, status := range statuses {
-		changed = changed || MustSetNamedStringField(&in.Status, status.OwnerStatusField, status.DependentName)
-		if changed {
-			msg = fmt.Sprintf("%s: %s changed to %s", msg, status.OwnerStatusField, status.DependentName)
-		}
-	}
-
-	if changed || halkyon.CapabilityReady != in.Status.Phase || msg != in.Status.Message {
+	if hasChangedFromStatusUpdate(&in.Status, statuses, msg) || halkyon.CapabilityReady != in.Status.Phase || msg != in.Status.Message {
 		in.Status.Phase = halkyon.CapabilityReady
 		in.Status.Message = msg
 		in.requeue = false

diff --git a/pkg/controller/capability_test.go b/pkg/controller/capability_test.go
@@ -1,15 +1,25 @@
 package controller
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestCapabilitySetSuccessStatus(t *testing.T) {
 	c := NewCapability()
 	c.Status.PodName = "initial"
 
 	const s = "new pod name"
 	const msg = "foo"
-	changed := c.SetSuccessStatus([]DependentResourceStatus{NewReadyDependentResourceStatus(s, "PodName")}, msg)
-	if !changed || c.Status.PodName != s || c.Status.Message != msg {
-		t.Fail()
+	const fieldName = "PodName"
+	changed := c.SetSuccessStatus([]DependentResourceStatus{NewReadyDependentResourceStatus(s, fieldName)}, msg)
+	if !changed {
+		t.Errorf("expected updates from SetSuccessStatus, got none")
+	}
+	if c.Status.PodName != s {
+		t.Errorf("expected pod name to be changed to '%s', got '%s'", s, c.Status.PodName)
+	}
+	if !strings.HasPrefix(c.Status.Message, msg) && !strings.Contains(c.Status.Message, fieldName) {
+		t.Errorf("expected message to start with '%s' and contain '%s', got \"%s\"", msg, fieldName, c.Status.Message)
 	}
 }
diff --git a/pkg/controller/component.go b/pkg/controller/component.go
@@ -1,7 +1,6 @@
 package controller
 
 import (
-	"fmt"
 	halkyon "halkyon.io/api/component/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
@@ -86,15 +85,7 @@ func (in *Component) DependentStatusFieldName() string {
 }
 
 func (in *Component) SetSuccessStatus(statuses []DependentResourceStatus, msg string) bool {
-	changed := false
-	for _, status := range statuses {
-		changed = changed || MustSetNamedStringField(&in.Status, status.OwnerStatusField, status.DependentName)
-		if changed {
-			msg = fmt.Sprintf("%s: %s changed to %s", msg, status.OwnerStatusField, status.DependentName)
-		}
-	}
-
-	if changed || halkyon.ComponentReady != in.Status.Phase || msg != in.Status.Message {
+	if hasChangedFromStatusUpdate(&in.Status, statuses, msg) || halkyon.ComponentReady != in.Status.Phase || msg != in.Status.Message {
 		in.Status.Phase = halkyon.ComponentReady
 		in.Status.Message = msg
 		in.requeue = false

diff --git a/pkg/controller/component_test.go b/pkg/controller/component_test.go
@@ -1,15 +1,25 @@
 package controller
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 func TestComponentSetSuccessStatus(t *testing.T) {
 	c := NewComponent()
 	c.Status.PodName = "initial"
 
 	const s = "new pod name"
 	const msg = "foo"
-	changed := c.SetSuccessStatus([]DependentResourceStatus{NewReadyDependentResourceStatus(s, "PodName")}, msg)
-	if !changed || c.Status.PodName != s || c.Status.Message != msg {
-		t.Fail()
+	const fieldName = "PodName"
+	changed := c.SetSuccessStatus([]DependentResourceStatus{NewReadyDependentResourceStatus(s, fieldName)}, msg)
+	if !changed {
+		t.Errorf("expected updates from SetSuccessStatus, got none")
+	}
+	if c.Status.PodName != s {
+		t.Errorf("expected pod name to be changed to '%s', got '%s'", s, c.Status.PodName)
+	}
+	if !strings.HasPrefix(c.Status.Message, msg) && !strings.Contains(c.Status.Message, fieldName) {
+		t.Errorf("expected message to start with '%s' and contain '%s', got \"%s\"", msg, fieldName, c.Status.Message)
 	}
 }
diff --git a/pkg/controller/resource.go b/pkg/controller/resource.go
@@ -1,6 +1,7 @@
 package controller
 
 import (
+	"fmt"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
@@ -20,3 +21,14 @@ type Resource interface {
 	GetAPIObject() runtime.Object
 	SetAPIObject(object runtime.Object)
 }
+
+func hasChangedFromStatusUpdate(status interface{}, statuses []DependentResourceStatus, msg string) bool {
+	changed := false
+	for _, s := range statuses {
+		changed = changed || MustSetNamedStringField(status, s.OwnerStatusField, s.DependentName)
+		if changed {
+			msg = fmt.Sprintf("%s: '%s' changed to '%s'", msg, s.OwnerStatusField, s.DependentName)
+		}
+	}
+	return changed
+}