Skip to content

fix:ci panic because res=nil #4517

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix:ci panic because res=nil #4517

wants to merge 1 commit into from

Conversation

ningmingxiao
Copy link
Contributor

@ningmingxiao ningmingxiao commented Sep 16, 2025 •
edited
Loading

fix ci panic @AkihiroSuda

--- FAIL: TestLoginAgainstVariants (388.10s)
panic: test executed panic(nil) or runtime.Goexit

goroutine 1612 [running]:
testing.tRunner.func1.2({0xe2e780, 0x17fa0e0})
	/usr/local/go/src/testing/testing.go:1872 +0x237
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1875 +0x35b
runtime.Goexit()
	/usr/local/go/src/runtime/panic.go:615 +0x5e
testing.(*common).FailNow(0xc0005048c0)
	/usr/local/go/src/testing/testing.go:1013 +0x4a
gotest.tools/v3/assert.Assert({0x7fcbb722a000, 0xc0005048c0}, {0xde3880, 0x112f2e0}, {0xc00044b608, 0x1, 0x1})
	/home/rootless/go/pkg/mod/gotest.tools/[email protected]/assert/assert.go:146 +0xe2
github.com/containerd/nerdctl/v2/pkg/testutil.(*Cmd).AssertExitCode(0xc00055c000, 0x0)
	/go/src/github.com/containerd/nerdctl/pkg/testutil/testutil.go:400 +0xb5
github.com/containerd/nerdctl/v2/pkg/testutil.(*Cmd).AssertOK(0xc00055c000)
	/go/src/github.com/containerd/nerdctl/pkg/testutil/testutil.go:388 +0x33
github.com/containerd/nerdctl/v2/pkg/testutil/testregistry.EnsureImages(0xc000202200)
	/go/src/github.com/containerd/nerdctl/pkg/testutil/testregistry/testregistry_linux.go:60 +0x307
github.com/containerd/nerdctl/v2/pkg/testutil/testregistry.NewAuthServer(0xc000202200, 0xc0000fe190, 0x0, {0xc000264180, 0x2b}, {0xc000264390, 0x2c}, 0x1)
	/go/src/github.com/containerd/nerdctl/pkg/testutil/testregistry/testregistry_linux.go:66 +0x65
github.com/containerd/nerdctl/v2/cmd/nerdctl/login.TestLoginAgainstVariants.func1(0xc00022d500)
	/go/src/github.com/containerd/nerdctl/cmd/nerdctl/login/login_linux_test.go:328 +0x265
testing.tRunner(0xc00022d500, 0xc00034c450)
	/usr/local/go/src/testing/testing.go:1934 +0xea
created by testing.(*T).Run in goroutine 19
	/usr/local/go/src/testing/testing.go:1997 +0x465
FAIL cmd/nerdctl/login.TestLoginAgainstVariants (388.10s)
FAIL cmd/nerdctl/login

@ningmingxiao
fix:ci panic because res=nil
fc003df 
Signed-off-by: ningmingxiao <[email protected]>
djdongjin
djdongjin requested changes Sep 17, 2025
View reviewed changes
pkg/testutil/testutil.go
func (c *Cmd) AssertExitCode(exitCode int) {
c.Base.T.Helper()
res := c.runIfNecessary()
assert.Equal(c.Base.T, res == nil, false)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
assert.Equal(c.Base.T, res == nil, false)
assert.NotNil(c.Base.T, res)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find assert dosen't have func NotNil @djdongjin

Copy link
Member

Choose a reason for hiding this comment

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

https://pkg.go.dev/gotest.tools/v3/assert#Assert seems the right choice here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda left review comments

@djdongjin djdongjin djdongjin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ningmingxiao @AkihiroSuda @djdongjin