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.
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/compose](https://github.com/docker/compose) | patch | `v5.1.1` → `v5.1.2` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>docker/compose (docker/compose)</summary> ### [`v5.1.2`](https://github.com/docker/compose/releases/tag/v5.1.2) [Compare Source](docker/compose@v5.1.1...v5.1.2) #### What's Changed ##### 🐛 Fixes - Fix TTY timer rendering when duration length changes by [@​MaybeSam05](https://github.com/MaybeSam05) in [#​13634](docker/compose#13634) - Fix up attach filtering by [@​false200](https://github.com/false200) in [#​13664](docker/compose#13664) - Preserve ssh:// URL scheme when resolving Dockerfile path by [@​ssam18](https://github.com/ssam18) in [#​13669](docker/compose#13669) - Initialize and pass envFiles map in processExtends by [@​Mohamed-Moumni](https://github.com/Mohamed-Moumni) in [#​13678](docker/compose#13678) - Fix TestRunHook\_ConsoleSize on macOS by [@​thaJeztah](https://github.com/thaJeztah) in [#​13686](docker/compose#13686) - Restore post-connect fallback for multi-network stacks on API < 1.44 by [@​jotka](https://github.com/jotka) in [#​13629](docker/compose#13629) - Publish: return api.ErrCanceled when user declines interactive prompts by [@​ishwar170695](https://github.com/ishwar170695) in [#​13674](docker/compose#13674) - Return error on non-ErrNotExist stat failures in Tar.Sync() by [@​Lidang-Jiang](https://github.com/Lidang-Jiang) in [#​13684](docker/compose#13684) ##### 🔧 Internal - Refactor: thread context through publish sensitive data check by [@​ishwar170695](https://github.com/ishwar170695) in [#​13653](docker/compose#13653) - Add AI-powered MR review workflow via `docker/cagent-action` by [@​glours](https://github.com/glours) in [#​13659](docker/compose#13659) - Update `cagent-action` to latest (with better permissions) by [@​derekmisler](https://github.com/derekmisler) in [#​13665](docker/compose#13665) - Pin GitHub Actions to commit SHA, remove pr-review workflow by [@​glours](https://github.com/glours) in [#​13662](docker/compose#13662) - Exclude hook\_test.go from Windows builds and propagate ExecStart error in runWaitExec by [@​pawannn](https://github.com/pawannn) in [#​13683](docker/compose#13683) - Skip MR review workflow for Dependabot MRs by [@​glours](https://github.com/glours) in [#​13679](docker/compose#13679) - Use negotiated API version for network setup by [@​glours](https://github.com/glours) in [#​13690](docker/compose#13690) - Fix mixed assertion libraries in tests by [@​thaJeztah](https://github.com/thaJeztah) in [#​13689](docker/compose#13689) - Test: use random host port for dind TLS build test by [@​ricardobranco777](https://github.com/ricardobranco777) in [#​13630](docker/compose#13630) - Remove direct dependency on `docker/docker` by [@​glours](https://github.com/glours) in [#​13706](docker/compose#13706) ##### ⚙️ Dependencies - Bump github.com/containerd/platforms from `1.0.0-rc.2` to `1.0.0-rc.3` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13657](docker/compose#13657) - Bump golangci-lint to `v2.11.3` and configure CLAUDE to use it on change by [@​ndeloof](https://github.com/ndeloof) in [#​13656](docker/compose#13656) - Bump google.golang.org/grpc from `1.78.0` to `1.79.3` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13642](docker/compose#13642) - Bump github.com/moby/patternmatcher from `0.6.0` to `0.6.1` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13667](docker/compose#13667) - Bump go.opentelemetry.io/otel/sdk from `1.39.0` to `1.42.0` by [@​glours](https://github.com/glours) in [#​13663](docker/compose#13663) - Bump github.com/docker/cli from `29.2.1+incompatible` to `29.3.1+incompatible` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13670](docker/compose#13670) - Bump github.com/hashicorp/go-version from `1.8.0` to `1.9.0` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13692](docker/compose#13692) - Bump github.com/docker/buildx `v0.33.0`, buildkit `v0.29.0` by [@​thaJeztah](https://github.com/thaJeztah) in [#​13693](docker/compose#13693) - Bump google.golang.org/grpc from `1.79.3` to `1.80.0` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13697](docker/compose#13697) - Bump github.com/containerd/platforms from `1.0.0-rc.3` to `1.0.0-rc.4` by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13696](docker/compose#13696) - Bump github.com/moby/moby/client `v0.4.0`, moby/api `v1.54.1` by [@​thaJeztah](https://github.com/thaJeztah) in [#​13708](docker/compose#13708) - Bump github.com/docker/cli `v29.4.0` by [@​thaJeztah](https://github.com/thaJeztah) in [#​13707](docker/compose#13707) - Bump compose-go to version `v2.10.2` by [@​glours](https://github.com/glours) in [#​13705](docker/compose#13705) - Bump to Go `1.25.9` by [@​thaJeztah](https://github.com/thaJeztah) in [#​13720](docker/compose#13720) #### New Contributors - [@​MaybeSam05](https://github.com/MaybeSam05) made their first contribution in [#​13634](docker/compose#13634) - [@​ishwar170695](https://github.com/ishwar170695) made their first contribution in [#​13653](docker/compose#13653) - [@​derekmisler](https://github.com/derekmisler) made their first contribution in [#​13665](docker/compose#13665) - [@​false200](https://github.com/false200) made their first contribution in [#​13664](docker/compose#13664) - [@​ssam18](https://github.com/ssam18) made their first contribution in [#​13669](docker/compose#13669) - [@​Mohamed-Moumni](https://github.com/Mohamed-Moumni) made their first contribution in [#​13678](docker/compose#13678) - [@​pawannn](https://github.com/pawannn) made their first contribution in [#​13683](docker/compose#13683) - [@​jotka](https://github.com/jotka) made their first contribution in [#​13629](docker/compose#13629) - [@​Lidang-Jiang](https://github.com/Lidang-Jiang) made their first contribution in [#​13684](docker/compose#13684) **Full Changelog**: <docker/compose@v5.1.1...v5.1.2> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMTAuOCIsInVwZGF0ZWRJblZlciI6IjQzLjExMC44IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiLCJhdXRvbWF0aW9uOmJvdC1hdXRob3JlZCIsImRlcGVuZGVuY3ktdHlwZTo6cGF0Y2giXX0=-->
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