move some spoof helpers to the spoof package (#1827)

This helps avoid pulling in the pkg/test globals

test/request.go

@@ -20,16 +20,12 @@ package test
 
 import (
 	"context"
-	"fmt"
-	"net/http"
 	"net/url"
 	"strings"
-	"sync"
 	"time"
 
 	"knative.dev/pkg/test/flags"
 
-	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/kubernetes"
 	"knative.dev/pkg/test/logging"
 	"knative.dev/pkg/test/spoof"
@@ -37,107 +33,37 @@ import (
 
 // RequestOption enables configuration of requests
 // when polling for endpoint states.
-type RequestOption func(*http.Request)
+type RequestOption = spoof.RequestOption
 
 // WithHeader will add the provided headers to the request.
-func WithHeader(header http.Header) RequestOption {
-	return func(r *http.Request) {
-		if r.Header == nil {
-			r.Header = header
-			return
-		}
-		for key, values := range header {
-			for _, value := range values {
-				r.Header.Add(key, value)
-			}
-		}
-	}
-}
+//
+// Deprecated: Use the spoof package version
+var WithHeader = spoof.WithHeader
 
 // Retrying modifies a ResponseChecker to retry certain response codes.
-func Retrying(rc spoof.ResponseChecker, codes ...int) spoof.ResponseChecker {
-	return func(resp *spoof.Response) (bool, error) {
-		for _, code := range codes {
-			if resp.StatusCode == code {
-				// Returning (false, nil) causes SpoofingClient.Poll to retry.
-				// sc.logger.Info("Retrying for code ", resp.StatusCode)
-				return false, nil
-			}
-		}
-
-		// If we didn't match any retryable codes, invoke the ResponseChecker that we wrapped.
-		return rc(resp)
-	}
-}
+//
+// Deprecated: Use the spoof package version
+var Retrying = spoof.Retrying
 
 // IsOneOfStatusCodes checks that the response code is equal to the given one.
-func IsOneOfStatusCodes(codes ...int) spoof.ResponseChecker {
-	return func(resp *spoof.Response) (bool, error) {
-		for _, code := range codes {
-			if resp.StatusCode == code {
-				return true, nil
-			}
-		}
-
-		return true, fmt.Errorf("status = %d %s, want one of: %v", resp.StatusCode, resp.Status, codes)
-	}
-}
+//
+// Deprecated: Use the spoof package version
+var IsOneOfStatusCodes = spoof.IsOneOfStatusCodes
 
 // IsStatusOK checks that the response code is a 200.
-func IsStatusOK(resp *spoof.Response) (bool, error) {
-	return IsOneOfStatusCodes(http.StatusOK)(resp)
-}
+//
+// Deprecated: Use the spoof package version
+var IsStatusOK = spoof.IsStatusOK
 
 // MatchesAllBodies checks that the *first* response body matches the "expected" body, otherwise failing.
-func MatchesAllBodies(all ...string) spoof.ResponseChecker {
-	var m sync.Mutex
-	// This helps with two things:
-	// 1. we can use Equal on sets
-	// 2. it will collapse the duplicates
-	want := sets.NewString(all...)
-	seen := make(sets.String, len(all))
-
-	return func(resp *spoof.Response) (bool, error) {
-		bs := string(resp.Body)
-		for expected := range want {
-			if !strings.Contains(bs, expected) {
-				// See if the next one matches.
-				continue
-			}
-
-			m.Lock()
-			defer m.Unlock()
-			seen.Insert(expected)
-
-			// Stop once we've seen them all.
-			return want.Equal(seen), nil
-		}
-
-		// Returning (true, err) causes SpoofingClient.Poll to fail.
-		return true, fmt.Errorf("body = %s, want one of: %s", bs, all)
-	}
-}
+//
+// Deprecated: Use the spoof package version
+var MatchesAllBodies = spoof.MatchesAllBodies
 
 // MatchesBody checks that the *first* response body matches the "expected" body, otherwise failing.
-func MatchesBody(expected string) spoof.ResponseChecker {
-	return func(resp *spoof.Response) (bool, error) {
-		if !strings.Contains(string(resp.Body), expected) {
-			// Returning (true, err) causes SpoofingClient.Poll to fail.
-			return true, fmt.Errorf("body = %s, want: %s", string(resp.Body), expected)
-		}
-
-		return true, nil
-	}
-}
-
-// EventuallyMatchesBody checks that the response body *eventually* matches the expected body.
-// TODO(#1178): Delete me. We don't want to need this; we should be waiting for an appropriate Status instead.
-func EventuallyMatchesBody(expected string) spoof.ResponseChecker {
-	return func(resp *spoof.Response) (bool, error) {
-		// Returning (false, nil) causes SpoofingClient.Poll to retry.
-		return strings.Contains(string(resp.Body), expected), nil
-	}
-}
+//
+// Deprecated: Use the spoof package version
+var MatchesBody = spoof.MatchesBody
 
 // MatchesAllOf combines multiple ResponseCheckers to one ResponseChecker with a logical AND. The
 // checkers are executed in order. The first function to trigger an error or a retry will short-circuit
@@ -147,14 +73,16 @@ func EventuallyMatchesBody(expected string) spoof.ResponseChecker {
 // MatchesAllOf(IsStatusOK, MatchesBody("test"))
 //
 // The MatchesBody check will only be executed after the IsStatusOK has passed.
-func MatchesAllOf(checkers ...spoof.ResponseChecker) spoof.ResponseChecker {
+//
+// Deprecated: Use the spoof package version
+var MatchesAllOf = spoof.MatchesAllOf
+
+// EventuallyMatchesBody checks that the response body *eventually* matches the expected body.
+// TODO(#1178): Delete me. We don't want to need this; we should be waiting for an appropriate Status instead.
+func EventuallyMatchesBody(expected string) spoof.ResponseChecker {
 	return func(resp *spoof.Response) (bool, error) {
-		for _, checker := range checkers {
-			if done, err := checker(resp); err != nil || !done {
-				return done, err
-			}
-		}
-		return true, nil
+		// Returning (false, nil) causes SpoofingClient.Poll to retry.
+		return strings.Contains(string(resp.Body), expected), nil
 	}
 }
 
@@ -193,24 +121,16 @@ func WaitForEndpointStateWithTimeout(
 	resolvable bool,
 	timeout time.Duration,
 	opts ...interface{}) (*spoof.Response, error) {
-	defer logging.GetEmitableSpan(ctx, "WaitForEndpointState/"+desc).End()
-
-	if url.Scheme == "" || url.Host == "" {
-		return nil, fmt.Errorf("invalid URL: %q", url.String())
-	}
-
-	req, err := http.NewRequest(http.MethodGet, url.String(), nil)
-	if err != nil {
-		return nil, err
-	}
 
 	var tOpts []spoof.TransportOption
+	var rOpts []spoof.RequestOption
+
 	for _, opt := range opts {
-		rOpt, ok := opt.(RequestOption)
-		if ok {
-			rOpt(req)
-		} else if tOpt, ok := opt.(spoof.TransportOption); ok {
-			tOpts = append(tOpts, tOpt)
+		switch o := opt.(type) {
+		case spoof.RequestOption:
+			rOpts = append(rOpts, o)
+		case spoof.TransportOption:
+			tOpts = append(tOpts, o)
 		}
 	}
 
@@ -220,5 +140,5 @@ func WaitForEndpointStateWithTimeout(
 	}
 	client.RequestTimeout = timeout
 
-	return client.Poll(req, inState)
+	return client.WaitForEndpointState(ctx, url, inState, desc, rOpts...)
 }

test/spoof/request.go

@@ -0,0 +1,38 @@
+/*
+Copyright 2020 The Knative Authors
+
+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 spoof
+
+import "net/http"
+
+// RequestOption enables configuration of requests
+// when polling for endpoint states.
+type RequestOption func(*http.Request)
+
+// WithHeader will add the provided headers to the request.
+func WithHeader(header http.Header) RequestOption {
+	return func(r *http.Request) {
+		if r.Header == nil {
+			r.Header = header
+			return
+		}
+		for key, values := range header {
+			for _, value := range values {
+				r.Header.Add(key, value)
+			}
+		}
+	}
+}

test/spoof/response_checks.go

@@ -0,0 +1,121 @@
+/*
+Copyright 2020 The Knative Authors
+
+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 spoof
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+	"sync"
+
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+// MatchesBody checks that the *first* response body matches the "expected" body, otherwise failing.
+func MatchesBody(expected string) ResponseChecker {
+	return func(resp *Response) (bool, error) {
+		if !strings.Contains(string(resp.Body), expected) {
+			// Returning (true, err) causes SpoofingClient.Poll to fail.
+			return true, fmt.Errorf("body = %s, want: %s", string(resp.Body), expected)
+		}
+
+		return true, nil
+	}
+}
+
+// MatchesAllOf combines multiple ResponseCheckers to one ResponseChecker with a logical AND. The
+// checkers are executed in order. The first function to trigger an error or a retry will short-circuit
+// the other functions (they will not be executed).
+//
+// This is useful for combining a body with a status check like:
+// MatchesAllOf(IsStatusOK, MatchesBody("test"))
+//
+// The MatchesBody check will only be executed after the IsStatusOK has passed.
+func MatchesAllOf(checkers ...ResponseChecker) ResponseChecker {
+	return func(resp *Response) (bool, error) {
+		for _, checker := range checkers {
+			if done, err := checker(resp); err != nil || !done {
+				return done, err
+			}
+		}
+		return true, nil
+	}
+}
+
+// MatchesAllBodies checks that the *first* response body matches the "expected" body, otherwise failing.
+func MatchesAllBodies(all ...string) ResponseChecker {
+	var m sync.Mutex
+	// This helps with two things:
+	// 1. we can use Equal on sets
+	// 2. it will collapse the duplicates
+	want := sets.NewString(all...)
+	seen := make(sets.String, len(all))
+
+	return func(resp *Response) (bool, error) {
+		bs := string(resp.Body)
+		for expected := range want {
+			if !strings.Contains(bs, expected) {
+				// See if the next one matches.
+				continue
+			}
+
+			m.Lock()
+			defer m.Unlock()
+			seen.Insert(expected)
+
+			// Stop once we've seen them all.
+			return want.Equal(seen), nil
+		}
+
+		// Returning (true, err) causes SpoofingClient.Poll to fail.
+		return true, fmt.Errorf("body = %s, want one of: %s", bs, all)
+	}
+}
+
+// IsStatusOK checks that the response code is a 200.
+func IsStatusOK(resp *Response) (bool, error) {
+	return IsOneOfStatusCodes(http.StatusOK)(resp)
+}
+
+// IsOneOfStatusCodes checks that the response code is equal to the given one.
+func IsOneOfStatusCodes(codes ...int) ResponseChecker {
+	return func(resp *Response) (bool, error) {
+		for _, code := range codes {
+			if resp.StatusCode == code {
+				return true, nil
+			}
+		}
+
+		return true, fmt.Errorf("status = %d %s, want one of: %v", resp.StatusCode, resp.Status, codes)
+	}
+}
+
+// Retrying modifies a ResponseChecker to retry certain response codes.
+func Retrying(rc ResponseChecker, codes ...int) ResponseChecker {
+	return func(resp *Response) (bool, error) {
+		for _, code := range codes {
+			if resp.StatusCode == code {
+				// Returning (false, nil) causes SpoofingClient.Poll to retry.
+				// sc.logger.Info("Retrying for code ", resp.StatusCode)
+				return false, nil
+			}
+		}
+
+		// If we didn't match any retryable codes, invoke the ResponseChecker that we wrapped.
+		return rc(resp)
+	}
+}

test/spoof/spoof.go

@@ -26,6 +26,7 @@ import (
 	"io/ioutil"
 	"net"
 	"net/http"
+	"net/url"
 	"time"
 
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -245,3 +246,28 @@ func (sc *SpoofingClient) logZipkinTrace(spoofResp *Response) {
 
 	sc.Logf("%s", json)
 }
+
+func (sc *SpoofingClient) WaitForEndpointState(
+	ctx context.Context,
+	url *url.URL,
+	inState ResponseChecker,
+	desc string,
+	opts ...RequestOption) (*Response, error) {
+
+	defer logging.GetEmitableSpan(ctx, "WaitForEndpointState/"+desc).End()
+
+	if url.Scheme == "" || url.Host == "" {
+		return nil, fmt.Errorf("invalid URL: %q", url.String())
+	}
+
+	req, err := http.NewRequest(http.MethodGet, url.String(), nil)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, opt := range opts {
+		opt(req)
+	}
+
+	return sc.Poll(req, inState)
+}