golangci-lint: enable thelper linter

Signed-off-by: Sebastiaan van Stijn <[email protected]>

.golangci.yml

@@ -16,6 +16,7 @@ linters:
     - nakedret
     - revive
     - staticcheck
+    - thelper
     - typecheck
     - unconvert
     - unparam

cli/command/cli_options_test.go

@@ -8,6 +8,7 @@ import (
 )
 
 func contentTrustEnabled(t *testing.T) bool {
+	t.Helper()
 	var cli DockerCli
 	assert.NilError(t, WithContentTrustFromEnv()(&cli))
 	return cli.contentTrust

cli/command/context/create_test.go

@@ -104,6 +104,7 @@ func TestCreate(t *testing.T) {
 }
 
 func assertContextCreateLogging(t *testing.T, cli *test.FakeCli, n string) {
+	t.Helper()
 	assert.Equal(t, n+"\n", cli.OutBuffer().String())
 	assert.Equal(t, fmt.Sprintf("Successfully created context %q\n", n), cli.ErrBuffer().String())
 }

cli/command/formatter/tabwriter/tabwriter_test.go

@@ -36,6 +36,7 @@ func (b *buffer) Write(buf []byte) (written int, err error) {
 func (b *buffer) String() string { return string(b.a) }
 
 func write(t *testing.T, testname string, w *Writer, src string) {
+	t.Helper()
 	written, err := io.WriteString(w, src)
 	if err != nil {
 		t.Errorf("--- test: %s\n--- src:\n%q\n--- write error: %v\n", testname, src, err)
@@ -46,6 +47,7 @@ func write(t *testing.T, testname string, w *Writer, src string) {
 }
 
 func verify(t *testing.T, testname string, w *Writer, b *buffer, src, expected string) {
+	t.Helper()
 	err := w.Flush()
 	if err != nil {
 		t.Errorf("--- test: %s\n--- src:\n%q\n--- flush error: %v\n", testname, src, err)
@@ -58,6 +60,7 @@ func verify(t *testing.T, testname string, w *Writer, b *buffer, src, expected s
 }
 
 func check(t *testing.T, testname string, minwidth, tabwidth, padding int, padchar byte, flags uint, src, expected string) {
+	t.Helper()
 	var b buffer
 	b.init(1000)
 
@@ -622,6 +625,7 @@ func (panicWriter) Write([]byte) (int, error) {
 }
 
 func wantPanicString(t *testing.T, want string) {
+	t.Helper()
 	if e := recover(); e != nil {
 		got, ok := e.(string)
 		switch {

cli/command/image/build/context_test.go

@@ -23,16 +23,19 @@ func prepareEmpty(_ *testing.T) string {
 }
 
 func prepareNoFiles(t *testing.T) string {
+	t.Helper()
 	return createTestTempDir(t)
 }
 
 func prepareOneFile(t *testing.T) string {
+	t.Helper()
 	contextDir := createTestTempDir(t)
 	createTestTempFile(t, contextDir, DefaultDockerfileName, dockerfileContents)
 	return contextDir
 }
 
 func testValidateContextDirectory(t *testing.T, prepare func(t *testing.T) string, excludes []string) {
+	t.Helper()
 	contextDir := prepare(t)
 	err := ValidateContextDirectory(contextDir, excludes)
 	assert.NilError(t, err)
@@ -250,6 +253,7 @@ func createTestTempFile(t *testing.T, dir, filename, contents string) string {
 // This function is meant to be executed as a deferred call.
 // When an error occurs, it terminates the test.
 func chdir(t *testing.T, dir string) {
+	t.Helper()
 	workingDirectory, err := os.Getwd()
 	assert.NilError(t, err)
 	assert.NilError(t, os.Chdir(dir))

cli/command/manifest/inspect_test.go

@@ -20,12 +20,14 @@ import (
 )
 
 func ref(t *testing.T, name string) reference.Named {
+	t.Helper()
 	named, err := reference.ParseNamed("example.com/" + name)
 	assert.NilError(t, err)
 	return named
 }
 
 func fullImageManifest(t *testing.T, ref reference.Named) types.ImageManifest {
+	t.Helper()
 	man, err := schema2.FromStruct(schema2.Manifest{
 		Versioned: schema2.SchemaVersion,
 		Config: distribution.Descriptor{

cli/command/service/inspect_test.go

@@ -17,6 +17,7 @@ import (
 )
 
 func formatServiceInspect(t *testing.T, format formatter.Format, now time.Time) string {
+	t.Helper()
 	b := new(bytes.Buffer)
 
 	endpointSpec := &swarm.EndpointSpec{

cli/command/trust/key_load_test.go

@@ -114,99 +114,91 @@ var testKeys = map[string][]byte{
 func TestLoadKeyFromPath(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows")
 	for keyID, keyBytes := range testKeys {
-		keyID, keyBytes := keyID, keyBytes
+		privKeyID, privKeyFixture := keyID, keyBytes
 		t.Run(fmt.Sprintf("load-key-id-%s-from-path", keyID), func(t *testing.T) {
-			testLoadKeyFromPath(t, keyID, keyBytes)
+			privKeyFilepath := filepath.Join(t.TempDir(), "privkey.pem")
+			assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, notary.PrivNoExecPerms))
+
+			keyStorageDir := t.TempDir()
+
+			const passwd = "password"
+			cannedPasswordRetriever := passphrase.ConstantRetriever(passwd)
+			keyFileStore, err := storage.NewPrivateKeyFileStorage(keyStorageDir, notary.KeyExtension)
+			assert.NilError(t, err)
+			privKeyImporters := []trustmanager.Importer{keyFileStore}
+
+			// get the privKeyBytes
+			privKeyBytes, err := getPrivKeyBytesFromPath(privKeyFilepath)
+			assert.NilError(t, err)
+
+			// import the key to our keyStorageDir
+			assert.Check(t, loadPrivKeyBytesToStore(privKeyBytes, privKeyImporters, privKeyFilepath, "signer-name", cannedPasswordRetriever))
+
+			// check that the appropriate ~/<trust_dir>/private/<key_id>.key file exists
+			expectedImportKeyPath := filepath.Join(keyStorageDir, notary.PrivDir, privKeyID+"."+notary.KeyExtension)
+			_, err = os.Stat(expectedImportKeyPath)
+			assert.NilError(t, err)
+
+			// verify the key content
+			from, _ := os.OpenFile(expectedImportKeyPath, os.O_RDONLY, notary.PrivExecPerms)
+			defer from.Close()
+			fromBytes, _ := io.ReadAll(from)
+			keyPEM, _ := pem.Decode(fromBytes)
+			assert.Check(t, is.Equal("signer-name", keyPEM.Headers["role"]))
+			// the default GUN is empty
+			assert.Check(t, is.Equal("", keyPEM.Headers["gun"]))
+			// assert encrypted header
+			assert.Check(t, is.Equal("ENCRYPTED PRIVATE KEY", keyPEM.Type))
+
+			decryptedKey, err := tufutils.ParsePKCS8ToTufKey(keyPEM.Bytes, []byte(passwd))
+			assert.NilError(t, err)
+			fixturePEM, _ := pem.Decode(privKeyFixture)
+			assert.Check(t, is.DeepEqual(fixturePEM.Bytes, decryptedKey.Private()))
 		})
 	}
 }
 
-func testLoadKeyFromPath(t *testing.T, privKeyID string, privKeyFixture []byte) {
-	privKeyFilepath := filepath.Join(t.TempDir(), "privkey.pem")
-	assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, notary.PrivNoExecPerms))
-
-	keyStorageDir := t.TempDir()
-
-	const passwd = "password"
-	cannedPasswordRetriever := passphrase.ConstantRetriever(passwd)
-	keyFileStore, err := storage.NewPrivateKeyFileStorage(keyStorageDir, notary.KeyExtension)
-	assert.NilError(t, err)
-	privKeyImporters := []trustmanager.Importer{keyFileStore}
-
-	// get the privKeyBytes
-	privKeyBytes, err := getPrivKeyBytesFromPath(privKeyFilepath)
-	assert.NilError(t, err)
-
-	// import the key to our keyStorageDir
-	assert.Check(t, loadPrivKeyBytesToStore(privKeyBytes, privKeyImporters, privKeyFilepath, "signer-name", cannedPasswordRetriever))
-
-	// check that the appropriate ~/<trust_dir>/private/<key_id>.key file exists
-	expectedImportKeyPath := filepath.Join(keyStorageDir, notary.PrivDir, privKeyID+"."+notary.KeyExtension)
-	_, err = os.Stat(expectedImportKeyPath)
-	assert.NilError(t, err)
-
-	// verify the key content
-	from, _ := os.OpenFile(expectedImportKeyPath, os.O_RDONLY, notary.PrivExecPerms)
-	defer from.Close()
-	fromBytes, _ := io.ReadAll(from)
-	keyPEM, _ := pem.Decode(fromBytes)
-	assert.Check(t, is.Equal("signer-name", keyPEM.Headers["role"]))
-	// the default GUN is empty
-	assert.Check(t, is.Equal("", keyPEM.Headers["gun"]))
-	// assert encrypted header
-	assert.Check(t, is.Equal("ENCRYPTED PRIVATE KEY", keyPEM.Type))
-
-	decryptedKey, err := tufutils.ParsePKCS8ToTufKey(keyPEM.Bytes, []byte(passwd))
-	assert.NilError(t, err)
-	fixturePEM, _ := pem.Decode(privKeyFixture)
-	assert.Check(t, is.DeepEqual(fixturePEM.Bytes, decryptedKey.Private()))
-}
-
 func TestLoadKeyTooPermissive(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows")
 	for keyID, keyBytes := range testKeys {
-		keyID, keyBytes := keyID, keyBytes
+		keyID, privKeyFixture := keyID, keyBytes
 		t.Run(fmt.Sprintf("load-key-id-%s-too-permissive", keyID), func(t *testing.T) {
-			testLoadKeyTooPermissive(t, keyBytes)
-		})
-	}
-}
-
-func testLoadKeyTooPermissive(t *testing.T, privKeyFixture []byte) {
-	privKeyDir := t.TempDir()
-	privKeyFilepath := filepath.Join(privKeyDir, "privkey477.pem")
-	assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o477))
+			privKeyDir := t.TempDir()
+			privKeyFilepath := filepath.Join(privKeyDir, "privkey477.pem")
+			assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o477))
 
-	// import the key to our keyStorageDir
-	_, err := getPrivKeyBytesFromPath(privKeyFilepath)
-	expected := fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath)
-	assert.Error(t, err, expected)
+			// import the key to our keyStorageDir
+			_, err := getPrivKeyBytesFromPath(privKeyFilepath)
+			expected := fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath)
+			assert.Error(t, err, expected)
 
-	privKeyFilepath = filepath.Join(privKeyDir, "privkey667.pem")
-	assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o677))
+			privKeyFilepath = filepath.Join(privKeyDir, "privkey667.pem")
+			assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o677))
 
-	_, err = getPrivKeyBytesFromPath(privKeyFilepath)
-	expected = fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath)
-	assert.Error(t, err, expected)
+			_, err = getPrivKeyBytesFromPath(privKeyFilepath)
+			expected = fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath)
+			assert.Error(t, err, expected)
 
-	privKeyFilepath = filepath.Join(privKeyDir, "privkey777.pem")
-	assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o777))
+			privKeyFilepath = filepath.Join(privKeyDir, "privkey777.pem")
+			assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o777))
 
-	_, err = getPrivKeyBytesFromPath(privKeyFilepath)
-	expected = fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath)
-	assert.Error(t, err, expected)
+			_, err = getPrivKeyBytesFromPath(privKeyFilepath)
+			expected = fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath)
+			assert.Error(t, err, expected)
 
-	privKeyFilepath = filepath.Join(privKeyDir, "privkey400.pem")
-	assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o400))
+			privKeyFilepath = filepath.Join(privKeyDir, "privkey400.pem")
+			assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o400))
 
-	_, err = getPrivKeyBytesFromPath(privKeyFilepath)
-	assert.NilError(t, err)
+			_, err = getPrivKeyBytesFromPath(privKeyFilepath)
+			assert.NilError(t, err)
 
-	privKeyFilepath = filepath.Join(privKeyDir, "privkey600.pem")
-	assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o600))
+			privKeyFilepath = filepath.Join(privKeyDir, "privkey600.pem")
+			assert.NilError(t, os.WriteFile(privKeyFilepath, privKeyFixture, 0o600))
 
-	_, err = getPrivKeyBytesFromPath(privKeyFilepath)
-	assert.NilError(t, err)
+			_, err = getPrivKeyBytesFromPath(privKeyFilepath)
+			assert.NilError(t, err)
+		})
+	}
 }
 
 var pubKeyFixture = []byte(`-----BEGIN PUBLIC KEY-----

cli/config/config_test.go

@@ -15,6 +15,7 @@ import (
 )
 
 func setupConfigDir(t *testing.T) string {
+	t.Helper()
 	tmpdir := t.TempDir()
 	oldDir := Dir()
 	SetDir(tmpdir)

cli/required_test.go

@@ -21,7 +21,11 @@ func TestRequiresNoArgs(t *testing.T) {
 			expectedError: "accepts no arguments.",
 		},
 	}
-	runTestCases(t, testCases)
+	for _, tc := range testCases {
+		cmd := newDummyCommand(tc.validateFunc)
+		cmd.SetArgs(tc.args)
+		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
+	}
 }
 
 func TestRequiresMinArgs(t *testing.T) {
@@ -40,7 +44,11 @@ func TestRequiresMinArgs(t *testing.T) {
 			expectedError: "at least 2 arguments.",
 		},
 	}
-	runTestCases(t, testCases)
+	for _, tc := range testCases {
+		cmd := newDummyCommand(tc.validateFunc)
+		cmd.SetArgs(tc.args)
+		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
+	}
 }
 
 func TestRequiresMaxArgs(t *testing.T) {
@@ -60,7 +68,11 @@ func TestRequiresMaxArgs(t *testing.T) {
 			expectedError: "at most 2 arguments.",
 		},
 	}
-	runTestCases(t, testCases)
+	for _, tc := range testCases {
+		cmd := newDummyCommand(tc.validateFunc)
+		cmd.SetArgs(tc.args)
+		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
+	}
 }
 
 func TestRequiresRangeArgs(t *testing.T) {
@@ -88,7 +100,11 @@ func TestRequiresRangeArgs(t *testing.T) {
 			expectedError: "at least 1 ",
 		},
 	}
-	runTestCases(t, testCases)
+	for _, tc := range testCases {
+		cmd := newDummyCommand(tc.validateFunc)
+		cmd.SetArgs(tc.args)
+		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
+	}
 }
 
 func TestExactArgs(t *testing.T) {
@@ -106,7 +122,11 @@ func TestExactArgs(t *testing.T) {
 			expectedError: "exactly 2 arguments.",
 		},
 	}
-	runTestCases(t, testCases)
+	for _, tc := range testCases {
+		cmd := newDummyCommand(tc.validateFunc)
+		cmd.SetArgs(tc.args)
+		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
+	}
 }
 
 type testCase struct {
@@ -115,17 +135,6 @@ type testCase struct {
 	expectedError string
 }
 
-func runTestCases(t *testing.T, testCases []testCase) {
-	for _, tc := range testCases {
-		cmd := newDummyCommand(tc.validateFunc)
-		cmd.SetArgs(tc.args)
-		cmd.SetOut(io.Discard)
-
-		err := cmd.Execute()
-		assert.ErrorContains(t, err, tc.expectedError)
-	}
-}
-
 func newDummyCommand(validationFunc cobra.PositionalArgs) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:  "dummy",
@@ -134,5 +143,6 @@ func newDummyCommand(validationFunc cobra.PositionalArgs) *cobra.Command {
 			return errors.New("no error")
 		},
 	}
+	cmd.SetOut(io.Discard)
 	return cmd
 }

e2e/cli-plugins/util_test.go

@@ -13,6 +13,7 @@ import (
 )
 
 func prepare(t *testing.T) (func(args ...string) icmd.Cmd, *configfile.ConfigFile, func()) {
+	t.Helper()
 	cfg := fs.NewDir(t, "plugin-test",
 		fs.WithFile("config.json", fmt.Sprintf(`{"cliPluginsExtraDirs": [%q]}`, os.Getenv("DOCKER_CLI_E2E_PLUGINS_EXTRA_DIRS"))),
 	)