Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make EventuallyWithT concurrency safe #1395

Merged
merged 2 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 20 additions & 19 deletions assert/assertions.go
Original file line number Diff line number Diff line change
Expand Up @@ -1873,23 +1873,18 @@ func (c *CollectT) Errorf(format string, args ...interface{}) {
}

// FailNow panics.
func (c *CollectT) FailNow() {
func (*CollectT) FailNow() {
panic("Assertion failed")
}

// Reset clears the collected errors.
func (c *CollectT) Reset() {
c.errors = nil
// Deprecated: That was a method for internal usage that should not have been published. Now just panics.
func (*CollectT) Reset() {
panic("Reset() is deprecated")
}

// Copy copies the collected errors to the supplied t.
func (c *CollectT) Copy(t TestingT) {
if tt, ok := t.(tHelper); ok {
tt.Helper()
}
for _, err := range c.errors {
t.Errorf("%v", err)
}
// Deprecated: That was a method for internal usage that should not have been published. Now just panics.
func (*CollectT) Copy(TestingT) {
panic("Copy() is deprecated")
}

// EventuallyWithT asserts that given condition will be met in waitFor time,
Expand All @@ -1915,8 +1910,8 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time
h.Helper()
}

collect := new(CollectT)
ch := make(chan bool, 1)
var lastFinishedTickErrs []error
ch := make(chan []error, 1)

timer := time.NewTimer(waitFor)
defer timer.Stop()
Expand All @@ -1927,19 +1922,25 @@ func EventuallyWithT(t TestingT, condition func(collect *CollectT), waitFor time
for tick := ticker.C; ; {
select {
case <-timer.C:
collect.Copy(t)
for _, err := range lastFinishedTickErrs {
t.Errorf("%v", err)
}
return Fail(t, "Condition never satisfied", msgAndArgs...)
case <-tick:
tick = nil
collect.Reset()
go func() {
collect := new(CollectT)
defer func() {
ch <- collect.errors
}()
condition(collect)
ch <- len(collect.errors) == 0
}()
case v := <-ch:
if v {
case errs := <-ch:
if len(errs) == 0 {
return true
}
// Keep the errors from the last ended condition, so that they can be copied to t if timeout is reached.
lastFinishedTickErrs = errs
tick = ticker.C
}
}
Expand Down
52 changes: 49 additions & 3 deletions assert/assertions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2766,19 +2766,30 @@ func TestEventuallyTrue(t *testing.T) {
True(t, Eventually(t, condition, 100*time.Millisecond, 20*time.Millisecond))
}

// errorsCapturingT is a mock implementation of TestingT that captures errors reported with Errorf.
type errorsCapturingT struct {
errors []error
}

func (t *errorsCapturingT) Errorf(format string, args ...interface{}) {
t.errors = append(t.errors, fmt.Errorf(format, args...))
}
dolmen marked this conversation as resolved.
Show resolved Hide resolved

func (t *errorsCapturingT) Helper() {}

func TestEventuallyWithTFalse(t *testing.T) {
mockT := new(CollectT)
mockT := new(errorsCapturingT)

condition := func(collect *CollectT) {
True(collect, false)
Fail(collect, "condition fixed failure")
}

False(t, EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond))
Len(t, mockT.errors, 2)
}

func TestEventuallyWithTTrue(t *testing.T) {
mockT := new(CollectT)
mockT := new(errorsCapturingT)

state := 0
condition := func(collect *CollectT) {
Expand All @@ -2792,6 +2803,41 @@ func TestEventuallyWithTTrue(t *testing.T) {
Len(t, mockT.errors, 0)
}

func TestEventuallyWithT_ConcurrencySafe(t *testing.T) {
dolmen marked this conversation as resolved.
Show resolved Hide resolved
mockT := new(errorsCapturingT)

condition := func(collect *CollectT) {
Fail(collect, "condition fixed failure")
}

// To trigger race conditions, we run EventuallyWithT with a nanosecond tick.
False(t, EventuallyWithT(mockT, condition, 100*time.Millisecond, time.Nanosecond))
Len(t, mockT.errors, 2)
}

func TestEventuallyWithT_ReturnsTheLatestFinishedConditionErrors(t *testing.T) {
// We'll use a channel to control whether a condition should sleep or not.
mustSleep := make(chan bool, 2)
mustSleep <- false
mustSleep <- true
close(mustSleep)

condition := func(collect *CollectT) {
if <-mustSleep {
// Sleep to ensure that the second condition runs longer than timeout.
time.Sleep(time.Second)
return
}
dolmen marked this conversation as resolved.
Show resolved Hide resolved

// The first condition will fail. We expect to get this error as a result.
Fail(collect, "condition fixed failure")
}

mockT := new(errorsCapturingT)
False(t, EventuallyWithT(mockT, condition, 100*time.Millisecond, 20*time.Millisecond))
Len(t, mockT.errors, 2)
}

func TestNeverFalse(t *testing.T) {
condition := func() bool {
return false
Expand Down