fix mixed assertion libraries in tests#13689
Conversation
2b7112a to
0344090
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| require.Eventuallyf(t, func() bool { | ||
| poll.WaitOn(t, func(logt poll.LogT) poll.Result { | ||
| bufContents.Reset() | ||
| b.m.Lock() | ||
| defer b.m.Unlock() | ||
| if _, err := b.b.WriteTo(&bufContents); err != nil { | ||
| require.FailNowf(t, "Failed to copy from buffer", | ||
| "Error: %v", err) | ||
| return poll.Error(fmt.Errorf("failed to copy from buffer. Error: %w", err)) | ||
| } | ||
| return strings.Contains(bufContents.String(), v) | ||
| }, 2*time.Second, 20*time.Millisecond, | ||
| "Buffer did not contain %q\n============\n%s\n============", | ||
| v, &bufContents) | ||
| if !strings.Contains(bufContents.String(), v) { | ||
| return poll.Continue( | ||
| "buffer does not contain %q\n============\n%s\n============", | ||
| v, &bufContents) | ||
| } | ||
| return poll.Success() | ||
| }, | ||
| poll.WithTimeout(2*time.Second), | ||
| poll.WithDelay(20*time.Millisecond), | ||
| ) |
There was a problem hiding this comment.
CodeCov is complaining about a test-utility, but probably doesn't ignore it, because it's not in a _test.go
0344090 to
b897706
Compare
Before this, assertion libraries were mixed, sometimes
even in the same file.
git grep -l '"gotest.tools/v3/' | wc -l
75
git grep -l '"github.com/stretchr/testify' | wc -l
24
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
b897706 to
cd32975
Compare
There was a problem hiding this comment.
Pull request overview
This PR standardizes the Go test assertion library usage across the repository by migrating tests away from stretchr/testify to gotest.tools/v3/assert, and tightens lint enforcement to prevent reintroducing testify.
Changes:
- Replace
testify/assert+testify/requireusages in multiple unit/e2e tests withgotest.tools/v3/assert(andassert/cmpwhere needed). - Remove
github.com/stretchr/testifyfromgo.modand add golangci-lintforbidigorules to forbid testify imports going forward. - Adjust a few tests that depended on testify-specific helpers (e.g., element matching) by sorting or switching to deep-equality/regex comparisons.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/watch/watcher_naive_test.go | Switch require assertions to gotest.tools assert.NilError. |
| pkg/watch/paths_test.go | Switch to gotest.tools asserts; update Windows path literal; use ErrorContains. |
| pkg/watch/notify_test.go | Replace testify asserts with gotest.tools equivalents. |
| pkg/watch/ephemeral_test.go | Replace testify formatting assertions with gotest.tools NilError/Assert. |
| pkg/utils/set_test.go | Replace testify checks; sort set elements before deep-equality where needed. |
| pkg/utils/safebuffer.go | Replace require.Eventuallyf with poll.WaitOn for eventual assertions. |
| pkg/e2e/watch_test.go | Replace require usage with assert.NilError. |
| pkg/e2e/up_test.go | Replace require usage with gotest.tools assertions. |
| pkg/e2e/start_stop_test.go | Replace testify regex assertions with gotest.tools assert/cmp regex comparison. |
| pkg/e2e/scale_test.go | Remove testify failure helper usage; fail via t.Fatalf. |
| pkg/e2e/restart_test.go | Replace testify regex assertions with gotest.tools assert/cmp. |
| pkg/e2e/ps_test.go | Replace testify assert/require with gotest.tools asserts + assert/cmp. |
| pkg/e2e/pause_test.go | Replace require usage; add extra error-type assertion for network error. |
| pkg/e2e/networks_test.go | Replace strings.Contains assertion with gotest.tools is.Contains. |
| pkg/e2e/framework.go | Replace require usage with gotest.tools (and t.Fatal in one spot). |
| pkg/e2e/compose_test.go | Replace testify regex assertion with gotest.tools assert/cmp. |
| pkg/e2e/build_test.go | Replace require usage with gotest.tools assert/cmp contains. |
| pkg/e2e/assert.go | Replace require usage with gotest.tools assertions + assert/cmp. |
| pkg/compose/watch_test.go | Replace ElementsMatch with deterministic sort + assert.DeepEqual. |
| pkg/compose/viz_test.go | Replace testify assertions with gotest.tools asserts + assert/cmp. |
| pkg/compose/logs_test.go | Replace testify assertions with gotest.tools asserts + assert/cmp. |
| pkg/compose/loader_test.go | Replace testify assertions with gotest.tools asserts + assert/cmp. |
| pkg/compose/dependencies_test.go | Replace testify assertions with gotest.tools asserts + assert/cmp. |
| internal/tracing/tracing_test.go | Replace require with gotest.tools assertions. |
| internal/tracing/attributes_test.go | Replace require with gotest.tools assertions. |
| internal/desktop/client_test.go | Replace require with gotest.tools assertions. |
| cmd/compose/viz_test.go | Replace testify assertions with gotest.tools asserts; adjust error expectation. |
| cmd/compose/top_test.go | Replace testify assertions with gotest.tools deep-equality and NilError. |
| cmd/compose/options_test.go | Replace require assertions with gotest.tools assertions. |
| go.mod | Remove testify (and related indirect deps) from module requirements. |
| .golangci.yml | Add forbidigo rules to forbid testify imports (assert/require/suite). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| src, err := os.Open(sourceFile) | ||
| require.NoError(t, err, "Failed to open source file: %s") | ||
| assert.NilError(t, err, "Failed to open source file: %s") |
There was a problem hiding this comment.
The failure message uses a %s format verb but no argument is provided, so the output will show a formatting "(MISSING)" placeholder. Pass sourceFile as the formatting argument (or remove the verb) so the error message is meaningful.
| assert.NilError(t, err, "Failed to open source file: %s") | |
| assert.NilError(t, err, "Failed to open source file: %s", sourceFile) |
| github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 | ||
| github.com/spf13/cobra v1.10.2 | ||
| github.com/spf13/pflag v1.0.10 | ||
| github.com/stretchr/testify v1.11.1 | ||
| github.com/tilt-dev/fsnotify v1.4.8-0.20220602155310-fff9c274a375 | ||
| go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.63.0 | ||
| go.opentelemetry.io/otel v1.42.0 |
There was a problem hiding this comment.
go.mod removes github.com/stretchr/testify (and related indirect deps), but go.sum is not updated in this PR. The repo's vendor-validate target runs go mod tidy and fails if go.sum differs, so please run make go-mod-tidy (or go mod tidy) and commit the resulting go.sum changes.
Before this, assertion libraries were mixed, sometimes even in the same file.
What I did
Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did