simplify Wait/Check unit tests and implementation

Signed-off-by: Fabian Lopez <lfabian@vmware.com>
knative · Jul 13, 2021 · 5356437 · 5356437
1 parent 3831cd6
commit 5356437
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 84 deletions.
diff --git a/test/request.go b/test/request.go
@@ -120,20 +120,21 @@ func WaitForEndpointStateWithTimeout(
 	timeout time.Duration,
 	opts ...interface{}) (*spoof.Response, error) {
 
-	return waitForEndpointStateWithTimeout(ctx, kubeClient, logf, url, inState, desc, resolvable, timeout, true, opts)
+	client, rOpts, err := makeSpoofClient(ctx, kubeClient, logf, url, resolvable, timeout /* true, */, opts)
+	if err != nil {
+		return nil, err
+	}
+	return client.WaitForEndpointState(ctx, url, inState, desc, rOpts...)
 }
 
-func waitForEndpointStateWithTimeout(
+func makeSpoofClient(
 	ctx context.Context,
 	kubeClient kubernetes.Interface,
 	logf logging.FormatLogger,
 	url *url.URL,
-	inState spoof.ResponseChecker,
-	desc string,
 	resolvable bool,
 	timeout time.Duration,
-	wait bool,
-	opts ...interface{}) (*spoof.Response, error) {
+	opts ...interface{}) (*spoof.SpoofingClient, []spoof.RequestOption, error) {
 
 	var tOpts []spoof.TransportOption
 	var rOpts []spoof.RequestOption
@@ -149,14 +150,11 @@ func waitForEndpointStateWithTimeout(
 
 	client, err := NewSpoofingClient(ctx, kubeClient, logf, url.Hostname(), resolvable, tOpts...)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	client.RequestTimeout = timeout
 
-	if wait {
-		return client.WaitForEndpointState(ctx, url, inState, desc, rOpts...)
-	}
-	return client.CheckEndpointState(ctx, url, inState, desc, rOpts...)
+	return client, rOpts, nil
 }
 
 func CheckEndpointState(
@@ -169,6 +167,9 @@ func CheckEndpointState(
 	resolvable bool,
 	opts ...interface{},
 ) (*spoof.Response, error) {
-	return waitForEndpointStateWithTimeout(ctx, kubeClient, logf, url, inState,
-		desc, resolvable, Flags.SpoofRequestTimeout, false, opts...)
+	client, rOpts, err := makeSpoofClient(ctx, kubeClient, logf, url, resolvable, Flags.SpoofRequestTimeout /* false, */, opts...)
+	if err != nil {
+		return nil, err
+	}
+	return client.CheckEndpointState(ctx, url, inState, desc, rOpts...)
 }
diff --git a/test/spoof/spoof.go b/test/spoof/spoof.go
@@ -313,13 +313,16 @@ func (sc *SpoofingClient) Check(req *http.Request, inState ResponseChecker) (*Re
 		Body:       body,
 	}
 	ok, err := inState(resp)
+
+	sc.logZipkinTrace(resp)
+
 	if err != nil {
 		return resp, fmt.Errorf("response: %s did not pass checks: %w", resp, err)
 	}
 	if ok {
-		sc.logZipkinTrace(resp)
 		return resp, nil
 	}
+
 	return nil, err
 }
 

diff --git a/test/spoof/spoof_test.go b/test/spoof/spoof_test.go
@@ -25,10 +25,6 @@ import (
 	"net/url"
 
 	"testing"
-
-	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"knative.dev/pkg/client/injection/kube/client/fake"
 )
 
 type fakeTransport struct{}
@@ -54,21 +50,6 @@ func (c *countCalls) count(rc ResponseChecker) ResponseChecker {
 }
 
 func TestSpoofingClient_CheckEndpointState(t *testing.T) {
-	ingress := &corev1.Service{
-		ObjectMeta: v1.ObjectMeta{
-			Name:      "istio-ingressgateway",
-			Namespace: "istio-system",
-		},
-		Status: corev1.ServiceStatus{
-			LoadBalancer: corev1.LoadBalancerStatus{
-				Ingress: []corev1.LoadBalancerIngress{
-					{
-						Hostname: "host",
-					},
-				},
-			},
-		},
-	}
 	type args struct {
 		url     *url.URL
 		inState ResponseChecker
@@ -121,27 +102,15 @@ func TestSpoofingClient_CheckEndpointState(t *testing.T) {
 		wantCalls: 1,
 	}}
 	for _, tt := range tests {
-		_, fKlient := fake.With(context.TODO(), ingress)
 		t.Run(tt.name, func(t *testing.T) {
-			sc, err := New(
-				context.TODO(),
-				fKlient,
-				t.Logf,
-				"some.svc.knative.dev",
-
-				false,
-				"host",
-				1,
-				1,
-			)
-			if err != nil {
-				t.Fatalf("Spoofing client not created: %v", err)
-			}
-			sc.Client = &http.Client{
-				Transport: &fakeTransport{},
+			sc := &SpoofingClient{
+				Client:          &http.Client{Transport: &fakeTransport{}},
+				Logf:            t.Logf,
+				RequestInterval: 1,
+				RequestTimeout:  1,
 			}
 			counter := countCalls{}
-			_, err = sc.CheckEndpointState(context.TODO(), tt.args.url, counter.count(tt.args.inState), tt.args.desc, tt.args.opts...)
+			_, err := sc.CheckEndpointState(context.TODO(), tt.args.url, counter.count(tt.args.inState), tt.args.desc, tt.args.opts...)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("SpoofingClient.CheckEndpointState() error = %v, wantErr %v", err, tt.wantErr)
 				return
@@ -154,21 +123,6 @@ func TestSpoofingClient_CheckEndpointState(t *testing.T) {
 }
 
 func TestSpoofingClient_WaitForEndpointState(t *testing.T) {
-	ingress := &corev1.Service{
-		ObjectMeta: v1.ObjectMeta{
-			Name:      "istio-ingressgateway",
-			Namespace: "istio-system",
-		},
-		Status: corev1.ServiceStatus{
-			LoadBalancer: corev1.LoadBalancerStatus{
-				Ingress: []corev1.LoadBalancerIngress{
-					{
-						Hostname: "host",
-					},
-				},
-			},
-		},
-	}
 	type args struct {
 		url     *url.URL
 		inState ResponseChecker
@@ -221,27 +175,15 @@ func TestSpoofingClient_WaitForEndpointState(t *testing.T) {
 		wantCalls: 3,
 	}}
 	for _, tt := range tests {
-		_, fKlient := fake.With(context.TODO(), ingress)
 		t.Run(tt.name, func(t *testing.T) {
-			sc, err := New(
-				context.TODO(),
-				fKlient,
-				t.Logf,
-				"some.svc.knative.dev",
-
-				false,
-				"host",
-				1,
-				1,
-			)
-			if err != nil {
-				t.Fatalf("Spoofing client not created: %v", err)
-			}
-			sc.Client = &http.Client{
-				Transport: &fakeTransport{},
+			sc := &SpoofingClient{
+				Client:          &http.Client{Transport: &fakeTransport{}},
+				Logf:            t.Logf,
+				RequestInterval: 1,
+				RequestTimeout:  1,
 			}
 			counter := countCalls{}
-			_, err = sc.WaitForEndpointState(context.TODO(), tt.args.url, counter.count(tt.args.inState), tt.args.desc, tt.args.opts...)
+			_, err := sc.WaitForEndpointState(context.TODO(), tt.args.url, counter.count(tt.args.inState), tt.args.desc, tt.args.opts...)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("SpoofingClient.CheckEndpointState() error = %v, wantErr %v", err, tt.wantErr)
 				return