golangci-lint: enable thelper linter

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

.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,14 +114,8 @@ 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)
-		})
-	}
-}
-
-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))
 
@@ -160,19 +154,15 @@ func testLoadKeyFromPath(t *testing.T, privKeyID string, privKeyFixture []byte)
 			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))
@@ -207,6 +197,8 @@ func testLoadKeyTooPermissive(t *testing.T, privKeyFixture []byte) {
 
 			_, 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"))),
 	)

e2e/container/attach_test.go

@@ -10,20 +10,14 @@ import (
 )
 
 func TestAttachExitCode(t *testing.T) {
-	containerID := runBackgroundContainsWithExitCode(t, 21)
+	const exitCode = 21
-
+	result := icmd.RunCommand("docker", "run", "-d", "-i", "--rm", fixtures.AlpineImage, "sh", "-c", fmt.Sprintf("read; exit %d", exitCode))
-	result := icmd.RunCmd(
-		icmd.Command("docker", "attach", containerID),
-		withStdinNewline)
-
-	result.Assert(t, icmd.Expected{ExitCode: 21})
-}
-
-func runBackgroundContainsWithExitCode(t *testing.T, exitcode int) string {
-	result := icmd.RunCommand("docker", "run", "-d", "-i", "--rm", fixtures.AlpineImage,
-		"sh", "-c", fmt.Sprintf("read; exit %d", exitcode))
 	result.Assert(t, icmd.Success)
-	return strings.TrimSpace(result.Stdout())
+
+	containerID := strings.TrimSpace(result.Stdout())
+
+	result = icmd.RunCmd(icmd.Command("docker", "attach", containerID), withStdinNewline)
+	result.Assert(t, icmd.Expected{ExitCode: exitCode})
 }
 
 func withStdinNewline(cmd *icmd.Cmd) {

e2e/container/kill_test.go

@@ -11,33 +11,27 @@ import (
 )
 
 func TestKillContainer(t *testing.T) {
-	containerID := runBackgroundTop(t)
+	result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage, "top")
+	result.Assert(t, icmd.Success)
+
+	containerID := strings.TrimSpace(result.Stdout())
 
 	// Kill with SIGTERM should kill the process
-	result := icmd.RunCmd(
+	result = icmd.RunCmd(icmd.Command("docker", "kill", "-s", "SIGTERM", containerID))
-		icmd.Command("docker", "kill", "-s", "SIGTERM", containerID),
-	)
 
 	result.Assert(t, icmd.Success)
 	poll.WaitOn(t, containerStatus(t, containerID, "exited"), poll.WithDelay(100*time.Millisecond), poll.WithTimeout(5*time.Second))
 
 	// Kill on a stop container should return an error
-	result = icmd.RunCmd(
+	result = icmd.RunCmd(icmd.Command("docker", "kill", containerID))
-		icmd.Command("docker", "kill", containerID),
-	)
 	result.Assert(t, icmd.Expected{
 		ExitCode: 1,
 		Err:      "is not running",
 	})
 }
 
-func runBackgroundTop(t *testing.T) string {
-	result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage, "top")
-	result.Assert(t, icmd.Success)
-	return strings.TrimSpace(result.Stdout())
-}
-
 func containerStatus(t *testing.T, containerID, status string) func(poll.LogT) poll.Result {
+	t.Helper()
 	return func(poll.LogT) poll.Result {
 		result := icmd.RunCommand("docker", "inspect", "-f", "{{ .State.Status }}", containerID)
 		result.Assert(t, icmd.Success)

e2e/container/run_test.go

@@ -132,6 +132,7 @@ func TestTrustedRunFromBadTrustServer(t *testing.T) {
 
 // TODO: create this with registry API instead of engine API
 func createRemoteImage(t *testing.T) string {
+	t.Helper()
 	image := registryPrefix + "/alpine:test-run-pulls"
 	icmd.RunCommand("docker", "pull", fixtures.AlpineImage).Assert(t, icmd.Success)
 	icmd.RunCommand("docker", "tag", fixtures.AlpineImage, image).Assert(t, icmd.Success)

e2e/image/push_test.go

@@ -323,6 +323,7 @@ func TestPushWithContentTrustSignsForRolesWithKeysAndValidPaths(t *testing.T) {
 }
 
 func createImage(t *testing.T, repo string, tags ...string) string {
+	t.Helper()
 	icmd.RunCommand("docker", "pull", fixtures.AlpineImage).Assert(t, icmd.Success)
 
 	for _, tag := range tags {
@@ -345,6 +346,7 @@ func withNotaryPassphrase(pwd string) func(*icmd.Cmd) {
 }
 
 func notaryImportPrivateKey(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef, role, privkey string) {
+	t.Helper()
 	icmd.RunCmd(
 		icmd.Command(notary, "-c", notaryDir.Join("client-config.json"), "key", "import", privkey, "-g", baseRef, "-r", role),
 		withNotaryPassphrase("foo"),
@@ -353,6 +355,7 @@ func notaryImportPrivateKey(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef, r
 }
 
 func notaryPublish(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef string) {
+	t.Helper()
 	icmd.RunCmd(
 		icmd.Command(notary, "-c", notaryDir.Join("client-config.json"), "publish", baseRef),
 		withNotaryPassphrase("foo"),
@@ -361,6 +364,7 @@ func notaryPublish(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef string) {
 }
 
 func notaryAddDelegation(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef, role, pubkey string, paths ...string) {
+	t.Helper()
 	pathsArg := "--all-paths"
 	if len(paths) > 0 {
 		pathsArg = "--paths=" + strings.Join(paths, ",")
@@ -373,6 +377,7 @@ func notaryAddDelegation(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef, role
 }
 
 func notaryInit(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef string) {
+	t.Helper()
 	icmd.RunCmd(
 		icmd.Command(notary, "-c", notaryDir.Join("client-config.json"), "init", baseRef),
 		withNotaryPassphrase("foo"),
@@ -381,6 +386,7 @@ func notaryInit(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef string) {
 }
 
 func notaryListTargetsInRole(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef, role string) map[string]string {
+	t.Helper()
 	result := icmd.RunCmd(
 		icmd.Command(notary, "-c", notaryDir.Join("client-config.json"), "list", baseRef, "-r", role),
 		fixtures.WithHome(homeDir.Path()),
@@ -413,6 +419,7 @@ func notaryListTargetsInRole(t *testing.T, notaryDir, homeDir *fs.Dir, baseRef,
 }
 
 func setupNotaryConfig(t *testing.T, dockerConfigDir fs.Dir) *fs.Dir {
+	t.Helper()
 	return fs.NewDir(t, "notary_test", fs.WithMode(0o700),
 		fs.WithFile("client-config.json", fmt.Sprintf(`
 {
@@ -425,5 +432,6 @@ func setupNotaryConfig(t *testing.T, dockerConfigDir fs.Dir) *fs.Dir {
 }
 
 func copyPrivateKey(t *testing.T, dir, source string) {
+	t.Helper()
 	icmd.RunCommand("/bin/cp", source, dir).Assert(t, icmd.Success)
 }

e2e/plugin/trust_test.go

@@ -72,6 +72,7 @@ func TestInstallWithContentTrustUntrusted(t *testing.T) {
 }
 
 func preparePluginDir(t *testing.T) *fs.Dir {
+	t.Helper()
 	p := &types.PluginConfig{
 		Interface: types.PluginConfigInterface{
 			Socket: "basic.sock",

e2e/stack/remove_test.go

@@ -22,6 +22,7 @@ func TestRemove(t *testing.T) {
 }
 
 func deployFullStack(t *testing.T, stackname string) {
+	t.Helper()
 	// TODO: this stack should have full options not minimal options
 	result := icmd.RunCommand("docker", "stack", "deploy",
 		"--compose-file=./testdata/full-stack.yml", stackname)
@@ -31,6 +32,7 @@ func deployFullStack(t *testing.T, stackname string) {
 }
 
 func cleanupFullStack(t *testing.T, stackname string) {
+	t.Helper()
 	// FIXME(vdemeester) we shouldn't have to do that. it is hiding a race on docker stack rm
 	poll.WaitOn(t, stackRm(stackname), pollSettings)
 	poll.WaitOn(t, taskCount(stackname, 0), pollSettings)

e2e/trust/revoke_test.go

@@ -47,6 +47,7 @@ func TestRevokeRepo(t *testing.T) {
 }
 
 func setupTrustedImagesForRevoke(t *testing.T, dir fs.Dir) {
+	t.Helper()
 	icmd.RunCmd(icmd.Command("docker", "pull", fixtures.AlpineImage)).Assert(t, icmd.Success)
 	icmd.RunCommand("docker", "tag", fixtures.AlpineImage, revokeImage).Assert(t, icmd.Success)
 	icmd.RunCmd(
@@ -56,6 +57,7 @@ func setupTrustedImagesForRevoke(t *testing.T, dir fs.Dir) {
 }
 
 func setupTrustedImagesForRevokeRepo(t *testing.T, dir fs.Dir) {
+	t.Helper()
 	icmd.RunCmd(icmd.Command("docker", "pull", fixtures.AlpineImage)).Assert(t, icmd.Success)
 	icmd.RunCommand("docker", "tag", fixtures.AlpineImage, fmt.Sprintf("%s:v1", revokeRepo)).Assert(t, icmd.Success)
 	icmd.RunCmd(

e2e/trust/sign_test.go

@@ -54,6 +54,7 @@ func TestSignWithLocalFlag(t *testing.T) {
 }
 
 func setupTrustedImageForOverwrite(t *testing.T, dir fs.Dir) {
+	t.Helper()
 	icmd.RunCmd(icmd.Command("docker", "pull", fixtures.AlpineImage)).Assert(t, icmd.Success)
 	icmd.RunCommand("docker", "tag", fixtures.AlpineImage, localImage).Assert(t, icmd.Success)
 	result := icmd.RunCmd(

internal/test/strings.go

@@ -10,6 +10,7 @@ import (
 
 // CompareMultipleValues compares comma-separated values, whatever the order is
 func CompareMultipleValues(t *testing.T, value, expected string) {
+	t.Helper()
 	// comma-separated values means probably a map input, which won't
 	// be guaranteed to have the same order as our expected value
 	// We'll create maps and use reflect.DeepEquals to check instead:

opts/port_test.go

@@ -361,6 +361,7 @@ func TestConvertPortToPortConfigWithIP(t *testing.T) {
 }
 
 func assertContains(t *testing.T, portConfigs []swarm.PortConfig, expected swarm.PortConfig) {
+	t.Helper()
 	contains := false
 	for _, portConfig := range portConfigs {
 		if portConfig == expected {