Skip to content

rewrite tests without ginkgo - #309

Merged
mikebrow merged 3 commits into
containerd:mainfrom
thaJeztah:gonomega
Aug 20, 2026
Merged

rewrite tests without ginkgo#309
mikebrow merged 3 commits into
containerd:mainfrom
thaJeztah:gonomega

Conversation

@thaJeztah

@thaJeztah thaJeztah commented Aug 20, 2026

Copy link
Copy Markdown
Member

@thaJeztah

Copy link
Copy Markdown
Member Author

Probably needs some further cleaning up; this was a draft I had.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Migrates core tests from Ginkgo/Gomega to Go’s testing package with Testify while incorporating the dependency updates from stacked PR #308.

Changes:

  • Rewrites test suites and helpers using standard subtests and Testify.
  • Replaces Ginkgo-based CI execution with go test race and coverage reporting.
  • Updates Logrus, Testify, YAML, and resulting transitive dependencies.

Reviewed changes

Copilot reviewed 19 out of 31 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.github/workflows/ci.yml Removes Ginkgo installation from CI.
.golangci.yml Removes obsolete dot-import exception.
Makefile Replaces Ginkgo test target with go test.
go.mod Removes Ginkgo/Gomega and updates dependencies.
go.sum Refreshes root dependency checksums.
pkg/adaptation/suite_test.go Migrates adaptation test helpers to Testify.
pkg/net/multiplex/mux_suite_test.go Rewrites multiplexer tests and concurrent helpers.
pkg/runtime-tools/generate/generate_suite_test.go Converts generator tests to table-driven subtests.
plugins/device-injector/go.mod Updates module dependencies.
plugins/device-injector/go.sum Refreshes module checksums.
plugins/differ/go.mod Updates Logrus.
plugins/differ/go.sum Refreshes module checksums.
plugins/hook-injector/go.mod Updates dependencies.
plugins/hook-injector/go.sum Refreshes module checksums.
plugins/logger/go.mod Updates Logrus and indirect dependencies.
plugins/logger/go.sum Refreshes module checksums.
plugins/network-device-injector/go.mod Updates Logrus dependencies.
plugins/network-device-injector/go.sum Refreshes module checksums.
plugins/network-logger/go.mod Updates Logrus.
plugins/network-logger/go.sum Refreshes module checksums.
plugins/rdt/go.mod Updates test and logging dependencies.
plugins/rdt/go.sum Refreshes module checksums.
plugins/template/go.mod Updates Logrus dependencies.
plugins/template/go.sum Refreshes module checksums.
plugins/ulimit-adjuster/go.mod Updates test and logging dependencies.
plugins/ulimit-adjuster/go.sum Refreshes module checksums.
plugins/wasm/go.mod Updates indirect Logrus version.
plugins/wasm/go.sum Refreshes module checksums.
plugins/writable-cgroups/go.mod Updates test and logging dependencies.
plugins/writable-cgroups/go.sum Refreshes module checksums.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/net/multiplex/mux_suite_test.go

@klihub klihub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@thaJeztah 🙏 Thank you for this ! I'd be happy to take this in already as this, and then do any further cleanup we might want to do with with subsequent PRs. I think, since any PR which touches or adds tests will almost inevitably conflict with this, the earlier we take this in the better.

PS. Gotta love that branch name...

Comment thread Makefile Outdated
@klihub
klihub requested review from mikebrow and samuelkarp August 20, 2026 07:44
@thaJeztah

Copy link
Copy Markdown
Member Author

I'd be happy to take this in already as this, and then do any further cleanup we might want to do with with subsequent PRs. I think, since any PR which touches or adds tests will almost inevitably conflict with this, the earlier we take this in the better.

Yes, I can move it out of draft; this initial rewrite was a bit AI-assisted to do the boring bits, and I tried to keep the shape "similar" to the original; I think rewriting it to be more aligning with stdlib tests would probably be good; currently we end up with some very long names that may look a bit odd;

Screenshot 2026-08-20 at 09 52 59

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 20 changed files in this pull request and generated 1 comment.

Comment thread pkg/net/multiplex/mux_suite_test.go
@thaJeztah
thaJeztah requested a balanced review from Copilot August 20, 2026 09:47
@thaJeztah
thaJeztah marked this pull request as ready for review August 20, 2026 09:49
@thaJeztah

Copy link
Copy Markdown
Member Author

OK; fixed the CoPilot comment, and made a small change to the runTable closures to return a func(*testing.T) directly to make it a bit more DRY.

I think this is in a reasonable shape now; PTAL 👍

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 20 changed files in this pull request and generated 1 comment.

Comment thread pkg/adaptation/adaptation_suite_test.go Outdated
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 20 changed files in this pull request and generated no new comments.

@klihub
klihub self-requested a review August 20, 2026 12:34

@mikebrow mikebrow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mikebrow
mikebrow merged commit 836d302 into containerd:main Aug 20, 2026
16 checks passed
@thaJeztah
thaJeztah deleted the gonomega branch August 20, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants