Skip to content
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
15 changes: 12 additions & 3 deletions .github/.golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ linters:
forbidigo:
forbid:
- pattern: time.Sleep
msg: "Please use require.Eventually or assert.Eventually instead unless you've no other option"
msg: "Please use await.Require / s.Await unless there's no better option"
- pattern: "^panic$"
msg: "Please avoid using panic in application code"
- pattern: time\.Now
Expand All @@ -48,8 +48,10 @@ linters:
msg: "FunctionalTestBase is deprecated. Use testcore.NewEnv(t) instead. See docs/development/testing.md for details."
- pattern: context\.Background\(\)
msg: "Avoid context.Background() in tests; use t.Context() to respect test timeouts and cancellation"
- pattern: '(^|\.)(Eventually|Eventuallyf|EventuallyWithT|EventuallyWithTf)(\(|$)'
msg: "Use await.Require / s.Await for assertion conditions, or await.RequireTrue / s.AwaitTrue for bool predicates, instead of testify Eventually helpers"
Comment on lines +51 to +52
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Require new code to use new Await

- pattern: 'assert\.\w+'
msg: "Use require.X / protorequire.X instead of assert.X / protoassert.X — assert doesn't stop the test on failure. assert.CollectT is still allowed for EventuallyWithT callbacks."
msg: "Use require.X / protorequire.X instead of assert.X / protoassert.X — assert doesn't stop the test on failure."
depguard:
rules:
main:
Expand Down Expand Up @@ -202,7 +204,14 @@ linters:
text: "context.Background"
linters:
- forbidigo
- text: "use of `assert\\.CollectT`" # allowed for EventuallyWithT callbacks
# Existing legacy call sites are tracked separately; keep this PR scoped
# to preventing new usage while migrating touched tests.
- path: tests/(nexus_standalone|nexus_workflow|schedule|schedule_migration)_test\.go$
text: "Eventually"
linters:
- forbidigo
- path: tests/(nexus_standalone|nexus_workflow)_test\.go$
text: "assert\\.CollectT"
linters:
- forbidigo
- text: "use of `softassert\\.\\w+`"
Expand Down
39 changes: 39 additions & 0 deletions common/testing/await/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Package await provides polling-based test assertions as a replacement
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in a doc.go file

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen both approaches (doc.go and same name as package), but happy to move it to doc.go

// for testify's Eventually, EventuallyWithT, and their formatted variants.
//
// Improvements over testify's eventually functions:
//
// - Misuse detection: accidentally using the real *testing.T (e.g. s.T() or
// suite assertion methods) instead of the callback's collect T is a
// common mistake. This package detects it and fails with a clear message.
//
// - Safer bool predicates: unlike testify's Eventually, [RequireTrue] only
// accepts func() bool, so returning false is the sole retry signal. If the
// predicate accidentally marks the real test failed, it reports that
// immediately instead of polling until timeout.
//
// - Timeout-aware callbacks: callbacks receive a context derived from the
// parent context and canceled when the await timeout or test deadline is
// reached, so RPCs and blocking waits can exit instead of continuing after
// the retry window has expired.
//
// - Panic propagation: if the condition panics (e.g. nil dereference), the
// panic is propagated immediately rather than being silently swallowed
// or retried until timeout.
// See https://github.com/stretchr/testify/issues/1810
//
// - Bounded goroutine lifetime: each attempt completes before the next
// starts, avoiding the overlapping-attempt data races and "panic: Fail
// in goroutine after Test has completed" crashes seen with testify's
// Eventually.
// See https://github.com/stretchr/testify/issues/1611
//
// - Deadlock detection: a condition that ignores t.Context() is abandoned
// after a grace period, producing a clear "does it honor t.Context()?"
// failure instead of hanging until go test -timeout.
//
// - Condition always runs: testify's Eventually can fail without ever
// running the condition due to a timer/ticker race with short timeouts.
// This package runs the condition immediately on the first iteration.
// See https://github.com/stretchr/testify/issues/1652
package await
65 changes: 65 additions & 0 deletions common/testing/await/report.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package await

import (
"fmt"
"strings"
"testing"
"time"
)

// reportAttemptErrors emits the collected attempt failures. When there are
// many, only the first and the last few are shown — long polls would
// otherwise produce hundreds of duplicate lines.
const (
reportHeadAttempts = 1
reportTailAttempts = 3
)

type attemptFailure struct {
attempt int
errors []string
}

// reportTimeout reports the timeout failure plus collected attempt errors.
func reportTimeout(tb testing.TB, failures []attemptFailure, funcName, timeoutMsg string, effectiveTimeout time.Duration, polls int) {
reportAttemptErrors(tb, failures)
if timeoutMsg != "" {
tb.Fatalf("%s: %s (not satisfied after %v, %d polls)", funcName, timeoutMsg, effectiveTimeout, polls)
} else {
tb.Fatalf("%s: condition not satisfied after %v (%d polls)", funcName, effectiveTimeout, polls)
}
}

func reportAttemptErrors(tb testing.TB, failures []attemptFailure) {
if len(failures) == 0 {
return
}

var b strings.Builder
b.WriteString("attempt errors:")
if len(failures) <= reportHeadAttempts+reportTailAttempts {
for _, f := range failures {
writeAttemptFailure(&b, f)
}
} else {
for _, f := range failures[:reportHeadAttempts] {
writeAttemptFailure(&b, f)
}
omitted := len(failures) - reportHeadAttempts - reportTailAttempts
fmt.Fprintf(&b, "\n ... %d attempts omitted ...", omitted)
for _, f := range failures[len(failures)-reportTailAttempts:] {
writeAttemptFailure(&b, f)
}
}
tb.Errorf("%s", b.String())
}

func writeAttemptFailure(b *strings.Builder, f attemptFailure) {
fmt.Fprintf(b, "\n attempt %d:", f.attempt)
for _, e := range f.errors {
for line := range strings.SplitSeq(e, "\n") {
b.WriteString("\n ")
b.WriteString(line)
}
}
}
Loading
Loading