Merge pull request #3910 from apostasie/sub-project-testing-3

CI: test framework, part III

Author: AkihiroSuda

go.mod

@@ -81,6 +81,7 @@ require (
 	github.com/containerd/plugin v1.0.0 // indirect
 	github.com/containerd/ttrpc v1.2.7 // indirect
 	github.com/containers/ocicrypt v1.2.1 // indirect
+	github.com/creack/pty v1.1.24 // indirect
 	github.com/djherbis/times v1.6.0 // indirect
 	github.com/docker/docker-credential-helpers v0.8.2 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect

go.sum

@@ -76,8 +76,8 @@ github.com/coreos/go-iptables v0.8.0/go.mod h1:Qe8Bv2Xik5FyTXwgIbLAnv2sWSBmvWdFE
 github.com/coreos/go-systemd/v22 v22.5.0 h1:RrqgGjYQKalulkV8NGVIfkXQf6YYmOyiJKk8iXXhfZs=
 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc=
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
-github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
-github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
+github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
+github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
 github.com/cyphar/filepath-securejoin v0.4.1 h1:JyxxyPEaktOD+GAnqIqTf9A8tHyAG22rowi7HkoSU1s=
 github.com/cyphar/filepath-securejoin v0.4.1/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

mod/tigron/.golangci.yml

@@ -0,0 +1,54 @@
+---
+output:
+  sort-results: true
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+
+run:
+  concurrency: 0
+  timeout: 5m
+  issues-exit-code: 2
+  tests: true
+  modules-download-mode: readonly
+  allow-parallel-runners: true
+  allow-serial-runners: true
+
+linters:
+  disable-all: false
+  enable-all: true
+  disable:
+    # Opting-out
+    - nonamedreturns  # named returns are occasionally useful
+    - exhaustruct  # does not serve much of a purpose
+    - funlen  # not interested
+    - cyclop  # not interested much
+    - godox  # having these are useful
+    # Duplicating
+    - gci  # we use go-imports instead
+    # Deprecated
+    - tenv
+    # TODO: Temporarily out until we wrap up all of them
+#    - wrapcheck
+
+linters-settings:
+  staticcheck:
+    checks:
+      - "all"
+
+  depguard:
+    rules:
+      main:
+        files:
+          - "$all"
+        allow:
+          - $gostd
+          - "github.com/containerd/nerdctl/mod/tigron"
+          # WATCHOUT! https://github.com/OpenPeeDeeP/depguard/issues/108
+          # Currently, depguard will fail detecting any dependency starting with a standard package name as third-party.
+          # Thus, the following three are allowed provisionally, though currently not "necessary".
+          - "golang.org/x/sync"
+          - "golang.org/x/term"
+          - "gotest.tools"
+          - "github.com/creack/pty"

mod/tigron/.yamllint

@@ -0,0 +1,13 @@
+---
+
+extends: default
+
+rules:
+  indentation:
+    spaces: 2
+    indent-sequences: consistent
+  truthy:
+    allowed-values: ['true', 'false', 'on', 'off']
+  comments-indentation: disable
+  document-start: disable
+  line-length: disable

mod/tigron/Makefile

@@ -75,14 +75,15 @@ lint-go-all:
 	$(call title, $@)
 	@cd $(MAKEFILE_DIR) \
 		&& GOOS=darwin make lint-go \
+		&& GOOS=freebsd make lint-go \
 		&& GOOS=linux make lint-go \
 		&& GOOS=windows make lint-go
 	$(call footer, $@)
 
 lint-imports:
 	$(call title, $@)
 	@cd $(MAKEFILE_DIR) \
-		&& goimports-reviser -recursive -list-diff -set-exit-status -output stdout -company-prefixes "github.com/containerd"  ./...
+		&& goimports-reviser -recursive -list-diff -set-exit-status -output stdout -company-prefixes "$(ORG_PREFIXES)"  ./...
 	$(call footer, $@)
 
 lint-yaml:
@@ -124,6 +125,7 @@ lint-licenses-all:
 	$(call title, $@)
 	@cd $(MAKEFILE_DIR) \
 		&& GOOS=darwin make lint-licenses \
+		&& GOOS=freebsd make lint-licenses \
 		&& GOOS=linux make lint-licenses \
 		&& GOOS=windows make lint-licenses
 	$(call footer, $@)
@@ -141,6 +143,7 @@ fix-go-all:
 	$(call title, $@)
 	@cd $(MAKEFILE_DIR) \
 		&& GOOS=darwin make fix-go \
+		&& GOOS=freebsd make fix-go \
 		&& GOOS=linux make fix-go \
 		&& GOOS=windows make fix-go
 	$(call footer, $@)

mod/tigron/go.mod

@@ -3,6 +3,7 @@ module github.com/containerd/nerdctl/mod/tigron
 go 1.23
 
 require (
+	github.com/creack/pty v1.1.24
 	golang.org/x/sync v0.11.0
 	golang.org/x/term v0.29.0
 	gotest.tools/v3 v3.5.2

mod/tigron/go.sum

@@ -1,3 +1,5 @@
+github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
+github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 golang.org/x/sync v0.11.0 h1:GGz8+XQP4FvTTrjZPzNKTMFtSXH80RAzG+5ghFPgK9w=

mod/tigron/require/requirement.go

@@ -26,7 +26,7 @@ import (
 
 func Binary(name string) *test.Requirement {
 	return &test.Requirement{
-		Check: func(data test.Data, helpers test.Helpers) (bool, string) {
+		Check: func(_ test.Data, _ test.Helpers) (bool, string) {
 			mess := fmt.Sprintf("executable %q has been found in PATH", name)
 			ret := true
 			if _, err := exec.LookPath(name); err != nil {
@@ -41,7 +41,7 @@ func Binary(name string) *test.Requirement {
 
 func OS(os string) *test.Requirement {
 	return &test.Requirement{
-		Check: func(data test.Data, helpers test.Helpers) (bool, string) {
+		Check: func(_ test.Data, _ test.Helpers) (bool, string) {
 			mess := fmt.Sprintf("current operating system is %q", runtime.GOOS)
 			ret := true
 			if runtime.GOOS != os {
@@ -55,7 +55,7 @@ func OS(os string) *test.Requirement {
 
 func Arch(arch string) *test.Requirement {
 	return &test.Requirement{
-		Check: func(data test.Data, helpers test.Helpers) (bool, string) {
+		Check: func(_ test.Data, _ test.Helpers) (bool, string) {
 			mess := fmt.Sprintf("current architecture is %q", runtime.GOARCH)
 			ret := true
 			if runtime.GOARCH != arch {
@@ -67,18 +67,22 @@ func Arch(arch string) *test.Requirement {
 	}
 }
 
-var Amd64 = Arch("amd64")
-var Arm64 = Arch("arm64")
-var Windows = OS("windows")
-var Linux = OS("linux")
-var Darwin = OS("darwin")
+//nolint:gochecknoglobals
+var (
+	Amd64   = Arch("amd64")
+	Arm64   = Arch("arm64")
+	Windows = OS("windows")
+	Linux   = OS("linux")
+	Darwin  = OS("darwin")
+)
 
-// NOTE: Not will always lose setups and cleanups...
+// NOTE: Not will always ignore any setup and cleanup inside the wrapped requirement.
 
 func Not(requirement *test.Requirement) *test.Requirement {
 	return &test.Requirement{
 		Check: func(data test.Data, helpers test.Helpers) (bool, string) {
 			ret, mess := requirement.Check(data, helpers)
+
 			return !ret, mess
 		},
 	}
@@ -93,10 +97,12 @@ func All(requirements ...*test.Requirement) *test.Requirement {
 			for _, requirement := range requirements {
 				ret, subMess = requirement.Check(data, helpers)
 				mess += "\n" + subMess
+
 				if !ret {
 					return ret, mess
 				}
 			}
+
 			return ret, mess
 		},
 		Setup: func(data test.Data, helpers test.Helpers) {

mod/tigron/test/case.go

@@ -59,10 +59,13 @@ type Case struct {
 	parent  *Case
 }
 
-// Run prepares and executes the test, and any possible subtests
+// Run prepares and executes the test, and any possible subtests.
+//
+//nolint:gocognit
 func (test *Case) Run(t *testing.T) {
 	t.Helper()
 	// Run the test
+	//nolint:thelper
 	testRun := func(subT *testing.T) {
 		subT.Helper()
 
@@ -81,10 +84,13 @@ func (test *Case) Run(t *testing.T) {
 
 		// If we have a parent, get parent env, data and config
 		var parentData Data
+
 		var parentConfig Config
+
 		if test.parent != nil {
 			parentData = test.parent.Data
 			parentConfig = test.parent.Config
+
 			for k, v := range test.parent.Env {
 				if _, ok := test.Env[k]; !ok {
 					test.Env[k] = v
@@ -96,23 +102,22 @@ func (test *Case) Run(t *testing.T) {
 		test.Data = configureData(test.t, test.Data, parentData)
 		test.Config = configureConfig(test.Config, parentConfig)
 
-		var b CustomizableCommand
+		var custCom CustomizableCommand
 		if registeredTestable == nil {
-			b = &GenericCommand{}
+			custCom = &GenericCommand{}
 		} else {
-			b = registeredTestable.CustomCommand(test, test.t)
+			custCom = registeredTestable.CustomCommand(test, test.t)
 		}
 
-		b.WithCwd(test.Data.TempDir())
-
-		b.withT(test.t)
-		b.withTempDir(test.Data.TempDir())
-		b.withEnv(test.Env)
-		b.withConfig(test.Config)
+		custCom.WithCwd(test.Data.TempDir())
+		custCom.withT(test.t)
+		custCom.withTempDir(test.Data.TempDir())
+		custCom.withEnv(test.Env)
+		custCom.withConfig(test.Config)
 
 		// Attach the base command, and t
 		test.helpers = &helpersInternal{
-			cmdInternal: b,
+			cmdInternal: custCom,
 			t:           test.t,
 		}
 
@@ -125,9 +130,11 @@ func (test *Case) Run(t *testing.T) {
 			if !shouldRun {
 				test.t.Skipf("test skipped as: %s", message)
 			}
+
 			if test.Require.Setup != nil {
 				setups = append(setups, test.Require.Setup)
 			}
+
 			if test.Require.Cleanup != nil {
 				cleanups = append(cleanups, test.Require.Cleanup)
 			}
@@ -154,35 +161,46 @@ func (test *Case) Run(t *testing.T) {
 		}
 
 		// Execute cleanups now
+		test.t.Log("")
 		test.t.Log("======================== Pre-test cleanup ========================")
+
 		for _, cleanup := range cleanups {
 			cleanup(test.Data, test.helpers)
 		}
 
 		// Register the cleanups, in reverse
 		test.t.Cleanup(func() {
+			test.t.Log("")
 			test.t.Log("======================== Post-test cleanup ========================")
+
 			slices.Reverse(cleanups)
+
 			for _, cleanup := range cleanups {
 				cleanup(test.Data, test.helpers)
 			}
 		})
 
 		// Run the setups
+		test.t.Log("")
 		test.t.Log("======================== Test setup ========================")
+
 		for _, setup := range setups {
 			setup(test.Data, test.helpers)
 		}
 
 		// Run the command if any, with expectations
 		// Note: if we have a command, we already know we DO have Expected
+		test.t.Log("")
 		test.t.Log("======================== Test Run ========================")
+
 		if test.Command != nil {
 			test.Command(test.Data, test.helpers).Run(test.Expected(test.Data, test.helpers))
 		}
 
 		// Now go for the subtests
+		test.t.Log("")
 		test.t.Log("======================== Processing subtests ========================")
+
 		for _, subTest := range test.SubTests {
 			subTest.parent = test
 			subTest.Run(test.t)