test: add upcloud linter config and fix lint errors

cmd/cmd_internal_test.go

@@ -1,3 +1,4 @@
+//nolint:paralleltest // These can not be run in parallel due to how args are overridden
 package cmd
 
 import (

cmd/root.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"errors"
 	"runtime"
 
 	"github.com/UpCloudLtd/mdtest/testrun"
@@ -15,33 +16,36 @@ var (
 		Short: "A testing tool with markdown testcases",
 		Long:  "Tool for combining examples and test cases. Parses markdown files for test steps and uses these to test command line applications.",
 		Args:  cobra.MinimumNArgs(1),
-		RunE: func(cmd *cobra.Command, args []string) error {
-			cmd.SilenceUsage = true
-			cmd.SilenceErrors = true
-
-			params := testrun.RunParameters{
-				NumberOfJobs: numberOfJobs,
-			}
-
-			res := testrun.Execute(args, params)
-			err := testrun.NewRunError(res)
-			if err != nil {
-				return err
-			}
-
-			return nil
-		},
 	}
 )
 
 func init() {
 	rootCmd.Flags().IntVarP(&numberOfJobs, "jobs", "j", runtime.NumCPU()*2, "number of jobs to use for executing tests in parallel")
+	rootCmd.RunE = func(cmd *cobra.Command, args []string) error {
+		cmd.SilenceUsage = true
+		cmd.SilenceErrors = true
+
+		params := testrun.RunParameters{
+			NumberOfJobs: numberOfJobs,
+			OutputTarget: rootCmd.OutOrStdout(),
+		}
+
+		res := testrun.Execute(args, params)
+		err := testrun.NewRunError(res)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	}
 }
 
 func Execute() int {
 	err := rootCmd.Execute()
 	if err != nil {
-		if runerr, ok := err.(*testrun.RunError); ok {
+		var runerr *testrun.RunError
+		isRunerr := errors.As(err, &runerr)
+		if isRunerr {
 			return runerr.ExitCode()
 		}
 		return 100

cmd/version.go

@@ -9,24 +9,22 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var (
-	versionCmd = &cobra.Command{
-		Use:   "version",
-		Short: "Print version information",
-		Run: func(cmd *cobra.Command, args []string) {
-			data := []output.SummaryItem{
-				{Key: "Version", Value: globals.Version},
-				{Key: "Build date", Value: globals.BuildDate},
-				{Key: "Built with", Value: runtime.Version()},
-				{Key: "System", Value: runtime.GOOS},
-				{Key: "Architecture", Value: runtime.GOARCH},
-			}
-
-			fmt.Print(output.SummaryTable(data))
-		},
-	}
-)
+var versionCmd = &cobra.Command{
+	Use:   "version",
+	Short: "Print version information",
+}
 
 func init() {
 	rootCmd.AddCommand(versionCmd)
+	versionCmd.Run = func(cmd *cobra.Command, args []string) {
+		data := []output.SummaryItem{
+			{Key: "Version", Value: globals.Version},
+			{Key: "Build date", Value: globals.BuildDate},
+			{Key: "Built with", Value: runtime.Version()},
+			{Key: "System", Value: runtime.GOOS},
+			{Key: "Architecture", Value: runtime.GOARCH},
+		}
+
+		fmt.Fprint(versionCmd.OutOrStdout(), output.SummaryTable(data))
+	}
 }

globals/globals.go

@@ -1,3 +1,4 @@
+//nolint:gochecknoglobals // These are defined as var instead of const to be able to override them on compile time.
 package globals
 
 var (

id/id.go

@@ -1,25 +1,22 @@
 package id
 
 import (
+	"crypto/rand"
 	"encoding/base64"
-	"math/rand"
-	"time"
 )
 
-func withIdSuffix(input string) string {
-	rand.Seed(time.Now().Unix())
-
+func withIDSuffix(input string) string {
 	randBytes := make([]byte, 8)
-	rand.Read(randBytes)
+	_, _ = rand.Read(randBytes)
 	randStr := base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(randBytes)
 
 	return input + randStr
 }
 
-func NewRunId() string {
-	return withIdSuffix("run_")
+func NewRunID() string {
+	return withIDSuffix("run_")
 }
 
-func NewTestId() string {
-	return withIdSuffix("test_")
+func NewTestID() string {
+	return withIDSuffix("test_")
 }

testcase/shstep.go

@@ -17,7 +17,7 @@ func unexpectedExitCode(expected, got int) error {
 }
 
 func (s shStep) Execute(t *testStatus) StepResult {
-	cmd := exec.Command("sh", "-xec", s.script)
+	cmd := exec.Command("sh", "-xec", s.script) //nolint:gosec // Here we trust that the user knows what their tests do
 	cmd.Env = t.Env
 	output, err := cmd.CombinedOutput()
 	if err != nil {
@@ -26,7 +26,7 @@ func (s shStep) Execute(t *testStatus) StepResult {
 		if !isExit {
 			return StepResult{
 				Success: false,
-				Error:   fmt.Errorf("unexpected error (%s)", err.Error()),
+				Error:   fmt.Errorf("unexpected error (%w)", err),
 				Output:  string(output),
 			}
 		} else if got := exit.ExitCode(); got != s.exitCode {
@@ -36,13 +36,11 @@ func (s shStep) Execute(t *testStatus) StepResult {
 				Output:  string(output),
 			}
 		}
-	} else {
-		if s.exitCode != 0 {
-			return StepResult{
-				Success: false,
-				Error:   unexpectedExitCode(s.exitCode, 0),
-				Output:  string(output),
-			}
+	} else if s.exitCode != 0 {
+		return StepResult{
+			Success: false,
+			Error:   unexpectedExitCode(s.exitCode, 0),
+			Output:  string(output),
 		}
 	}
 

testcase/testcase.go

@@ -19,19 +19,19 @@ type testStatus struct {
 func NewTestStatus(params TestParameters) testStatus {
 	return testStatus{
 		Env: append(os.Environ(),
-			fmt.Sprintf("MDTEST_JOBID=%d", params.JobId),
-			"MDTEST_RUNID="+params.RunId,
-			"MDTEST_TESTID="+params.TestId,
+			fmt.Sprintf("MDTEST_JOBID=%d", params.JobID),
+			"MDTEST_RUNID="+params.RunID,
+			"MDTEST_TESTID="+params.TestID,
 			"MDTEST_VERSION="+globals.Version,
 		),
 		Params: params,
 	}
 }
 
 type TestParameters struct {
-	JobId   int
-	RunId   string
-	TestId  string
+	JobID   int
+	RunID   string
+	TestID  string
 	TestLog *progress.Progress
 }
 
@@ -47,7 +47,7 @@ type TestResult struct {
 func parse(path string) ([]Step, error) {
 	file, err := os.Open(path)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf(`failed to open test file at "%s" (%w)`, path, err)
 	}
 	defer file.Close()
 
@@ -90,24 +90,24 @@ func getFailureDetails(test TestResult) string {
 func Execute(path string, params TestParameters) TestResult {
 	testLog := params.TestLog
 
-	testLog.Push(messages.Update{Key: path, Message: fmt.Sprintf("Parsing %s", path), Status: messages.MessageStatusStarted})
+	_ = testLog.Push(messages.Update{Key: path, Message: fmt.Sprintf("Parsing %s", path), Status: messages.MessageStatusStarted})
 
 	steps, err := parse(path)
 	if err != nil {
-		testLog.Push(messages.Update{
+		_ = testLog.Push(messages.Update{
 			Key:     path,
 			Status:  messages.MessageStatusError,
 			Details: fmt.Sprintf("Error: %s", err.Error()),
 		})
 		return TestResult{Error: err}
 	}
 
-	testLog.Push(messages.Update{Key: path, Message: fmt.Sprintf("Running %s", path)})
+	_ = testLog.Push(messages.Update{Key: path, Message: fmt.Sprintf("Running %s", path)})
 
 	test := TestResult{StepsCount: len(steps)}
 	status := NewTestStatus(params)
 	for i, step := range steps {
-		testLog.Push(messages.Update{
+		_ = testLog.Push(messages.Update{
 			Key:             path,
 			ProgressMessage: fmt.Sprintf("(Step %d of %d)", i+1, len(steps)),
 		})
@@ -124,9 +124,9 @@ func Execute(path string, params TestParameters) TestResult {
 
 	test.Success = test.FailureCount == 0
 	if test.Success {
-		testLog.Push(messages.Update{Key: path, Status: messages.MessageStatusSuccess})
+		_ = testLog.Push(messages.Update{Key: path, Status: messages.MessageStatusSuccess})
 	} else {
-		testLog.Push(messages.Update{Key: path, Status: messages.MessageStatusError, Details: getFailureDetails(test)})
+		_ = testLog.Push(messages.Update{Key: path, Status: messages.MessageStatusError, Details: getFailureDetails(test)})
 	}
 	return test
 }

testcase/teststep.go

@@ -60,6 +60,6 @@ func parseStep(scanner *bufio.Scanner) (Step, error) {
 	case "sh":
 		return parseShStep(options, content)
 	default:
-		return nil, nil
+		return nil, nil //nolint:nilnil // "Parsed" non-step code block without errors
 	}
 }

testrun/execute.go

@@ -21,24 +21,24 @@ func executeTests(paths []string, params RunParameters, testLog *progress.Progre
 
 	for {
 		select {
-		case curJobId := <-jobQueue:
+		case curJobID := <-jobQueue:
 			if len(testQueue) == 0 {
 				break
 			}
 			curTest := testQueue[0]
 			testQueue = testQueue[1:]
 
-			go func(jobId int, test string) {
+			go func(jobID int, test string) {
 				defer func() {
-					jobQueue <- jobId
+					jobQueue <- jobID
 				}()
 				returnChan <- testcase.Execute(curTest, testcase.TestParameters{
-					JobId:   jobId,
-					RunId:   run.Id,
-					TestId:  id.NewTestId(),
+					JobID:   jobID,
+					RunID:   run.ID,
+					TestID:  id.NewTestID(),
 					TestLog: testLog,
 				})
-			}(curJobId, curTest)
+			}(curJobID, curTest)
 		case res := <-returnChan:
 			if res.Success {
 				run.SuccessCount++

testrun/testrun.go

@@ -2,6 +2,7 @@ package testrun
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"path"
 	"strings"
@@ -16,10 +17,11 @@ import (
 
 type RunParameters struct {
 	NumberOfJobs int
+	OutputTarget io.Writer
 }
 
 type RunResult struct {
-	Id           string
+	ID           string
 	Started      time.Time
 	Finished     time.Time
 	Success      bool
@@ -79,7 +81,7 @@ func parseFilePaths(rawPaths []string, depth int) ([]string, []PathWarning) {
 	return paths, warnings
 }
 
-func PrintSummary(run RunResult) {
+func PrintSummary(target io.Writer, run RunResult) {
 	tests := output.Total(len(run.TestResults))
 	if run.SuccessCount > 0 {
 		tests = output.Passed(run.SuccessCount) + ", " + tests
@@ -95,7 +97,7 @@ func PrintSummary(run RunResult) {
 		{Key: "Elapsed", Value: elapsed},
 	}
 
-	fmt.Printf("\n%s", output.SummaryTable((data)))
+	fmt.Fprintf(target, "\n%s", output.SummaryTable((data)))
 }
 
 func Execute(rawPaths []string, params RunParameters) RunResult {
@@ -106,11 +108,11 @@ func Execute(rawPaths []string, params RunParameters) RunResult {
 	testLog.Start()
 
 	for _, warning := range warnings {
-		testLog.Push(warning.Message())
+		_ = testLog.Push(warning.Message())
 	}
 
 	run := RunResult{
-		Id:      id.NewRunId(),
+		ID:      id.NewRunID(),
 		Started: started,
 		Success: true,
 	}
@@ -120,6 +122,6 @@ func Execute(rawPaths []string, params RunParameters) RunResult {
 	run.Success = run.FailureCount == 0
 	run.Finished = time.Now()
 
-	PrintSummary(run)
+	PrintSummary(params.OutputTarget, run)
 	return run
 }