assert: report failures instead of panicking in Eventually and Never

assert/assertions.go

@@ -2009,6 +2009,25 @@ func Eventually(t TestingT, condition func() bool, waitFor time.Duration, tick t
 		h.Helper()
 	}
 
+	// A nil condition would be dereferenced on the goroutine below, where the
+	// panic cannot be recovered by the caller and takes the whole binary down.
+	// Reported as a failure rather than treated as vacuously satisfied: Never is
+	// given the same treatment as Eventually so that one malformed argument fails
+	// the same way in all three entry points, instead of Never quietly passing
+	// because there was no condition for it to violate.
+	if condition == nil {
+		return Fail(t, "Condition must not be nil", msgAndArgs...)
+	}
+
+	// The ticker is built here, before the condition has been called even once, and
+	// time.NewTicker panics on a non-positive interval. That panic happens in the
+	// caller's goroutine, so it aborts the test run instead of failing one
+	// assertion -- and there is nothing to wait for, since a zero or negative tick
+	// can never be a usable polling interval.
+	if tick <= 0 {
+		return Fail(t, "Tick must be positive", msgAndArgs...)
+	}
+
 	ch := make(chan bool, 1)
 	checkCond := func() { ch <- condition() }
 
@@ -2104,6 +2123,25 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time
 		h.Helper()
 	}
 
+	// A nil condition would be dereferenced on the goroutine below, where the
+	// panic cannot be recovered by the caller and takes the whole binary down.
+	// Reported as a failure rather than treated as vacuously satisfied: Never is
+	// given the same treatment as Eventually so that one malformed argument fails
+	// the same way in all three entry points, instead of Never quietly passing
+	// because there was no condition for it to violate.
+	if condition == nil {
+		return Fail(t, "Condition must not be nil", msgAndArgs...)
+	}
+
+	// The ticker is built here, before the condition has been called even once, and
+	// time.NewTicker panics on a non-positive interval. That panic happens in the
+	// caller's goroutine, so it aborts the test run instead of failing one
+	// assertion -- and there is nothing to wait for, since a zero or negative tick
+	// can never be a usable polling interval.
+	if tick <= 0 {
+		return Fail(t, "Tick must be positive", msgAndArgs...)
+	}
+
 	var lastFinishedTickErrs []error
 	ch := make(chan *CollectT, 1)
 
@@ -2156,6 +2194,25 @@ func Never(t TestingT, condition func() bool, waitFor time.Duration, tick time.D
 		h.Helper()
 	}
 
+	// A nil condition would be dereferenced on the goroutine below, where the
+	// panic cannot be recovered by the caller and takes the whole binary down.
+	// Reported as a failure rather than treated as vacuously satisfied: Never is
+	// given the same treatment as Eventually so that one malformed argument fails
+	// the same way in all three entry points, instead of Never quietly passing
+	// because there was no condition for it to violate.
+	if condition == nil {
+		return Fail(t, "Condition must not be nil", msgAndArgs...)
+	}
+
+	// The ticker is built here, before the condition has been called even once, and
+	// time.NewTicker panics on a non-positive interval. That panic happens in the
+	// caller's goroutine, so it aborts the test run instead of failing one
+	// assertion -- and there is nothing to wait for, since a zero or negative tick
+	// can never be a usable polling interval.
+	if tick <= 0 {
+		return Fail(t, "Tick must be positive", msgAndArgs...)
+	}
+
 	ch := make(chan bool, 1)
 	checkCond := func() { ch <- condition() }
 

assert/assertions_test.go

@@ -3634,6 +3634,40 @@ func TestNeverFailQuickly(t *testing.T) {
 	False(t, Never(mockT, condition, 100*time.Millisecond, time.Second))
 }
 
+// A nil condition used to be dereferenced on the goroutine Eventually/Never
+// spawn, so the panic could not be recovered by the caller and took the whole
+// test binary down.
+func TestEventuallyNilCondition(t *testing.T) {
+	t.Parallel()
+
+	mockT := new(testing.T)
+
+	NotPanics(t, func() {
+		False(t, Eventually(mockT, nil, 100*time.Millisecond, 20*time.Millisecond))
+	})
+	False(t, Never(mockT, nil, 100*time.Millisecond, 20*time.Millisecond))
+
+	mockCollectT := new(CollectT)
+	False(t, EventuallyWithT(mockCollectT, nil, 100*time.Millisecond, 20*time.Millisecond))
+	Len(t, mockCollectT.errors, 1)
+}
+
+// time.NewTicker panics on a non-positive interval, and the ticker is created
+// unconditionally even though it is only read after the first result.
+func TestEventuallyNonPositiveTick(t *testing.T) {
+	t.Parallel()
+
+	for _, tick := range []time.Duration{0, -time.Second} {
+		mockT := new(testing.T)
+
+		NotPanics(t, func() {
+			False(t, Eventually(mockT, func() bool { return true }, 100*time.Millisecond, tick))
+			False(t, Never(mockT, func() bool { return true }, 100*time.Millisecond, tick))
+			False(t, EventuallyWithT(mockT, func(*CollectT) {}, 100*time.Millisecond, tick))
+		})
+	}
+}
+
 func Test_validateEqualArgs(t *testing.T) {
 	t.Parallel()