Repository: gavv/httpexpect
Pull request #198 (2022-12-10 17:12:00)

Add retry sleep with context
Issue: https://github.com/gavv/httpexpect/issues/163

Changes:

- Update `sleepFn` to return `<-chan time.Time`
- Update unit test
- Update e2e test

neverbeenthisweeb: hi @gavv wdyt about this? will proceed to e2e test when impl and unit test are OK
coveralls: 
[![Coverage Status](https://coveralls.io/builds/55042048/badge)](https://coveralls.io/builds/55042048)

Coverage increased (+0.006%) to 93.053% when pulling **21d2838b84d551f43c65bf035589e955cd58a244 on neverbeenthisweeb:163-sleep-with-context** into **213c224c2fc02424934bce5f394727eec568f9d8 on gavv:master**.

neverbeenthisweeb: updated, ready for review
neverbeenthisweeb: updated, ready for review

Patch:
diff --git a/assertion_validation.go b/assertion_validation.go
index 6ce1e29..c8c37b4 100644
--- a/assertion_validation.go
+++ b/assertion_validation.go
@@ -3,6 +3,7 @@ package httpexpect
 import (
 	"errors"
 	"fmt"
+	"reflect"
 )
 
 func validateAssertion(failure *AssertionFailure) error {
@@ -130,6 +131,24 @@ func validateTraits(failure *AssertionFailure, traits fieldTraits) error {
 		break
 	}
 
+	if traits.Range == fieldRequired || traits.Range == fieldOptional {
+		if failure.Expected != nil {
+			if _, ok := failure.Expected.Value.(*AssertionRange); ok {
+				return errors.New(
+					"AssertionValue should contain AssertionRange, not *AssertionRange")
+			}
+
+			if rng, ok := failure.Expected.Value.(AssertionRange); ok {
+				if rng.Min == nil {
+					return errors.New("AssertionRange value should have non-nil Min field")
+				}
+				if rng.Max == nil {
+					return errors.New("AssertionRange value should have non-nil Max field")
+				}
+			}
+		}
+	}
+
 	switch traits.Range {
 	case fieldRequired:
 		if failure.Expected == nil {
@@ -138,18 +157,11 @@ func validateTraits(failure *AssertionFailure, traits fieldTraits) error {
 				failure.Type)
 		}
 
-		if rng, ok := failure.Expected.Value.(AssertionRange); !ok {
+		if _, ok := failure.Expected.Value.(AssertionRange); !ok {
 			return fmt.Errorf(
 				"AssertionFailure of type %s"+
 					" should have Expected field with AssertionRange value",
 				failure.Type)
-		} else {
-			if rng.Min == nil {
-				return errors.New("AssertionRange value should have non-nil Min field")
-			}
-			if rng.Max == nil {
-				return errors.New("AssertionRange value should have non-nil Max field")
-			}
 		}
 
 	case fieldDenied:
@@ -159,6 +171,27 @@ func validateTraits(failure *AssertionFailure, traits fieldTraits) error {
 		break
 	}
 
+	if traits.Range == fieldRequired || traits.Range == fieldOptional {
+		if failure.Expected != nil {
+			if _, ok := failure.Expected.Value.(*AssertionList); ok {
+				return errors.New(
+					"AssertionValue should contain AssertionList, not *AssertionList")
+			}
+
+			if lst, ok := failure.Expected.Value.(AssertionList); ok {
+				if len(lst) == 0 {
+					return errors.New("AssertionList should be non-empty")
+				}
+
+				if len(lst) == 1 && reflect.ValueOf(lst[0]).Kind() == reflect.Slice {
+					return errors.New(
+						"AssertionList should contain a list of values," +
+							" but it contains a single element which itself is a list")
+				}
+			}
+		}
+	}
+
 	switch traits.List {
 	case fieldRequired:
 		if failure.Expected == nil {
@@ -167,15 +200,11 @@ func validateTraits(failure *AssertionFailure, traits fieldTraits) error {
 				failure.Type)
 		}
 
-		if lst, ok := failure.Expected.Value.(AssertionList); !ok {
+		if _, ok := failure.Expected.Value.(AssertionList); !ok {
 			return fmt.Errorf(
 				"AssertionFailure of type %s"+
 					" should have Expected field with AssertionList value",
 				failure.Type)
-		} else {
-			if len(lst) == 0 {
-				return errors.New("AssertionList can't be empty")
-			}
 		}
 
 	case fieldDenied:
diff --git a/request.go b/request.go
index 5715d1b..c7d03a3 100644
--- a/request.go
+++ b/request.go
@@ -38,7 +38,7 @@ type Request struct {
 	maxRetries    int
 	minRetryDelay time.Duration
 	maxRetryDelay time.Duration
-	sleepFn       func(d time.Duration)
+	sleepFn       func(d time.Duration) <-chan time.Time
 
 	timeout time.Duration
 
@@ -123,7 +123,9 @@ func newRequest(
 		maxRetries:    0,
 		minRetryDelay: time.Millisecond * 50,
 		maxRetryDelay: time.Second * 5,
-		sleepFn:       time.Sleep,
+		sleepFn: func(d time.Duration) <-chan time.Time {
+			return time.After(d)
+		},
 	}
 
 	r.initPath(path, pathargs...)
@@ -1922,7 +1924,15 @@ func (r *Request) retryRequest(reqFunc func() (*http.Response, error)) (
 			resp.Body.Close()
 		}
 
-		r.sleepFn(delay)
+		if configCtx := r.config.Context; configCtx != nil {
+			select {
+			case <-configCtx.Done():
+				return nil, elapsed, configCtx.Err()
+			case <-r.sleepFn(delay):
+			}
+		} else {
+			<-r.sleepFn(delay)
+		}
 
 		delay *= 2
 		if delay > r.maxRetryDelay {


Test patch:
diff --git a/assertion_test.go b/assertion_test.go
index 5d108aa..f18fefd 100644
--- a/assertion_test.go
+++ b/assertion_test.go
@@ -279,6 +279,18 @@ func TestAssertionValidation(t *testing.T) {
 				Expected: &AssertionValue{"test"},
 			},
 		},
+		{
+			testName:          "Range is pointer",
+			errorContainsText: "AssertionRange",
+			input: AssertionFailure{
+				Type: AssertInRange,
+				Errors: []error{
+					errors.New("test"),
+				},
+				Actual:   &AssertionValue{},
+				Expected: &AssertionValue{&AssertionRange{Min: 0, Max: 0}},
+			},
+		},
 		{
 			testName:          "Range Min is nil",
 			errorContainsText: "Min",
@@ -339,6 +351,18 @@ func TestAssertionValidation(t *testing.T) {
 				Expected: &AssertionValue{"test"},
 			},
 		},
+		{
+			testName:          "List is pointer",
+			errorContainsText: "AssertionList",
+			input: AssertionFailure{
+				Type: AssertBelongs,
+				Errors: []error{
+					errors.New("test"),
+				},
+				Actual:   &AssertionValue{},
+				Expected: &AssertionValue{&AssertionList{1}},
+			},
+		},
 		{
 			testName:          "List is typed nil",
 			errorContainsText: "AssertionList",
@@ -363,6 +387,18 @@ func TestAssertionValidation(t *testing.T) {
 				Expected: &AssertionValue{AssertionList{}},
 			},
 		},
+		{
+			testName:          "List has one element and it's list",
+			errorContainsText: "AssertionList",
+			input: AssertionFailure{
+				Type: AssertBelongs,
+				Errors: []error{
+					errors.New("test"),
+				},
+				Actual:   &AssertionValue{},
+				Expected: &AssertionValue{AssertionList{[]string{"test"}}},
+			},
+		},
 	}
 
 	for _, test := range tests {
diff --git a/e2e_context_test.go b/e2e_context_test.go
index ea18e62..7b862c4 100644
--- a/e2e_context_test.go
+++ b/e2e_context_test.go
@@ -393,3 +393,95 @@ func TestContextPerRequestWithTimeoutCancelledByContext(t *testing.T) {
 	// expected error should occur
 	assert.True(t, suppressor.expErrorOccurred)
 }
+
+func TestContextPerRequestRetry(t *testing.T) {
+	var m sync.Mutex
+	TimeOutDuration := 5 * time.Minute
+
+	t.Run("not cancelled", func(t *testing.T) {
+		var callCount int
+		retryDelayDuration := 1 * time.Second
+
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			m.Lock()
+			defer m.Unlock()
+			callCount++
+
+			if callCount > 1 {
+				w.WriteHeader(http.StatusOK)
+				return
+			}
+
+			w.WriteHeader(http.StatusInternalServerError)
+		}))
+		defer ts.Close()
+
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+
+		e := WithConfig(Config{
+			BaseURL:          ts.URL,
+			AssertionHandler: &mockAssertionHandler{},
+		})
+
+		e.GET("/").
+			WithContext(ctx).
+			WithMaxRetries(1).
+			WithRetryPolicy(RetryAllErrors).
+			WithRetryDelay(retryDelayDuration, TimeOutDuration).
+			Expect()
+
+		assert.Equal(t, 2, callCount)
+	})
+
+	t.Run("cancelled after first retry attempt", func(t *testing.T) {
+		var callCount int
+		var isCtxCancelled bool
+		ctxCancellation := make(chan bool, 1) // To cancel context after first retry attempt
+
+		ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			m.Lock()
+			defer m.Unlock()
+			callCount++
+
+			if callCount == 2 {
+				ctxCancellation <- true
+			}
+
+			w.WriteHeader(http.StatusInternalServerError)
+		}))
+		defer ts.Close()
+
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		go func() {
+			<-ctxCancellation
+			cancel()
+			m.Lock()
+			defer m.Unlock()
+			isCtxCancelled = true
+		}()
+
+		// Config with context cancelled error
+		suppressor := newExpErrorSuppressor(t,
+			func(err error) bool {
+				return strings.Contains(err.Error(), context.Canceled.Error())
+			})
+
+		e := WithConfig(Config{
+			BaseURL:          ts.URL,
+			AssertionHandler: suppressor,
+		})
+
+		e.GET("/").
+			WithContext(ctx).
+			WithMaxRetries(100).
+			WithRetryPolicy(RetryAllErrors).
+			WithRetryDelay(10*time.Millisecond, 50*time.Millisecond).
+			Expect()
+
+		assert.Equal(t, 2, callCount)
+		assert.True(t, isCtxCancelled)
+		assert.True(t, suppressor.expErrorOccurred)
+	})
+}
diff --git a/request_test.go b/request_test.go
index 5340680..9683b30 100644
--- a/request_test.go
+++ b/request_test.go
@@ -2255,7 +2255,9 @@ func TestRequestRetry(t *testing.T) {
 		}
 	}
 
-	noopSleepFn := func(time.Duration) {}
+	noopSleepFn := func(time.Duration) <-chan time.Time {
+		return time.After(0)
+	}
 
 	t.Run("dont retry policy", func(t *testing.T) {
 		t.Run("no error", func(t *testing.T) {
@@ -2812,8 +2814,9 @@ func TestRequestRetry(t *testing.T) {
 				WithRetryPolicy(RetryAllErrors).
 				WithMaxRetries(3).
 				WithRetryDelay(100*time.Millisecond, 1000*time.Millisecond)
-			req.sleepFn = func(d time.Duration) {
+			req.sleepFn = func(d time.Duration) <-chan time.Time {
 				totalSleepTime += d
+				return time.After(0)
 			}
 			req.chain.assertOK(t)
 
@@ -2848,9 +2851,11 @@ func TestRequestRetry(t *testing.T) {
 				WithRetryPolicy(RetryAllErrors).
 				WithMaxRetries(3).
 				WithRetryDelay(100*time.Millisecond, 300*time.Millisecond)
-			req.sleepFn = func(d time.Duration) {
+			req.sleepFn = func(d time.Duration) <-chan time.Time {
 				totalSleepTime += d
+				return time.After(0)
 			}
+
 			req.chain.assertOK(t)
 
 			resp := req.Expect().
@@ -2861,4 +2866,40 @@ func TestRequestRetry(t *testing.T) {
 			assert.Equal(t, int64(100+200+300), totalSleepTime.Milliseconds())
 		})
 	})
+
+	t.Run("cancelled retries", func(t *testing.T) {
+		callCount := 0
+
+		client := newHTTPErrClient(func(req *http.Request) {
+			callCount++
+
+			assert.Error(t, req.Context().Err(), context.Canceled.Error())
+
+			b, err := ioutil.ReadAll(req.Body)
+			assert.NoError(t, err)
+			assert.Equal(t, "test body", string(b))
+		})
+
+		config := Config{
+			Client:   client,
+			Reporter: reporter,
+		}
+
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel() // Cancel immediately to trigger error
+
+		req := NewRequest(config, http.MethodPost, "/url").
+			WithText("test body").
+			WithRetryPolicy(RetryAllErrors).
+			WithMaxRetries(1).
+			WithContext(ctx).
+			WithRetryDelay(1*time.Minute, 5*time.Minute)
+		req.chain.assertOK(t)
+
+		resp := req.Expect()
+		resp.chain.assertFailed(t)
+
+		// Should not retry
+		assert.Equal(t, 1, callCount)
+	})
 }