Merge pull request #5228 from twz123/testifylint

Enable testifylint linter

.golangci.yml

@@ -18,6 +18,7 @@ linters:
     - goheader    # Checks is file headers matche a given pattern
     - intrange    # checking for loops that could use an integer range
     - revive      # Stricter drop-in replacement for golint
+    - testifylint # Checks usage of github.com/stretchr/testify
 
 linters-settings:
   depguard:
@@ -72,6 +73,12 @@ linters-settings:
       - name: redefines-builtin-id
         disabled: true
 
+  testifylint:
+    enable-all: true
+    disable:
+      - require-error # flags too many legitimate use cases
+      - suite-thelper # flags too many legitimate use cases
+
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0

internal/http/download_test.go

@@ -209,7 +209,7 @@ func startFakeDownloadServer(t *testing.T, usetls bool, handler http.Handler) st
 		assert.NoError(t, err)
 
 		cert, err := tls.X509KeyPair(certData, keyData)
-		require.NoError(t, err)
+		assert.NoError(t, err)
 
 		server.TLSConfig = &tls.Config{Certificates: []tls.Certificate{cert}}
 		serverError <- server.ServeTLS(listener, "", "")

internal/oci/download_test.go

@@ -128,7 +128,7 @@ func startOCIMockServer(t *testing.T, tname string, test testFile) string {
 	server := starter(
 		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			t.Log(r.Proto, r.Method, r.RequestURI)
-			if !assert.Equal(t, r.Method, http.MethodGet) {
+			if !assert.Equal(t, http.MethodGet, r.Method) {
 				w.WriteHeader(http.StatusMethodNotAllowed)
 				return
 			}

internal/pkg/dir/dir_test.go

@@ -36,7 +36,7 @@ func TestGetAll(t *testing.T) {
 	t.Run("filter dirs", func(t *testing.T) {
 		tmpDir := t.TempDir()
 		require.NoError(t, dir.Init(filepath.Join(tmpDir, "dir1"), 0750))
-		require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "file1"), []byte{}, 0600), "Unable to create file %s:", "file1")
+		require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "file1"), []byte{}, 0600))
 		dirs, err := dir.GetAll(tmpDir)
 		require.NoError(t, err)
 		require.Equal(t, []string{"dir1"}, dirs)

internal/pkg/file/atomic_test.go

@@ -74,7 +74,7 @@ func TestWriteAtomically(t *testing.T) {
 
 	assertPathError := func(t *testing.T, err error, op, dir string) (pathErr *os.PathError, ok bool) {
 		t.Helper()
-		if ok = assert.True(t, errors.As(err, &pathErr), "Not a PathError: %v", err); ok {
+		if assert.ErrorAsf(t, err, &pathErr, "Not a PathError: %v", err) {
 			assert.Equal(t, op, pathErr.Op)
 			assert.Equal(
 				t, dir, filepath.Dir(pathErr.Path),
@@ -135,7 +135,7 @@ func TestWriteAtomically(t *testing.T) {
 
 		errs := flatten(WriteAtomically(file, 0644, func(file io.Writer) error {
 			a, ok := file.(*Atomic)
-			require.True(t, ok, "Not an Atomic: %T", file)
+			require.Truef(t, ok, "Not an Atomic: %T", file)
 			require.NoError(t, a.fd.Close())
 			return nil
 		}))
@@ -146,15 +146,15 @@ func TestWriteAtomically(t *testing.T) {
 		// The first error should be about the failed attempt to sync the temporary file.
 		if err, ok := assertPathError(t, errs[0], "sync", dir); ok {
 			tempPath = err.Path
-			assert.True(t, errors.Is(err, fs.ErrClosed), "Expected fs.ErrClosed: %v", err.Err)
+			assert.ErrorIsf(t, err, fs.ErrClosed, "Expected fs.ErrClosed: %v", err.Err)
 		}
 
 		// The second error should be about the failed attempt to close the temporary file.
 		if err, ok := assertPathError(t, errs[1], "close", dir); ok {
 			if tempPath != "" {
 				assert.Equal(t, tempPath, err.Path, "Temp paths differ between errors")
 			}
-			assert.True(t, errors.Is(err, fs.ErrClosed), "Expected fs.ErrClosed: %v", err.Err)
+			assert.ErrorIsf(t, err, fs.ErrClosed, "Expected fs.ErrClosed: %v", err.Err)
 		}
 
 		assertDirEmpty(t, dir)
@@ -170,7 +170,7 @@ func TestWriteAtomically(t *testing.T) {
 		var tempPath string
 		err := WriteAtomically(file, 0644, func(file io.Writer) error {
 			a, ok := file.(*Atomic)
-			require.True(t, ok, "Not an Atomic: %T", file)
+			require.Truef(t, ok, "Not an Atomic: %T", file)
 			tempPath = a.fd.Name()
 			require.Equal(t, dir, filepath.Dir(tempPath))
 			if runtime.GOOS == "windows" {
@@ -185,7 +185,7 @@ func TestWriteAtomically(t *testing.T) {
 		// The error should be about the failed chmod.
 		if err, ok := assertPathError(t, err, "chmod", dir); ok {
 			assert.Equal(t, tempPath, err.Path, "Error refers to unexpected path")
-			assert.True(t, errors.Is(err, fs.ErrNotExist), "Expected fs.ErrNotExist: %v", err.Err)
+			assert.ErrorIsf(t, err, fs.ErrNotExist, "Expected fs.ErrNotExist: %v", err.Err)
 		}
 
 		assertDirEmpty(t, dir)
@@ -206,7 +206,7 @@ func TestWriteAtomically(t *testing.T) {
 		require.Len(t, errs, 1)
 
 		var linkErr *os.LinkError
-		if assert.True(t, errors.As(errs[0], &linkErr), "Not a LinkError: %#+v", errs[0]) {
+		if assert.ErrorAsf(t, errs[0], &linkErr, "Not a LinkError: %#+v", errs[0]) {
 			assert.Equal(t, "rename", linkErr.Op)
 			assert.Equal(
 				t, dir, filepath.Dir(linkErr.Old),
@@ -220,9 +220,9 @@ func TestWriteAtomically(t *testing.T) {
 				var errno syscall.Errno
 				ok := errors.As(linkErr.Err, &errno)
 				ok = ok && errno == ERROR_ACCESS_DENIED
-				assert.True(t, ok, "Expected ERROR_ACCESS_DENIED: %v", linkErr.Err)
+				assert.Truef(t, ok, "Expected ERROR_ACCESS_DENIED: %v", linkErr.Err)
 			} else {
-				assert.True(t, errors.Is(linkErr.Err, fs.ErrExist), "Expected fs.ErrExist: %v", linkErr.Err)
+				assert.ErrorIsf(t, linkErr.Err, fs.ErrExist, "Expected fs.ErrExist: %v", linkErr.Err)
 			}
 		}
 
@@ -231,7 +231,7 @@ func TestWriteAtomically(t *testing.T) {
 			e := entries[0]
 			name := e.Name()
 			assert.Equal(t, filepath.Base(file), name)
-			assert.True(t, e.IsDir(), "Not a directory: %s", name)
+			assert.Truef(t, e.IsDir(), "Not a directory: %s", name)
 		}
 	})
 
@@ -246,7 +246,7 @@ func TestWriteAtomically(t *testing.T) {
 		var tempPath string
 		errs := flatten(WriteAtomically(file, 0755, func(file io.Writer) error {
 			a, ok := file.(*Atomic)
-			require.True(t, ok, "Not an Atomic: %T", file)
+			require.Truef(t, ok, "Not an Atomic: %T", file)
 			tempPath = a.fd.Name()
 			require.Equal(t, dir, filepath.Dir(tempPath))
 
@@ -274,23 +274,23 @@ func TestWriteAtomically(t *testing.T) {
 
 		// The first error should be about the failed rename.
 		var linkErr *os.LinkError
-		if assert.True(t, errors.As(errs[0], &linkErr), "Not a LinkError: %v", linkErr) {
+		if assert.ErrorAsf(t, errs[0], &linkErr, "Not a LinkError: %v", linkErr) {
 			assert.Equal(t, "rename", linkErr.Op)
 			assert.Equal(t, tempPath, linkErr.Old, "Error refers to unexpected path")
 			assert.Equal(t, file, linkErr.New)
-			assert.True(t, errors.Is(linkErr.Err, fs.ErrExist), "Expected fs.ErrExist: %v", linkErr.Err)
+			assert.ErrorIsf(t, linkErr.Err, fs.ErrExist, "Expected fs.ErrExist: %v", linkErr.Err)
 		}
 
 		// The second error should be about the failed removal.
 		if err, ok := assertPathError(t, errs[1], "remove", dir); ok {
 			assert.Equal(t, tempPath, err.Path, "Error refers to unexpected path")
-			assert.True(t, errors.Is(err, fs.ErrExist), "Expected fs.ErrExist: %v", err.Err)
+			assert.ErrorIsf(t, err, fs.ErrExist, "Expected fs.ErrExist: %v", err.Err)
 		}
 
 		// Expect to see two directories left behind.
 		if entries, err := os.ReadDir(dir); assert.NoError(t, err) && assert.Len(t, entries, 2) {
 			for _, e := range entries {
-				assert.True(t, e.IsDir(), "Not a directory: %s", e.Name())
+				assert.Truef(t, e.IsDir(), "Not a directory: %s", e.Name())
 			}
 		}
 	})

internal/pkg/flags/flags_test.go

@@ -19,6 +19,8 @@ package flags
 import (
 	"testing"
 
+	"github.com/k0sproject/k0s/internal/pkg/stringmap"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -27,17 +29,17 @@ func TestFlagSplitting(t *testing.T) {
 
 	m := Split(args)
 
-	assert.Equal(t, 3, len(m))
-	assert.Equal(t, "bar", m["--foo"])
-	assert.Equal(t, "xyz,asd", m["--foobar"])
-	assert.Equal(t, "", m["--bool-flag"])
+	assert.Equal(t, stringmap.StringMap{
+		"--foo":       "bar",
+		"--foobar":    "xyz,asd",
+		"--bool-flag": "",
+	}, m)
 }
 
 func TestFlagSplittingBoolFlags(t *testing.T) {
 	args := "--bool-flag"
 
 	m := Split(args)
 
-	assert.Equal(t, 1, len(m))
-	assert.Equal(t, "", m["--bool-flag"])
+	assert.Equal(t, stringmap.StringMap{"--bool-flag": ""}, m)
 }

internal/pkg/sysinfo/probes/linux/kernel_test.go

@@ -116,7 +116,7 @@ func TestKConfigProber(t *testing.T) {
 		probeKConfig := newKConfigProber(newUnameProber())
 
 		option, err := probeKConfig(ensureKConfig("I_CERTAINLY_DONT_EXIST"))
-		assert.Equal(t, option, kConfigUnknown)
+		assert.Equal(t, kConfigUnknown, option)
 		var notFoundErr *noKConfigsFound
 		if errors.As(err, &notFoundErr) {
 			t.Logf("System doesn't seem to expose its kernel config: %v", err)

internal/pkg/sysinfo/probes/memory_linux_test.go

@@ -51,8 +51,8 @@ func TestTotalMemoryProber(t *testing.T) {
 		line := lines.Text()
 		if matches := re.FindStringSubmatch(line); matches != nil {
 			kibiBytes, err := strconv.ParseUint(matches[1], 10, 64)
-			require.NoError(t, err, "expected an unsigned integer: %s", line)
-			require.Equal(t, kibiBytes*1024, ram, "/proc/meminfo differs: %s", line)
+			require.NoErrorf(t, err, "expected an unsigned integer: %s", line)
+			require.Equalf(t, kibiBytes*1024, ram, "/proc/meminfo differs: %s", line)
 			return
 		}
 	}

inttest/addons/addons_test.go

@@ -114,7 +114,7 @@ func (as *AddonsSuite) renameChart(ctx context.Context) {
 	i := slices.IndexFunc(cfg.Spec.Extensions.Helm.Charts, func(c k0sv1beta1.Chart) bool {
 		return c.Name == "tgz-addon"
 	})
-	as.Require().GreaterOrEqual(i, 0, "Didn't find tgz-addon in %v", cfg.Spec.Extensions.Helm.Charts)
+	as.Require().GreaterOrEqualf(i, 0, "Didn't find tgz-addon in %v", cfg.Spec.Extensions.Helm.Charts)
 	cfg.Spec.Extensions.Helm.Charts[i].Name = "tgz-renamed-addon"
 
 	cfg, err = configs.Update(ctx, cfg, metav1.UpdateOptions{FieldManager: as.T().Name()})

inttest/airgap/airgap_test.go

@@ -94,7 +94,7 @@ func (s *AirgapSuite) TestK0sGetsUp() {
 	for _, i := range airgap.GetImageURIs(v1beta1.DefaultClusterSpec(), true) {
 		output, err := ssh.ExecWithOutput(ctx, fmt.Sprintf(`k0s ctr i ls "name==%s"`, i))
 		s.Require().NoError(err)
-		s.Require().Contains(output, "io.cri-containerd.pinned=pinned", "expected %s image to have io.cri-containerd.pinned=pinned label", i)
+		s.Require().Containsf(output, "io.cri-containerd.pinned=pinned", "expected %s image to have io.cri-containerd.pinned=pinned label", i)
 	}
 }
 

inttest/ap-airgap/airgap_test.go

@@ -82,7 +82,7 @@ func (s *airgapSuite) SetupTest() {
 			continue // The pause image is pinned by containerd itself
 		}
 		output, err := ssh.ExecWithOutput(ctx, fmt.Sprintf(`k0s ctr i ls "name==%s"`, i))
-		if s.NoError(err, "Failed to check %s", i) {
+		if s.NoErrorf(err, "Failed to check %s", i) {
 			s.NotContains(output, "io.cri-containerd.pinned=pinned", "%s is already pinned", i)
 		}
 	}
@@ -197,7 +197,7 @@ spec:
 	defer ssh.Disconnect()
 	for _, i := range airgap.GetImageURIs(v1beta1.DefaultClusterSpec(), true) {
 		output, err := ssh.ExecWithOutput(ctx, fmt.Sprintf(`k0s ctr i ls "name==%s"`, i))
-		if s.NoError(err, "Failed to check %s", i) {
+		if s.NoErrorf(err, "Failed to check %s", i) {
 			s.Contains(output, "io.cri-containerd.pinned=pinned", "%s is not pinned", i)
 		}
 	}

inttest/ap-controllerworker/controllerworker_test.go

@@ -156,16 +156,17 @@ spec:
 	plan, err := aptest.WaitForPlanState(ctx, client, apconst.AutopilotName, appc.PlanCompleted)
 	s.Require().NoError(err)
 
-	s.Equal(1, len(plan.Status.Commands))
-	cmd := plan.Status.Commands[0]
+	if s.Len(plan.Status.Commands, 1) {
+		cmd := plan.Status.Commands[0]
 
-	s.Equal(appc.PlanCompleted, cmd.State)
-	s.NotNil(cmd.K0sUpdate)
-	s.NotNil(cmd.K0sUpdate.Controllers)
-	s.Empty(cmd.K0sUpdate.Workers)
+		s.Equal(appc.PlanCompleted, cmd.State)
+		s.NotNil(cmd.K0sUpdate)
+		s.NotNil(cmd.K0sUpdate.Controllers)
+		s.Empty(cmd.K0sUpdate.Workers)
 
-	for _, node := range cmd.K0sUpdate.Controllers {
-		s.Equal(appc.SignalCompleted, node.State)
+		for _, node := range cmd.K0sUpdate.Controllers {
+			s.Equal(appc.SignalCompleted, node.State)
+		}
 	}
 
 	kc, err := s.KubeClient(s.ControllerNode(0))

inttest/ap-platformselect/platformselect_test.go

@@ -96,13 +96,14 @@ spec:
 	plan, err := aptest.WaitForPlanState(ctx, client, apconst.AutopilotName, appc.PlanCompleted)
 	s.Require().NoError(err)
 
-	s.Equal(1, len(plan.Status.Commands))
-	cmd := plan.Status.Commands[0]
-
-	s.NotNil(cmd.K0sUpdate)
-	s.NotNil(cmd.K0sUpdate.Controllers)
-	s.NotEmpty(cmd.K0sUpdate.Controllers)
-	s.Equal(appc.SignalCompleted, cmd.K0sUpdate.Controllers[0].State)
+	if s.Len(plan.Status.Commands, 1) {
+		cmd := plan.Status.Commands[0]
+
+		s.NotNil(cmd.K0sUpdate)
+		s.NotNil(cmd.K0sUpdate.Controllers)
+		s.NotEmpty(cmd.K0sUpdate.Controllers)
+		s.Equal(appc.SignalCompleted, cmd.K0sUpdate.Controllers[0].State)
+	}
 }
 
 // TestPlatformSelectSuite sets up a suite using a single controller, running various

inttest/ap-quorum/quorum_test.go

@@ -118,16 +118,17 @@ spec:
 	plan, err := aptest.WaitForPlanState(ctx, client, apconst.AutopilotName, appc.PlanCompleted)
 	s.Require().NoError(err)
 
-	s.Equal(1, len(plan.Status.Commands))
-	cmd := plan.Status.Commands[0]
+	if s.Len(plan.Status.Commands, 1) {
+		cmd := plan.Status.Commands[0]
 
-	s.Equal(appc.PlanCompleted, cmd.State)
-	s.NotNil(cmd.K0sUpdate)
-	s.NotNil(cmd.K0sUpdate.Controllers)
-	s.Empty(cmd.K0sUpdate.Workers)
+		s.Equal(appc.PlanCompleted, cmd.State)
+		s.NotNil(cmd.K0sUpdate)
+		s.NotNil(cmd.K0sUpdate.Controllers)
+		s.Empty(cmd.K0sUpdate.Workers)
 
-	for _, node := range cmd.K0sUpdate.Controllers {
-		s.Equal(appc.SignalCompleted, node.State)
+		for _, node := range cmd.K0sUpdate.Controllers {
+			s.Equal(appc.SignalCompleted, node.State)
+		}
 	}
 }
 

inttest/ap-selector/selector_test.go

@@ -143,17 +143,18 @@ spec:
 	plan, err := aptest.WaitForPlanState(ctx, client, apconst.AutopilotName, appc.PlanCompleted)
 	s.Require().NoError(err)
 
-	s.Equal(1, len(plan.Status.Commands))
-	cmd := plan.Status.Commands[0]
-
-	s.Equal(appc.PlanCompleted, cmd.State)
-	s.NotNil(cmd.K0sUpdate)
-	s.NotNil(cmd.K0sUpdate.Controllers)
-	s.NotNil(cmd.K0sUpdate.Workers)
-
-	for _, group := range [][]apv1beta2.PlanCommandTargetStatus{cmd.K0sUpdate.Controllers, cmd.K0sUpdate.Workers} {
-		for _, node := range group {
-			s.Equal(appc.SignalCompleted, node.State)
+	if s.Len(plan.Status.Commands, 1) {
+		cmd := plan.Status.Commands[0]
+
+		s.Equal(appc.PlanCompleted, cmd.State)
+		s.NotNil(cmd.K0sUpdate)
+		s.NotNil(cmd.K0sUpdate.Controllers)
+		s.NotNil(cmd.K0sUpdate.Workers)
+
+		for _, group := range [][]apv1beta2.PlanCommandTargetStatus{cmd.K0sUpdate.Controllers, cmd.K0sUpdate.Workers} {
+			for _, node := range group {
+				s.Equal(appc.SignalCompleted, node.State)
+			}
 		}
 	}
 }

inttest/ap-single/single_test.go

@@ -99,14 +99,15 @@ spec:
 	plan, err := aptest.WaitForPlanState(ctx, client, apconst.AutopilotName, appc.PlanCompleted)
 	s.Require().NoError(err, "While waiting for plan to complete")
 
-	s.Equal(1, len(plan.Status.Commands))
-	cmd := plan.Status.Commands[0]
-
-	s.Equal(appc.PlanCompleted, cmd.State)
-	s.NotNil(cmd.K0sUpdate)
-	s.NotNil(cmd.K0sUpdate.Controllers)
-	s.Empty(cmd.K0sUpdate.Workers)
-	s.Equal(appc.SignalCompleted, cmd.K0sUpdate.Controllers[0].State)
+	if s.Len(plan.Status.Commands, 1) {
+		cmd := plan.Status.Commands[0]
+
+		s.Equal(appc.PlanCompleted, cmd.State)
+		s.NotNil(cmd.K0sUpdate)
+		s.NotNil(cmd.K0sUpdate.Controllers)
+		s.Empty(cmd.K0sUpdate.Workers)
+		s.Equal(appc.SignalCompleted, cmd.K0sUpdate.Controllers[0].State)
+	}
 }
 
 // TestPlansSingleControllerSuite sets up a suite using a single controller, running various