test: refactor compose_ps_linux_test.go to use tigron by Siddhesh002 · Pull Request #4790 · containerd/nerdctl

Siddhesh002 · 2026-03-12T15:14:41Z

Refactor compose_ps_linux_test.go to use tigron testing framework

Related: #4613

mikebrow · 2026-03-13T14:18:08Z

cmd/nerdctl/compose/compose_ps_linux_test.go

@@ -27,10 +27,20 @@ import (

 	"github.com/containerd/nerdctl/v2/pkg/tabutil"


I'm on VSC using the GO tools extension from google. It has a setting for the GO Format tool that defaults to using gofmt. With that extension plugged in gofmt sorts the imports on saving the file. git commit --amend .. push the commit to your origin and should be all good to go

Thanks for the suggestion
I've applied the formatting fix and amended the commit
Looks like the lint issue is resolved now

mikebrow · 2026-03-13T21:28:42Z

log of your modified test cases appears correct, not sure about all the flake/other test issues.

https://github.com/containerd/nerdctl/actions/runs/23056928875/job/66973560088?pr=4790

PASS cmd/nerdctl/compose.TestComposePs/compose_ps_alpine_-a (0.04s)
PASS cmd/nerdctl/compose.TestComposePs/compose_ps_wordpress (0.06s)
PASS cmd/nerdctl/compose.TestComposePs/compose_ps_should_not_show_alpine_unless_running (0.09s)
PASS cmd/nerdctl/compose.TestComposePs/compose_ps_db (0.11s)
PASS cmd/nerdctl/compose.TestComposePs/compose_ps_services (0.09s)
PASS cmd/nerdctl/compose.TestComposePs/compose_ps_filter_exited (0.09s)
PASS cmd/nerdctl/compose.TestComposePs (24.10s)
PASS cmd/nerdctl/compose.TestComposePsJSON/ps_json_all_services (0.03s)
PASS cmd/nerdctl/compose.TestComposePsJSON/wordpress_running (0.03s)
PASS cmd/nerdctl/compose.TestComposePsJSON/stop_wordpress (1.16s)
PASS cmd/nerdctl/compose.TestComposePsJSON/wordpress_exited (0.03s)
PASS cmd/nerdctl/compose.TestComposePsJSON/remove_wordpress (6.12s)
PASS cmd/nerdctl/compose.TestComposePsJSON/wordpress_removed (0.02s)
PASS cmd/nerdctl/compose.TestComposePsJSON/unsupported_format_should_fail (0.01s)
PASS cmd/nerdctl/compose.TestComposePsJSON (20.79s)

haytok

Thanks for creating this PR! I’ve made a few comments, so could you check when you have time?

haytok · 2026-03-13T05:42:10Z

cmd/nerdctl/compose/compose_ps_linux_test.go

-	base := testutil.NewBase(t)
+	testCase := nerdtest.Setup()
+
+	testCase.NoParallel = true


Does NoParallel need to be true?

https://github.com/containerd/nerdctl/blob/ccabeb1caa8c66786547b94de3b5124f7de7966a/docs/testing/tools.md#parallelism

haytok · 2026-03-13T05:43:23Z

cmd/nerdctl/compose/compose_ps_linux_test.go

+	testCase := nerdtest.Setup()
+
+	testCase.NoParallel = true
+	testCase.Require = nerdtest.Private


Is it really necessary to keep it private?

https://github.com/containerd/nerdctl/blob/ccabeb1caa8c66786547b94de3b5124f7de7966a/docs/testing/tools.md#about-nerdtestprivate

haytok · 2026-03-13T05:46:13Z

cmd/nerdctl/compose/compose_ps_linux_test.go

+
+		helpers.Ensure("compose", "-f", composePath, "up", "-d")
+
+		time.Sleep(2 * time.Second)


Is this used to ensure the container starts? If so, you can use EnsureContainerStarted.

haytok · 2026-03-13T05:47:12Z

cmd/nerdctl/compose/compose_ps_linux_test.go

 }

 func TestComposePsJSON(t *testing.T) {
+


IMO, this new line is unnecessary.

haytok · 2026-03-13T05:47:54Z

cmd/nerdctl/compose/compose_ps_linux_test.go

+
+		helpers.Ensure("compose", "-f", composePath, "up", "-d")
+
+		time.Sleep(2 * time.Second)


Can we use EnsureContainerStarted ?

haytok · 2026-03-13T05:56:56Z

cmd/nerdctl/compose/compose_ps_linux_test.go

 func TestComposePsJSON(t *testing.T) {
+
 	// docker parses unknown 'format' as a Go template and won't output an error
 	testutil.DockerIncompatible(t)


Could you re-write using nerdtest.Docker

haytok · 2026-03-14T03:14:44Z

cmd/nerdctl/compose/compose_ps_linux_test.go

+	testCase := nerdtest.Setup()
+
+	testCase.NoParallel = true
+	testCase.Require = nerdtest.Private


Same as above.

haytok · 2026-03-14T03:29:33Z

log of your modified test cases appears correct, not sure about all the flake/other test issues.
https://github.com/containerd/nerdctl/actions/runs/23056928875/job/66973560088?pr=4790

Thanks for checking. The tests that are currently failing are flaky ... :( So they need to be retried in CI.

=== Failing tests ===
TestLogs
TestLogs/since_60s
TestLogs/until_60s
=====================

=== Failing tests ===
TestLoadStdinFromPipe
=====================

=== Failing tests ===
TestIPFSAddrWithKubo
TestLoadQuiet
=====================

Since the tests that @Siddhesh002 refactored are passing, it would be better to retry them once the fixes are complete.

Signed-off-by: Siddhesh Suryawanshi <siddheshsuryawanshi@ibm.com>

Siddhesh002 · 2026-03-25T10:11:50Z

Thanks for the review! @haytok

Removed NoParallel
Kept nerdtest.Private for isolation
Replaced time.Sleep with EnsureContainerStarted
Updated Docker check to require.Not(nerdtest.Docker)

Let me know if any further changes are needed

haytok · 2026-03-28T08:42:12Z

cmd/nerdctl/compose/compose_ps_linux_test.go

-	comp := testutil.NewComposeDir(t, dockerComposeYAML)
-	defer comp.CleanUp()
-	projectName := comp.ProjectName()
-	t.Logf("projectName=%q", projectName)


Please write this deleted row in testCase.Setup.

haytok · 2026-03-28T12:05:18Z

cmd/nerdctl/compose/compose_ps_linux_test.go

+	}
+
+	testCase.SubTests = []*test.Case{
+


NIT: Even when considering the tests for other refactoring PRs, there are no new lines between the tests, so it’s better to remove them (e.g., L117, L127, L137, ...). Please check the other relevant sections as well.

haytok · 2026-03-28T12:07:49Z

cmd/nerdctl/compose/compose_ps_linux_test.go

-	base.ComposeCmd("-f", comp.YAMLFullPath(), "ps", "--format", "json", "wordpress").
-		AssertOutWithFunc(assertHandler("wordpress", 0))
+	testCase.SubTests = []*test.Case{
+


NIT: Even when considering the tests for other refactoring PRs, there are no new lines between the tests, so it’s better to remove them (e.g., L262, L278, ...). Please check the other relevant sections as well.

haytok · 2026-03-28T12:08:21Z

cmd/nerdctl/compose/compose_ps_linux_test.go

-	comp := testutil.NewComposeDir(t, dockerComposeYAML)
-	defer comp.CleanUp()
-	projectName := comp.ProjectName()
-	t.Logf("projectName=%q", projectName)


Please write this deleted row in testCase.Setup.

AkihiroSuda mentioned this pull request Mar 12, 2026

Rewrite tests using testutil.NewBase to use nerdtest.Setup (Tigron) #4613

Open

46 tasks

mikebrow reviewed Mar 13, 2026

View reviewed changes

Siddhesh002 force-pushed the refactor-compose-ps-tests branch from 38f7c72 to d2d0621 Compare March 13, 2026 15:04

haytok reviewed Mar 14, 2026

View reviewed changes

test: refactor compose_ps_linux_test.go to use tigron

f8495d3

Signed-off-by: Siddhesh Suryawanshi <siddheshsuryawanshi@ibm.com>

Siddhesh002 force-pushed the refactor-compose-ps-tests branch from d2d0621 to f8495d3 Compare March 25, 2026 08:13

haytok reviewed Mar 28, 2026

View reviewed changes

		@@ -27,10 +27,20 @@ import (

		"github.com/containerd/nerdctl/v2/pkg/tabutil"


		helpers.Ensure("compose", "-f", composePath, "up", "-d")

		time.Sleep(2 * time.Second)

Conversation

Siddhesh002 commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikebrow commented Mar 13, 2026

Uh oh!

haytok left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haytok commented Mar 14, 2026

Uh oh!

Siddhesh002 commented Mar 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants