diff --git a/mod/tigron/.golangci.yml b/mod/tigron/.golangci.yml index 1c17cd11206..22d2a86dd71 100644 --- a/mod/tigron/.golangci.yml +++ b/mod/tigron/.golangci.yml @@ -12,25 +12,81 @@ issues: linters: default: all + enable: + # These are the default set of golangci (errcheck is disabled, see below) + - govet # Vet examines Go source code and reports suspicious constructs. It is roughly the same as 'go vet' and uses its passes. + - ineffassign # Detects when assignments to existing variables are not used. + - staticcheck # It's the set of rules from staticcheck. + - unused # Checks Go code for unused constants, variables, functions and types. + # These are the linters we knowingly want enabled in addition to the default set + - containedctx # avoid embedding context into structs + - depguard # Allows to explicitly allow or disallow third party modules + - err113 # encourage static errors + - gochecknoglobals # globals should be avoided as much as possible + - godot # forces dot at the end of comments + - gosec # various security checks + - interfacebloat # limit complexity in public APIs + - paralleltest # enforces tests using parallel + - revive # meta linter (see settings below) + - testpackage # test packages should be separate from the package they test (eg: name them package_test) + - testableexamples # makes sure that examples are testable (have an expected output) + - thelper # enforces use of t.Helper() + - varnamelen # encourage readable descriptive names for variables instead of x, y, z disable: # These are the linters that we know we do not want - - cyclop # provided by revive - - exhaustruct # does not serve much of a purpose - - funlen # provided by revive - - gocognit # provided by revive - - goconst # provided by revive - - godox # not helpful unless we could downgrade it to warning / info - - ginkgolinter # no ginkgo - - gomodguard # we use depguard instead - - ireturn # too annoying with not enough value - - lll # provided by golines - - nonamedreturns # named returns are occasionally useful - - prealloc # premature optimization - - promlinter # no prometheus - - sloglint # no slog - - testifylint # no testify - - zerologlint # no zerolog + - cyclop # provided by revive + - exhaustruct # does not serve much of a purpose + - errcheck # provided by revive + - forcetypeassert # provided by revive + - funlen # provided by revive + - gocognit # provided by revive + - goconst # provided by revive + - godox # not helpful unless we could downgrade it to warning / info + - ginkgolinter # no ginkgo + - gomodguard # we use depguard instead + - ireturn # too annoying with not enough value + - lll # provided by golines + - nestif # already provided ten different ways with revive cognitive complexity, etc + - nonamedreturns # named returns are occasionally useful + - prealloc # premature optimization + - promlinter # no prometheus + - sloglint # no slog + - testifylint # no testify + - zerologlint # no zerolog settings: + interfacebloat: + # Default is 10 + max: 13 + revive: + enable-all-rules: true + rules: + - name: cognitive-complexity + # Default is 7 + arguments: [60] + - name: function-length + # Default is 50, 75 + arguments: [80, 180] + - name: cyclomatic + # Default is 10 + arguments: [30] + - name: add-constant + arguments: + - allowInts: "0,1,2" + allowStrs: '""' + - name: flag-parameter + # Not sure why this is valuable. + disabled: true + - name: line-length-limit + # Formatter `golines` takes care of this. + disabled: true + - name: unhandled-error + arguments: + - "fmt.Print" + - "fmt.Println" + - "fmt.Printf" + - "fmt.Fprint" + - "fmt.Fprintln" + - "fmt.Fprintf" depguard: rules: main: diff --git a/mod/tigron/expect/comparators_test.go b/mod/tigron/expect/comparators_test.go index 211ca7ad03e..c6710f6073b 100644 --- a/mod/tigron/expect/comparators_test.go +++ b/mod/tigron/expect/comparators_test.go @@ -14,8 +14,11 @@ limitations under the License. */ +//revive:disable:add-constant package expect_test +// TODO: add a lot more tests including failure conditions with mimicry + import ( "regexp" "testing" diff --git a/mod/tigron/internal/com/command.go b/mod/tigron/internal/com/command.go index e71869d5ade..cc8959a7e13 100644 --- a/mod/tigron/internal/com/command.go +++ b/mod/tigron/internal/com/command.go @@ -43,16 +43,14 @@ var ( ErrFailedStarting = errors.New("command failed starting") // ErrSignaled is returned by Wait() if a signal was sent to the command while running. ErrSignaled = errors.New("command execution signaled") - // ErrExecutionFailed is returned by Wait() when a command executes but returns a non-zero error - // code. + // ErrExecutionFailed is returned by Wait() when a command executes but returns a non-zero error code. ErrExecutionFailed = errors.New("command returned a non-zero exit code") // ErrFailedSendingSignal may happen if sending a signal to an already terminated process. ErrFailedSendingSignal = errors.New("failed sending signal") // ErrExecAlreadyStarted is a system error normally indicating a bogus double call to Run(). ErrExecAlreadyStarted = errors.New("command has already been started (double `Run`)") - // ErrExecNotStarted is a system error normally indicating that Wait() has been called without - // first calling Run(). + // ErrExecNotStarted is a system error normally indicating that Wait() has been called without first calling Run(). ErrExecNotStarted = errors.New("command has not been started (call `Run` first)") // ErrExecAlreadyFinished is a system error indicating a double call to Wait(). ErrExecAlreadyFinished = errors.New("command is already finished") @@ -75,7 +73,7 @@ type Result struct { } type execution struct { - //nolint:containedctx + //nolint:containedctx // Is there a way around this? context context.Context cancel context.CancelFunc command *exec.Cmd @@ -93,10 +91,10 @@ type Command struct { WrapArgs []string Timeout time.Duration - WorkingDir string - Env map[string]string - // FIXME: EnvBlackList might change for a better mechanism (regexp and/or whitelist + blacklist) + WorkingDir string + Env map[string]string EnvBlackList []string + EnvWhiteList []string writers []func() io.Reader @@ -122,6 +120,7 @@ func (gc *Command) Clone() *Command { WorkingDir: gc.WorkingDir, Env: map[string]string{}, EnvBlackList: append([]string(nil), gc.EnvBlackList...), + EnvWhiteList: append([]string(nil), gc.EnvWhiteList...), writers: append([]func() io.Reader(nil), gc.writers...), @@ -137,9 +136,8 @@ func (gc *Command) Clone() *Command { return com } -// WithPTY requests that the command be executed with a pty for std streams. Parameters allow -// showing which streams -// are to be tied to the pty. +// WithPTY requests that the command be executed with a pty for std streams. +// Parameters allow showing which streams are to be tied to the pty. // This command has no effect if Run has already been called. func (gc *Command) WithPTY(stdin, stdout, stderr bool) { gc.ptyStdout = stdout @@ -147,17 +145,15 @@ func (gc *Command) WithPTY(stdin, stdout, stderr bool) { gc.ptyStdin = stdin } -// WithFeeder ensures that the provider function will be executed and its output fed to the command -// stdin. WithFeeder, like Feed, can be used multiple times, and writes will be performed -// sequentially, in order. +// WithFeeder ensures that the provider function will be executed and its output fed to the command stdin. +// WithFeeder, like Feed, can be used multiple times, and writes will be performed sequentially, in order. // This command has no effect if Run has already been called. func (gc *Command) WithFeeder(writers ...func() io.Reader) { gc.writers = append(gc.writers, writers...) } // Feed ensures that the provider reader will be copied on the command stdin. -// Feed, like WithFeeder, can be used multiple times, and writes will be performed in sequentially, -// in order. +// Feed, like WithFeeder, can be used multiple times, and writes will be performed in sequentially, in order. // This command has no effect if Run has already been called. func (gc *Command) Feed(reader io.Reader) { gc.writers = append(gc.writers, func() io.Reader { @@ -197,7 +193,6 @@ func (gc *Command) Run(parentCtx context.Context) error { // Create a contextual command, set the logger cmd = gc.buildCommand(ctx) - // Get a debug-logger from the context var ( log logger.Logger @@ -338,8 +333,7 @@ func (gc *Command) wrap() error { err error ) - // XXXgolang: this is troubling. cmd.ProcessState.ExitCode() is always fine, even if - // cmd.ProcessState is nil. + // XXXgolang: this is troubling. cmd.ProcessState.ExitCode() is always fine, even if cmd.ProcessState is nil. exitCode = cmd.ProcessState.ExitCode() if cmd.ProcessState != nil { @@ -356,7 +350,7 @@ func (gc *Command) wrap() error { } } - // Catch-up on the context + // Catch-up on the context. switch ctx.Err() { case context.DeadlineExceeded: err = ErrTimeout @@ -365,7 +359,7 @@ func (gc *Command) wrap() error { default: } - // Stuff everything in Result and return err + // Stuff everything in Result and return err. gc.result = &Result{ ExitCode: exitCode, Stdout: pipes.fromStdout, @@ -382,7 +376,7 @@ func (gc *Command) wrap() error { } func (gc *Command) buildCommand(ctx context.Context) *exec.Cmd { - // Build arguments and binary + // Build arguments and binary. args := gc.Args if gc.PrependArgs != nil { args = append(gc.PrependArgs, args...) @@ -399,26 +393,55 @@ func (gc *Command) buildCommand(ctx context.Context) *exec.Cmd { //nolint:gosec cmd := exec.CommandContext(ctx, binary, args...) - // Add dir + // Add dir. cmd.Dir = gc.WorkingDir - // Set wait delay after waits returns + // Set wait delay after waits returns. cmd.WaitDelay = delayAfterWait - // Build env + // Build env. cmd.Env = []string{} - // TODO: replace with regexps? and/or whitelist? + + const ( + star = "*" + equal = "=" + ) + for _, envValue := range os.Environ() { add := true - for _, b := range gc.EnvBlackList { - if b == "*" || strings.HasPrefix(envValue, b+"=") { + for _, needle := range gc.EnvBlackList { + if strings.HasSuffix(needle, star) { + needle = strings.TrimSuffix(needle, star) + } else if needle != star && !strings.Contains(needle, equal) { + needle += equal + } + + if needle == star || strings.HasPrefix(envValue, needle) { add = false break } } + if len(gc.EnvWhiteList) > 0 { + add = false + + for _, needle := range gc.EnvWhiteList { + if strings.HasSuffix(needle, star) { + needle = strings.TrimSuffix(needle, star) + } else if needle != star && !strings.Contains(needle, equal) { + needle += equal + } + + if needle == star || strings.HasPrefix(envValue, needle) { + add = true + + break + } + } + } + if add { cmd.Env = append(cmd.Env, envValue) } @@ -429,12 +452,12 @@ func (gc *Command) buildCommand(ctx context.Context) *exec.Cmd { cmd.Env = append(cmd.Env, k+"="+v) } - // Attach platform ProcAttr and get optional custom cancellation routine + // Attach platform ProcAttr and get optional custom cancellation routine. if cancellation := addAttr(cmd); cancellation != nil { cmd.Cancel = func() error { gc.exec.log.Log("command cancelled") - // Call the platform dependent cancellation routine + // Call the platform dependent cancellation routine. return cancellation() } } diff --git a/mod/tigron/internal/com/command_other.go b/mod/tigron/internal/com/command_other.go index 7bddc09c9ff..a510753b019 100644 --- a/mod/tigron/internal/com/command_other.go +++ b/mod/tigron/internal/com/command_other.go @@ -24,10 +24,10 @@ import ( ) func addAttr(cmd *exec.Cmd) func() error { - // Default shutdown will leave child processes behind in certain circumstances + // Default shutdown will leave child processes behind in certain circumstances. cmd.SysProcAttr = &syscall.SysProcAttr{ Setsid: true, - // FIXME: understand why we would want that + // FIXME: understand why we would want that. // Setctty: true, } diff --git a/mod/tigron/internal/com/command_test.go b/mod/tigron/internal/com/command_test.go index 2f62f1ef246..7652a772675 100644 --- a/mod/tigron/internal/com/command_test.go +++ b/mod/tigron/internal/com/command_test.go @@ -14,6 +14,7 @@ limitations under the License. */ +//revive:disable:add-constant package com_test import ( @@ -49,18 +50,18 @@ func TestFaultyDoubleRunWait(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") err = command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIs(t, err, com.ErrExecAlreadyStarted) + assertive.ErrorIs(t, err, com.ErrExecAlreadyStarted, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, expect.ExitCodeSuccess, res.ExitCode) - assertive.IsEqual(t, "one", res.Stdout) - assertive.IsEqual(t, "", res.Stderr) + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, expect.ExitCodeSuccess, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "one", res.Stdout, "Stdout") + assertive.IsEqual(t, "", res.Stderr, "Stderr") } func TestFaultyRunDoubleWait(t *testing.T) { @@ -75,21 +76,21 @@ func TestFaultyRunDoubleWait(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, expect.ExitCodeSuccess, res.ExitCode) - assertive.IsEqual(t, "one", res.Stdout) - assertive.IsEqual(t, "", res.Stderr) + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, expect.ExitCodeSuccess, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "one", res.Stdout, "Stdout") + assertive.IsEqual(t, "", res.Stderr, "Stderr") res, err = command.Wait() - assertive.ErrorIs(t, err, com.ErrExecAlreadyFinished) - assertive.IsEqual(t, expect.ExitCodeSuccess, res.ExitCode) - assertive.IsEqual(t, "one", res.Stdout) - assertive.IsEqual(t, "", res.Stderr) + assertive.ErrorIs(t, err, com.ErrExecAlreadyFinished, "Err") + assertive.IsEqual(t, expect.ExitCodeSuccess, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "one", res.Stdout, "Stdout") + assertive.IsEqual(t, "", res.Stderr, "Stderr") } func TestFailRun(t *testing.T) { @@ -101,25 +102,25 @@ func TestFailRun(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIs(t, err, com.ErrFailedStarting) + assertive.ErrorIs(t, err, com.ErrFailedStarting, "Err") err = command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIs(t, err, com.ErrExecAlreadyFinished) + assertive.ErrorIs(t, err, com.ErrExecAlreadyFinished, "Err") res, err := command.Wait() - assertive.ErrorIs(t, err, com.ErrFailedStarting) - assertive.IsEqual(t, -1, res.ExitCode) - assertive.IsEqual(t, "", res.Stdout) - assertive.IsEqual(t, "", res.Stderr) + assertive.ErrorIs(t, err, com.ErrFailedStarting, "Err") + assertive.IsEqual(t, -1, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "", res.Stdout, "Stdout") + assertive.IsEqual(t, "", res.Stderr, "Stderr") res, err = command.Wait() - assertive.ErrorIs(t, err, com.ErrFailedStarting) - assertive.IsEqual(t, -1, res.ExitCode) - assertive.IsEqual(t, "", res.Stdout) - assertive.IsEqual(t, "", res.Stderr) + assertive.ErrorIs(t, err, com.ErrFailedStarting, "Err") + assertive.IsEqual(t, -1, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "", res.Stdout, "Stdout") + assertive.IsEqual(t, "", res.Stderr, "Stderr") } func TestBasicRunWait(t *testing.T) { @@ -132,14 +133,14 @@ func TestBasicRunWait(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, 0, res.ExitCode) - assertive.IsEqual(t, "one", res.Stdout) - assertive.IsEqual(t, "", res.Stderr) + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, 0, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "one", res.Stdout, "Stdout") + assertive.IsEqual(t, "", res.Stderr, "Stderr") } func TestBasicFail(t *testing.T) { @@ -152,14 +153,14 @@ func TestBasicFail(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIs(t, err, com.ErrExecutionFailed) - assertive.IsEqual(t, 127, res.ExitCode) - assertive.IsEqual(t, "", res.Stdout) - assertive.HasSuffix(t, res.Stderr, "does-not-exist: command not found\n") + assertive.ErrorIs(t, err, com.ErrExecutionFailed, "Err") + assertive.IsEqual(t, 127, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "", res.Stdout, "Stdout") + assertive.HasSuffix(t, res.Stderr, "does-not-exist: command not found\n", "Stderr") } func TestWorkingDir(t *testing.T) { @@ -173,12 +174,12 @@ func TestWorkingDir(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, 0, res.ExitCode) + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, 0, res.ExitCode, "ExitCode") // Note: // - darwin will link to /private/DIR, so, check with HasSuffix @@ -187,43 +188,72 @@ func TestWorkingDir(t *testing.T) { t.Skip("skipping last check on windows, see note") } - assertive.HasSuffix(t, res.Stdout, dir+"\n") + assertive.HasSuffix(t, res.Stdout, dir+"\n", "Stdout") } func TestEnvBlacklist(t *testing.T) { t.Setenv("FOO", "BAR") t.Setenv("FOOBAR", "BARBAR") + // First, test that environment gets through to the command command := &com.Command{ Binary: "env", + // Note: LS_COLORS is just too loud + EnvBlackList: []string{ + "LS_COLORS", + }, } err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, 0, res.ExitCode) - assertive.Contains(t, res.Stdout, "FOO=BAR") - assertive.Contains(t, res.Stdout, "FOOBAR=BARBAR") + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, 0, res.ExitCode, "ExitCode") + assertive.Contains(t, res.Stdout, "FOO=BAR", "Stdout") + assertive.Contains(t, res.Stdout, "FOOBAR=BARBAR", "Stdout") + // Now test that we can blacklist a single variable with fully qualified name (FOO) command = &com.Command{ - Binary: "env", - EnvBlackList: []string{"FOO"}, + Binary: "env", + EnvBlackList: []string{ + "LS_COLORS", + "FOO", + }, + } + + err = command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) + + assertive.ErrorIsNil(t, err, "Err") + + res, err = command.Wait() + + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, res.ExitCode, 0, "ExitCode") + assertive.DoesNotContain(t, res.Stdout, "FOO=", "Stdout") + assertive.Contains(t, res.Stdout, "FOOBAR=BARBAR", "Stdout") + + // Now test that we can blacklist multiple variables with FOO* + command = &com.Command{ + Binary: "env", + EnvBlackList: []string{ + "LS_COLORS", + "FOO*", + }, } err = command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err = command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, res.ExitCode, 0) - assertive.DoesNotContain(t, res.Stdout, "FOO=BAR") - assertive.Contains(t, res.Stdout, "FOOBAR=BARBAR") + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, res.ExitCode, 0, "ExitCode") + assertive.DoesNotContain(t, res.Stdout, "FOO=", "Stdout") + assertive.DoesNotContain(t, res.Stdout, "FOOBAR=", "Stdout") // On windows, with mingw, SYSTEMROOT,TERM and HOME (possibly others) will be forcefully added // to the environment regardless, so, we can't test "*" blacklist @@ -233,20 +263,93 @@ func TestEnvBlacklist(t *testing.T) { ) } + // Now, test that we can blacklist everything command = &com.Command{ Binary: "env", EnvBlackList: []string{"*"}, } err = command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) + assertive.ErrorIsNil(t, err, "Err") + + res, err = command.Wait() + + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, res.ExitCode, 0, "ExitCode") + assertive.IsEqual(t, res.Stdout, "", "Stdout") +} + +func TestEnvWhiteList(t *testing.T) { + t.Setenv("FOO", "BAR") + t.Setenv("FOOBAR", "BARBAR") + + // Test that whitelist does let through only FOO + command := &com.Command{ + Binary: "env", + EnvWhiteList: []string{ + "FOO", + }, + } + + err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") + + res, err := command.Wait() + + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, 0, res.ExitCode, "ExitCode") + assertive.Contains(t, res.Stdout, "FOO=BAR", "Stdout") + assertive.DoesNotContain(t, res.Stdout, "FOOBAR=", "Stdout") + assertive.DoesNotContain(t, res.Stdout, "LS_COLORS=", "Stdout") + + // Test that whitelist does let through FOO and FOOBAR with FOO* + command = &com.Command{ + Binary: "env", + EnvWhiteList: []string{ + "FOO*", + }, + } + + err = command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) + + assertive.ErrorIsNil(t, err, "Err") res, err = command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, res.ExitCode, 0) - assertive.IsEqual(t, res.Stdout, "") + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, 0, res.ExitCode, "ExitCode") + assertive.Contains(t, res.Stdout, "FOO=BAR", "Stdout") + assertive.Contains(t, res.Stdout, "FOOBAR=BARBAR", "Stdout") + assertive.DoesNotContain(t, res.Stdout, "LS_COLORS=", "Stdout") +} + +func TestEnvBlacklistWhiteList(t *testing.T) { + t.Setenv("FOO", "BAR") + t.Setenv("FOOBAR", "BARBAR") + + // Test that if both are specified, only whitelist is taken into account + command := &com.Command{ + Binary: "env", + EnvBlackList: []string{ + "*", + "FOO*", + }, + EnvWhiteList: []string{ + "*", + }, + } + + err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) + + assertive.ErrorIsNil(t, err, "Err") + + res, err := command.Wait() + + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, 0, res.ExitCode, "ExitCode") + assertive.Contains(t, res.Stdout, "FOO=BAR", "Stdout") + assertive.Contains(t, res.Stdout, "FOOBAR=BARBAR", "Stdout") } func TestEnvAdd(t *testing.T) { @@ -261,21 +364,28 @@ func TestEnvAdd(t *testing.T) { "BAR": "NEW", "BLED": "EXPLICIT", }, - EnvBlackList: []string{"BLED"}, + EnvBlackList: []string{ + "LS_COLORS", + "BLED", + }, } err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, res.ExitCode, 0) - assertive.Contains(t, res.Stdout, "FOO=REPLACE") - assertive.Contains(t, res.Stdout, "BAR=NEW") - assertive.Contains(t, res.Stdout, "BAZ=OLD") - assertive.Contains(t, res.Stdout, "BLED=EXPLICIT") + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, res.ExitCode, 0, "ExitCode") + // Confirm explicit Env: declaration overrides os.Environ + assertive.Contains(t, res.Stdout, "FOO=REPLACE", "Stdout") + // Confirm explicit Env: declaration does add a new variable + assertive.Contains(t, res.Stdout, "BAR=NEW", "Stdout") + // Confirm explicit Env: declaration for unrelated variable does not reset os.Environ + assertive.Contains(t, res.Stdout, "BAZ=OLD", "Stdout") + // Confirm that blacklist only operates on os.Environ and not on any explicitly added Env: declaration + assertive.Contains(t, res.Stdout, "BLED=EXPLICIT", "Stdout") } func TestStdoutStderr(t *testing.T) { @@ -288,14 +398,14 @@ func TestStdoutStderr(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, res.ExitCode, 0) - assertive.IsEqual(t, res.Stdout, "onstdout") - assertive.IsEqual(t, res.Stderr, "onstderr") + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, res.ExitCode, 0, "ExitCode") + assertive.IsEqual(t, res.Stdout, "onstdout", "Stdout", "Stdout") + assertive.IsEqual(t, res.Stderr, "onstderr", "Stderr", "Stderr") } func TestTimeoutPlain(t *testing.T) { @@ -312,17 +422,17 @@ func TestTimeoutPlain(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() end := time.Now() - assertive.ErrorIs(t, err, com.ErrTimeout) - assertive.IsEqual(t, res.ExitCode, -1) - assertive.IsEqual(t, res.Stdout, "one") - assertive.IsEqual(t, res.Stderr, "") - assertive.IsLessThan(t, end.Sub(start), 2*time.Second) + assertive.ErrorIs(t, err, com.ErrTimeout, "Err") + assertive.IsEqual(t, res.ExitCode, -1, "ExitCode") + assertive.IsEqual(t, res.Stdout, "one", "Stdout") + assertive.IsEqual(t, res.Stderr, "", "Stderr") + assertive.IsLessThan(t, end.Sub(start), 2*time.Second, "Total execution time") } func TestTimeoutDelayed(t *testing.T) { @@ -339,7 +449,7 @@ func TestTimeoutDelayed(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") time.Sleep(1 * time.Second) @@ -347,11 +457,11 @@ func TestTimeoutDelayed(t *testing.T) { end := time.Now() - assertive.ErrorIs(t, err, com.ErrTimeout) - assertive.IsEqual(t, res.ExitCode, -1) - assertive.IsEqual(t, res.Stdout, "one") - assertive.IsEqual(t, res.Stderr, "") - assertive.IsLessThan(t, end.Sub(start), 2*time.Second) + assertive.ErrorIs(t, err, com.ErrTimeout, "Err") + assertive.IsEqual(t, res.ExitCode, -1, "ExitCode") + assertive.IsEqual(t, res.Stdout, "one", "Stdout") + assertive.IsEqual(t, res.Stderr, "", "Stderr") + assertive.IsLessThan(t, end.Sub(start), 2*time.Second, "Total execution time") } func TestPTYStdout(t *testing.T) { @@ -375,14 +485,14 @@ func TestPTYStdout(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, res.ExitCode, 0) - assertive.IsEqual(t, res.Stdout, "onstdout") - assertive.IsEqual(t, res.Stderr, "onstderr") + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, res.ExitCode, 0, "ExitCode") + assertive.IsEqual(t, res.Stdout, "onstdout", "Stdout") + assertive.IsEqual(t, res.Stderr, "onstderr", "Stderr") } func TestPTYStderr(t *testing.T) { @@ -406,14 +516,14 @@ func TestPTYStderr(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, res.ExitCode, 0) - assertive.IsEqual(t, res.Stdout, "onstdout") - assertive.IsEqual(t, res.Stderr, "onstderr") + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, res.ExitCode, 0, "ExitCode") + assertive.IsEqual(t, res.Stdout, "onstdout", "Stdout") + assertive.IsEqual(t, res.Stderr, "onstderr", "Stderr") } func TestPTYBoth(t *testing.T) { @@ -435,14 +545,14 @@ func TestPTYBoth(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, res.ExitCode, 0) - assertive.IsEqual(t, res.Stdout, "onstdoutonstderr") - assertive.IsEqual(t, res.Stderr, "") + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, res.ExitCode, 0, "ExitCode") + assertive.IsEqual(t, res.Stdout, "onstdoutonstderr", "Stdout") + assertive.IsEqual(t, res.Stderr, "", "Stderr") } func TestWriteStdin(t *testing.T) { @@ -468,13 +578,13 @@ func TestWriteStdin(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIsNil(t, err) - assertive.IsEqual(t, 0, res.ExitCode) - assertive.IsEqual(t, "from stdinhello firsthello worldhello again", res.Stdout) + assertive.ErrorIsNil(t, err, "Err") + assertive.IsEqual(t, 0, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "from stdinhello firsthello worldhello again", res.Stdout, "Stdout") } func TestWritePTYStdin(t *testing.T) { @@ -503,13 +613,13 @@ func TestWritePTYStdin(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIs(t, err, com.ErrTimeout) - assertive.IsEqual(t, -1, res.ExitCode) - assertive.IsEqual(t, "hello firsthello worldhello again", res.Stdout) + assertive.ErrorIs(t, err, com.ErrTimeout, "Err") + assertive.IsEqual(t, -1, res.ExitCode, "ExitCode") + assertive.IsEqual(t, "hello firsthello worldhello again", res.Stdout, "Stdout") } func TestSignalOnCompleted(t *testing.T) { @@ -524,15 +634,15 @@ func TestSignalOnCompleted(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") _, err = command.Wait() - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") err = command.Signal(usig) - assertive.ErrorIs(t, err, com.ErrFailedSendingSignal) + assertive.ErrorIs(t, err, com.ErrFailedSendingSignal, "Err") } // FIXME: this is not working as expected, and proc.Signal returns nil error while it should not. @@ -549,7 +659,7 @@ func TestSignalOnCompleted(t *testing.T) { // // err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) // -// assertive.ErrorIsNil(t, err) +// assertive.ErrorIsNil(t, err, "Err") // // time.Sleep(1 * time.Second) // @@ -583,22 +693,22 @@ func TestSignalNormal(t *testing.T) { err := command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") // A bit arbitrary - just want to wait for stdout to go through before sending the signal time.Sleep(100 * time.Millisecond) _ = command.Signal(usig) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err := command.Wait() - assertive.ErrorIs(t, err, com.ErrExecutionFailed) - assertive.IsEqual(t, res.Stdout, "entrysetcaught") - assertive.IsEqual(t, res.Stderr, "") - assertive.IsEqual(t, res.ExitCode, 42) - assertive.True(t, res.Signal == nil) + assertive.ErrorIs(t, err, com.ErrExecutionFailed, "Err") + assertive.IsEqual(t, res.Stdout, "entrysetcaught", "Stdout") + assertive.IsEqual(t, res.Stderr, "", "Stderr") + assertive.IsEqual(t, res.ExitCode, 42, "ExitCode") + assertive.True(t, res.Signal == nil, "Signal") command = &com.Command{ Binary: "sleep", @@ -608,17 +718,17 @@ func TestSignalNormal(t *testing.T) { err = command.Run(context.WithValue(context.Background(), com.LoggerKey, t)) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") err = command.Signal(usig) - assertive.ErrorIsNil(t, err) + assertive.ErrorIsNil(t, err, "Err") res, err = command.Wait() - assertive.ErrorIs(t, err, com.ErrSignaled) - assertive.IsEqual(t, res.Stdout, "") - assertive.IsEqual(t, res.Stderr, "") - assertive.IsEqual(t, res.Signal, usig) - assertive.IsEqual(t, res.ExitCode, -1) + assertive.ErrorIs(t, err, com.ErrSignaled, "Err") + assertive.IsEqual(t, res.Stdout, "", "Stdout") + assertive.IsEqual(t, res.Stderr, "", "Stderr") + assertive.IsEqual(t, res.Signal, usig, "Signal") + assertive.IsEqual(t, res.ExitCode, -1, "ExitCode") } diff --git a/mod/tigron/internal/com/package_example_test.go b/mod/tigron/internal/com/package_example_test.go index 38df0249fef..4090a81b9e8 100644 --- a/mod/tigron/internal/com/package_example_test.go +++ b/mod/tigron/internal/com/package_example_test.go @@ -14,6 +14,7 @@ limitations under the License. */ +//revive:disable:add-constant package com_test import ( diff --git a/mod/tigron/internal/com/package_test.go b/mod/tigron/internal/com/package_test.go index 1b6c1c5d526..eaadadc4c8a 100644 --- a/mod/tigron/internal/com/package_test.go +++ b/mod/tigron/internal/com/package_test.go @@ -50,10 +50,10 @@ func TestMain(m *testing.M) { diff := highk.Diff(string(before), string(after)) if len(diff) != 0 { - _, _ = fmt.Fprintln(os.Stderr, "Leaking file descriptors") + fmt.Fprintln(os.Stderr, "Leaking file descriptors") for _, file := range diff { - _, _ = fmt.Fprintln(os.Stderr, file) + fmt.Fprintln(os.Stderr, file) } exitCode = 1 @@ -61,8 +61,8 @@ func TestMain(m *testing.M) { } if err := highk.FindGoRoutines(); err != nil { - _, _ = fmt.Fprintln(os.Stderr, "Leaking go routines") - _, _ = fmt.Fprintln(os.Stderr, err.Error()) + fmt.Fprintln(os.Stderr, "Leaking go routines") + fmt.Fprintln(os.Stderr, err.Error()) exitCode = 1 } diff --git a/mod/tigron/internal/com/pipes.go b/mod/tigron/internal/com/pipes.go index fc8f9e32baf..adda3d2ed0e 100644 --- a/mod/tigron/internal/com/pipes.go +++ b/mod/tigron/internal/com/pipes.go @@ -33,11 +33,9 @@ import ( var ( // ErrFailedCreating could be returned by newStdPipes() on pty creation failure. ErrFailedCreating = errors.New("failed acquiring pipe") - // ErrFailedReading could be returned by the ioGroup in case the go routines fails to read out - // of a pipe. + // ErrFailedReading could be returned by the ioGroup in case the go routines fails to read out of a pipe. ErrFailedReading = errors.New("failed reading") - // ErrFailedWriting could be returned by the ioGroup in case the go routines fails to write on a - // pipe. + // ErrFailedWriting could be returned by the ioGroup in case the go routines fails to write on a pipe. ErrFailedWriting = errors.New("failed writing") ) @@ -108,7 +106,6 @@ func (pipes *stdPipes) closeCaller() { } } -//nolint:gocognit func newStdPipes( ctx context.Context, log *logger.ConcreteLogger, diff --git a/mod/tigron/internal/doc.go b/mod/tigron/internal/doc.go new file mode 100644 index 00000000000..861260a5b73 --- /dev/null +++ b/mod/tigron/internal/doc.go @@ -0,0 +1,21 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +// Package internal provides an assert library, pty, a command wrapper, and a leak detection library +// for internal use in Tigron. The objective for these is not to become generic use-cases libraries, +// but instead to deliver what Tigron +// needs in the simplest possible form. +package internal diff --git a/mod/tigron/internal/exit.go b/mod/tigron/internal/exit.go index 3fddd602491..7a124a01730 100644 --- a/mod/tigron/internal/exit.go +++ b/mod/tigron/internal/exit.go @@ -14,11 +14,6 @@ limitations under the License. */ -// Package internal provides an assert library, pty, a command wrapper, and a leak detection library -// for internal use in Tigron. -// The objective for these is not to become generic use-cases libraries, but instead to deliver what -// Tigron needs -// in the simplest possible form. package internal // This is duplicated from `expect` to avoid circular imports. diff --git a/mod/tigron/internal/formatter/osc8.go b/mod/tigron/internal/formatter/osc8.go index 4a4874a3a9d..6917dad4c68 100644 --- a/mod/tigron/internal/formatter/osc8.go +++ b/mod/tigron/internal/formatter/osc8.go @@ -27,5 +27,6 @@ type OSC8 struct { func (o *OSC8) String() string { // FIXME: not sure if any desktop software does support line numbers anchors? + // FIXME: test that the terminal is able to display these and fallback to printing the information if not. return fmt.Sprintf("\x1b]8;;%s#%d:1\x07%s\x1b]8;;\x07"+"\u001b[0m", o.Location, o.Line, o.Text) } diff --git a/mod/tigron/internal/highk/doc.go b/mod/tigron/internal/highk/doc.go index 1383569de95..46c419008b1 100644 --- a/mod/tigron/internal/highk/doc.go +++ b/mod/tigron/internal/highk/doc.go @@ -14,8 +14,8 @@ limitations under the License. */ -// Package highk (for "high-κ dielectric") is a highly experimental leak detection library -// (for file descriptors and go routines). +// Package highk (for "high-κ dielectric") is a highly experimental leak detection library (for file descriptors and go +// routines). // It is purely internal for now and used only as part of the tests for tigron. // TODO: // - get rid of lsof and implement in go diff --git a/mod/tigron/internal/highk/fileleak.go b/mod/tigron/internal/highk/fileleak.go index af8da5fcfd3..7c3d167a38f 100644 --- a/mod/tigron/internal/highk/fileleak.go +++ b/mod/tigron/internal/highk/fileleak.go @@ -26,9 +26,9 @@ import ( "syscall" ) -// FIXME: it seems that lsof (or go test) is interefering and showing false positive KQUEUE / inodes +// FIXME: it seems that lsof (or go test) is interfering and showing false positive KQUEUE / inodes // -//nolint:gochecknoglobals +//nolint:gochecknoglobals // FIXME rewrite all of this anyhow var whitelist = map[string]bool{ "KQUEUE": true, "a_inode": true, @@ -36,10 +36,10 @@ var whitelist = map[string]bool{ // SnapshotOpenFiles will capture the list of currently open-files for the process. // -//nolint:wrapcheck +//nolint:wrapcheck // FIXME: work in progress func SnapshotOpenFiles(file *os.File) ([]byte, error) { - // Using a buffer would add a pipe to the list of files - // Reimplement this stuff in go ASAP and toss lsof instead of passing around fd + // Using a buffer would add a pipe to the list of files. + // Reimplement this stuff in go ASAP and toss lsof instead of passing around fd. _, _ = file.Seek(0, 0) _ = file.Truncate(0) @@ -48,7 +48,7 @@ func SnapshotOpenFiles(file *os.File) ([]byte, error) { return nil, err } - //nolint:gosec + //nolint:gosec // G204 is fine here cmd := exec.Command(exe, "-nP", "-p", strconv.Itoa(syscall.Getpid())) cmd.Stdout = file diff --git a/mod/tigron/internal/highk/goroutines.go b/mod/tigron/internal/highk/goroutines.go index 121facf8816..30c80e4bb1a 100644 --- a/mod/tigron/internal/highk/goroutines.go +++ b/mod/tigron/internal/highk/goroutines.go @@ -22,7 +22,7 @@ import ( // FindGoRoutines retrieves leaked go routines, which are returned as an error. // -//nolint:wrapcheck +//nolint:wrapcheck // FIXME: work in progress func FindGoRoutines() error { return goleak.Find() } diff --git a/mod/tigron/internal/logger/doc.go b/mod/tigron/internal/logger/doc.go index 29cbb608fd6..8731bcaf6ce 100644 --- a/mod/tigron/internal/logger/doc.go +++ b/mod/tigron/internal/logger/doc.go @@ -14,8 +14,8 @@ limitations under the License. */ -// Package logger is a very simple stub allowing developers to hook whatever logger they want to -// debug internal behavior of the com package. -// The passed logger just has to implement the Log(args...interface{}) method. -// Typically, that would be testing.T. +// Package logger is a very simple stub allowing developers to hook whatever logger they want to debug internal behavior +// of the com package. +// The passed logger just has to implement the Log(args...any) method. +// Typically, that would be a *testing.T. package logger diff --git a/mod/tigron/internal/logger/logger.go b/mod/tigron/internal/logger/logger.go index 7fa26e5b718..a9be51a296c 100644 --- a/mod/tigron/internal/logger/logger.go +++ b/mod/tigron/internal/logger/logger.go @@ -22,13 +22,13 @@ import ( // Logger describes a passed logger, useful only for debugging. type Logger interface { - Log(args ...interface{}) + Log(args ...any) Helper() } // ConcreteLogger is a simple struct allowing to set additional metadata for a Logger. type ConcreteLogger struct { - meta []interface{} + meta []any wrappedLog Logger } @@ -41,12 +41,12 @@ func (cl *ConcreteLogger) Set(key, value string) *ConcreteLogger { } // Log prints a message using the Log method of the embedded Logger. -func (cl *ConcreteLogger) Log(args ...interface{}) { +func (cl *ConcreteLogger) Log(args ...any) { if cl.wrappedLog != nil { cl.wrappedLog.Helper() cl.wrappedLog.Log( append( - append([]interface{}{"[" + time.Now().Format(time.RFC3339) + "]"}, cl.meta...), + append([]any{"[" + time.Now().Format(time.RFC3339) + "]"}, cl.meta...), args...)...) } } diff --git a/mod/tigron/internal/mimicry/mimicry.go b/mod/tigron/internal/mimicry/mimicry.go index 60deb6c1a34..d9afa081dac 100644 --- a/mod/tigron/internal/mimicry/mimicry.go +++ b/mod/tigron/internal/mimicry/mimicry.go @@ -137,8 +137,7 @@ func (mi *Core) Register(fun, handler any) { func getFunID(fun any) string { // The point of keeping only the func name is to avoid type mismatch dependent on what interface - // is used by the - // consumer. + // is used by the consumer. origin := runtime.FuncForPC(reflect.ValueOf(fun).Pointer()).Name() seg := strings.Split(origin, ".") origin = seg[len(seg)-1] diff --git a/mod/tigron/internal/pty/pty.go b/mod/tigron/internal/pty/pty.go index cd564a2731b..a7facf80105 100644 --- a/mod/tigron/internal/pty/pty.go +++ b/mod/tigron/internal/pty/pty.go @@ -35,8 +35,8 @@ var ( ) // Open will allocate and return a new pty. -func Open() (*os.File, *os.File, error) { - pty, tty, err := creack.Open() +func Open() (pty, tty *os.File, err error) { + pty, tty, err = creack.Open() if err != nil { if errors.Is(err, creack.ErrUnsupported) { err = errors.Join(ErrUnsupportedPlatform, err) diff --git a/mod/tigron/require/requirement_test.go b/mod/tigron/require/requirement_test.go index aa4e0fec0f2..040e77e7e08 100644 --- a/mod/tigron/require/requirement_test.go +++ b/mod/tigron/require/requirement_test.go @@ -57,7 +57,7 @@ func TestRequire(t *testing.T) { pass, _ = require.Arch(runtime.GOARCH).Check(nil, nil) } - assertive.IsEqual(t, pass, true) + assertive.IsEqual(t, pass, true, "Require works as expected") } func TestNot(t *testing.T) { @@ -76,7 +76,7 @@ func TestNot(t *testing.T) { pass, _ = require.Not(require.Linux).Check(nil, nil) } - assertive.IsEqual(t, pass, true) + assertive.IsEqual(t, pass, true, "require.Not works as expected") } func TestAllSuccess(t *testing.T) { @@ -99,7 +99,7 @@ func TestAllSuccess(t *testing.T) { require.Not(require.Darwin)).Check(nil, nil) } - assertive.IsEqual(t, pass, true) + assertive.IsEqual(t, pass, true, "require.All works as expected") } func TestAllOneFail(t *testing.T) { @@ -122,5 +122,5 @@ func TestAllOneFail(t *testing.T) { require.Not(require.Darwin)).Check(nil, nil) } - assertive.IsEqual(t, pass, true) + assertive.IsEqual(t, pass, true, "mixing require.All and require.Not works as expected") } diff --git a/mod/tigron/test/command.go b/mod/tigron/test/command.go index ef41610338a..67289079867 100644 --- a/mod/tigron/test/command.go +++ b/mod/tigron/test/command.go @@ -38,7 +38,7 @@ const defaultExecutionTimeout = 3 * time.Minute // FIXME: now that most of the logic got moved to the internal command, consider simplifying this / // removing some of the extra layers from here // -//nolint:interfacebloat + type CustomizableCommand interface { TestableCommand diff --git a/mod/tigron/test/config.go b/mod/tigron/test/config.go index 366b3651fbc..e9f37ddef61 100644 --- a/mod/tigron/test/config.go +++ b/mod/tigron/test/config.go @@ -17,8 +17,6 @@ package test // WithConfig returns a config object with a certain config property set. -// -//nolint:ireturn func WithConfig(key ConfigKey, value ConfigValue) Config { cfg := &config{} cfg.Write(key, value) @@ -26,9 +24,7 @@ func WithConfig(key ConfigKey, value ConfigValue) Config { return cfg } -// Contains the implementation of the Config interface - -//nolint:ireturn +// Contains the implementation of the Config interface. func configureConfig(cfg, parent Config) Config { if cfg == nil { cfg = &config{ @@ -49,7 +45,6 @@ type config struct { config map[ConfigKey]ConfigValue } -//nolint:ireturn func (cfg *config) Write(key ConfigKey, value ConfigValue) Config { if cfg.config == nil { cfg.config = make(map[ConfigKey]ConfigValue) diff --git a/mod/tigron/test/config_test.go b/mod/tigron/test/config_test.go index 6112aee0d09..6f8e85912cd 100644 --- a/mod/tigron/test/config_test.go +++ b/mod/tigron/test/config_test.go @@ -14,7 +14,8 @@ limitations under the License. */ -//nolint:testpackage +//revive:disable:add-constant +//nolint:testpackage // We need to test some internals here package test import ( @@ -46,8 +47,11 @@ func TestConfig(t *testing.T) { cfg2 := WithConfig("test", "two") cfg2.Write("adopt", "two") - //nolint:forcetypeassert - cfg.(*config).adopt(cfg2) + cnf, ok := cfg.(*config) + + assertive.True(t, ok) + + cnf.adopt(cfg2) assertive.IsEqual(t, string(cfg.Read("test")), "one") assertive.IsEqual(t, string(cfg.Read("adopt")), "two") diff --git a/mod/tigron/test/data.go b/mod/tigron/test/data.go index 59bb8218384..8907b9ca199 100644 --- a/mod/tigron/test/data.go +++ b/mod/tigron/test/data.go @@ -25,12 +25,12 @@ import ( ) const ( - identifierMaxLength = 76 + identifierMaxLength = 76 + identifierSeparator = "-" + identifierSignatureLength = 8 ) // WithData returns a data object with a certain key value set. -// -//nolint:ireturn func WithData(key, value string) Data { dat := &data{} dat.Set(key, value) @@ -38,9 +38,7 @@ func WithData(key, value string) Data { return dat } -// Contains the implementation of the Data interface - -//nolint:ireturn +// Contains the implementation of the Data interface. func configureData(t *testing.T, seedData, parent Data) Data { t.Helper() @@ -77,7 +75,6 @@ func (dt *data) Get(key string) string { return dt.labels[key] } -//nolint:ireturn func (dt *data) Set(key, value string) Data { if dt.labels == nil { dt.labels = map[string]string{} @@ -98,11 +95,12 @@ func (dt *data) TempDir() string { func (dt *data) adopt(parent Data) { // Note: implementation dependent - //nolint:forcetypeassert - for k, v := range parent.(*data).labels { - // Only copy keys that are not set already - if _, ok := dt.labels[k]; !ok { - dt.Set(k, v) + if castData, ok := parent.(*data); ok { + for k, v := range castData.labels { + // Only copy keys that are not set already + if _, ok := dt.labels[k]; !ok { + dt.Set(k, v) + } } } } @@ -110,11 +108,11 @@ func (dt *data) adopt(parent Data) { func defaultIdentifierHashing(names ...string) string { // Notes: identifier MAY be used for namespaces, image names, etc. // So, the rules are stringent on what it can contain. - replaceWith := []byte("-") + replaceWith := []byte(identifierSeparator) name := strings.ToLower(strings.Join(names, string(replaceWith))) // Ensure we have a unique identifier despite characters replacements // (well, as unique as the names collection being passed) - signature := fmt.Sprintf("%x", sha256.Sum256([]byte(name)))[0:8] + signature := fmt.Sprintf("%x", sha256.Sum256([]byte(name)))[0:identifierSignatureLength] // Make sure we do not use any unsafe characters safeName := regexp.MustCompile(`[^a-z0-9-]+`) // And we avoid repeats of the separator @@ -126,11 +124,11 @@ func defaultIdentifierHashing(names ...string) string { // Ensure we will never go above 76 characters in length (with signature) if len(name) > (identifierMaxLength - len(signature)) { - name = name[0:67] + name = name[0 : identifierMaxLength-identifierSignatureLength-len(identifierSeparator)] } - if name[len(name)-1:] != "-" { - signature = "-" + signature + if name[len(name)-1:] != identifierSeparator { + signature = identifierSeparator + signature } return name + signature diff --git a/mod/tigron/test/data_test.go b/mod/tigron/test/data_test.go index 9e39ed677ca..dff839d6026 100644 --- a/mod/tigron/test/data_test.go +++ b/mod/tigron/test/data_test.go @@ -15,6 +15,7 @@ */ //nolint:testpackage +//revive:disable:add-constant package test import ( diff --git a/mod/tigron/test/funct.go b/mod/tigron/test/funct.go index 446c003aba5..8614b9cbefb 100644 --- a/mod/tigron/test/funct.go +++ b/mod/tigron/test/funct.go @@ -26,6 +26,7 @@ type Evaluator func(data Data, helpers Helpers) (bool, string) type Butler func(data Data, helpers Helpers) // A Comparator is the function signature to implement for the Output property of an Expected. +// TODO: when we will break API, remove the info parameter. type Comparator func(stdout, info string, t *testing.T) // A Manager is the function signature meant to produce expectations for a command. diff --git a/mod/tigron/test/helpers.go b/mod/tigron/test/helpers.go index 3cc6cae6317..681226047fa 100644 --- a/mod/tigron/test/helpers.go +++ b/mod/tigron/test/helpers.go @@ -74,8 +74,6 @@ func (help *helpersInternal) Err(args ...string) string { } // Command will return a clone of your base command without running it. -// -//nolint:ireturn,nolintlint func (help *helpersInternal) Command(args ...string) TestableCommand { cc := help.cmdInternal.Clone() cc.WithArgs(args...) @@ -85,8 +83,6 @@ func (help *helpersInternal) Command(args ...string) TestableCommand { // Custom will return a command for the requested binary and args, with the environment of your test // (eg: Env, Cwd, etc.) -// -//nolint:ireturn,nolintlint func (help *helpersInternal) Custom(binary string, args ...string) TestableCommand { cc := help.cmdInternal.clear() cc.WithBinary(binary) diff --git a/mod/tigron/test/interfaces.go b/mod/tigron/test/interfaces.go index 8f8727a8298..07f348999c5 100644 --- a/mod/tigron/test/interfaces.go +++ b/mod/tigron/test/interfaces.go @@ -75,7 +75,7 @@ type Helpers interface { // with an Expected. A TestableCommand can be used as a Case Command obviously, but also as part of // a Setup or Cleanup routine, and as the basis of any type of helper. // For more powerful use-cases outside of test cases, see below CustomizableCommand. -type TestableCommand interface { //nolint:interfacebloat +type TestableCommand interface { // WithBinary specifies what binary to execute. WithBinary(binary string) // WithArgs specifies the args to pass to the binary. Note that WithArgs can be used multiple diff --git a/mod/tigron/test/package_test.go b/mod/tigron/test/package_test.go index 5e10a796398..d637f02e53d 100644 --- a/mod/tigron/test/package_test.go +++ b/mod/tigron/test/package_test.go @@ -50,10 +50,10 @@ func TestMain(m *testing.M) { diff := highk.Diff(string(before), string(after)) if len(diff) != 0 { - _, _ = fmt.Fprintln(os.Stderr, "Leaking file descriptors") + fmt.Fprintln(os.Stderr, "Leaking file descriptors") for _, file := range diff { - _, _ = fmt.Fprintln(os.Stderr, file) + fmt.Fprintln(os.Stderr, file) } exitCode = 1 @@ -61,8 +61,8 @@ func TestMain(m *testing.M) { } if err := highk.FindGoRoutines(); err != nil { - _, _ = fmt.Fprintln(os.Stderr, "Leaking go routines") - _, _ = fmt.Fprintln(os.Stderr, os.Stderr, err.Error()) + fmt.Fprintln(os.Stderr, "Leaking go routines") + fmt.Fprintln(os.Stderr, os.Stderr, err.Error()) exitCode = 1 } diff --git a/mod/tigron/test/types.go b/mod/tigron/test/types.go index 0b8130bc2ef..af74be37ef9 100644 --- a/mod/tigron/test/types.go +++ b/mod/tigron/test/types.go @@ -23,8 +23,7 @@ type ( ConfigValue string ) -// A Requirement offers a way to verify random conditions to decide if a test should be skipped or -// run. +// A Requirement offers a way to verify random conditions to decide if a test should be skipped or run. // It can also (optionally) provide custom Setup and Cleanup routines. type Requirement struct { // Check is expected to verify if the requirement is fulfilled, and return a boolean and an