Skip to content

Commit

Permalink
Fix all the linter errors in cmd package
Browse files Browse the repository at this point in the history
Well most are silenced actually as a lot of them will require a lot of
code changes.
  • Loading branch information
mstoykov committed Jan 25, 2022
1 parent 34f899c commit 1ed7af9
Show file tree
Hide file tree
Showing 26 changed files with 144 additions and 100 deletions.
1 change: 1 addition & 0 deletions cmd/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"go.k6.io/k6/lib/metrics"
)

//nolint:gochecknoglobals
var archiveOut = "archive.tar"

func getArchiveCmd(logger *logrus.Logger) *cobra.Command {
Expand Down
4 changes: 2 additions & 2 deletions cmd/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ var (
showCloudLogs = true
)

//nolint:funlen,gocognit,gocyclo
//nolint:funlen,gocognit,gocyclo,cyclop
func getCloudCmd(ctx context.Context, logger *logrus.Logger) *cobra.Command {
cloudCmd := &cobra.Command{
Use: "cloud",
Expand Down Expand Up @@ -250,7 +250,7 @@ This will execute the test on the k6 cloud service. Use "k6 login cloud" to auth
defer progressBarWG.Wait()
defer progressCancel()
go func() {
showProgress(progressCtx, conf, []*pb.ProgressBar{progressBar}, logger)
showProgress(progressCtx, []*pb.ProgressBar{progressBar}, logger)
progressBarWG.Done()
}()

Expand Down
9 changes: 6 additions & 3 deletions cmd/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,11 @@ import (
)

// Use these when interacting with fs and writing to terminal, makes a command testable
var defaultFs = afero.NewOsFs()
var defaultWriter io.Writer = os.Stdout
//nolint:gochecknoglobals
var (
defaultFs = afero.NewOsFs()
defaultWriter io.Writer = os.Stdout
)

// Panic if the given error is not nil.
func must(err error) {
Expand All @@ -48,7 +51,7 @@ func must(err error) {
}
}

//TODO: refactor the CLI config so these functions aren't needed - they
// TODO: refactor the CLI config so these functions aren't needed - they
// can mask errors by failing only at runtime, not at compile time
func getNullBool(flags *pflag.FlagSet, key string) null.Bool {
v, err := flags.GetBool(key)
Expand Down
10 changes: 6 additions & 4 deletions cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func configFlagSet() *pflag.FlagSet {
return flags
}

// Config ...
type Config struct {
lib.Options

Expand All @@ -65,12 +66,13 @@ type Config struct {
// Validate checks if all of the specified options make sense
func (c Config) Validate() []error {
errors := c.Options.Validate()
//TODO: validate all of the other options... that we should have already been validating...
//TODO: maybe integrate an external validation lib: https://github.com/avelino/awesome-go#validation
// TODO: validate all of the other options... that we should have already been validating...
// TODO: maybe integrate an external validation lib: https://github.com/avelino/awesome-go#validation

return errors
}

// Apply the provided config on top of the current one, returning a new one. The provided config has priority.
func (c Config) Apply(cfg Config) Config {
c.Options = c.Options.Apply(cfg.Options)
if len(cfg.Out) > 0 {
Expand Down Expand Up @@ -146,11 +148,11 @@ func writeDiskConfig(fs afero.Fs, configPath string, conf Config) error {
return err
}

if err := fs.MkdirAll(filepath.Dir(configPath), 0755); err != nil {
if err := fs.MkdirAll(filepath.Dir(configPath), 0o755); err != nil {
return err
}

return afero.WriteFile(fs, configPath, data, 0644)
return afero.WriteFile(fs, configPath, data, 0o644)
}

// Reads configuration variables from the environment.
Expand Down
11 changes: 7 additions & 4 deletions cmd/config_consolidation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,8 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase {
},
{opts{cli: []string{"-u", "1", "-i", "6", "-d", "10s"}}, exp{}, func(t *testing.T, c Config) {
verifySharedIters(I(1), I(6))(t, c)
sharedIterConfig := c.Scenarios[lib.DefaultScenarioName].(executor.SharedIterationsConfig)
sharedIterConfig, ok := c.Scenarios[lib.DefaultScenarioName].(executor.SharedIterationsConfig)
require.True(t, ok)
assert.Equal(t, sharedIterConfig.MaxDuration.TimeDuration(), 10*time.Second)
}},
// This should get a validation error since VUs are more than the shared iterations
Expand Down Expand Up @@ -535,6 +536,7 @@ func runTestCase(
newFlagSet flagSetInit,
logHook *testutils.SimpleLogrusHook,
) {
t.Helper()
t.Logf("Test with opts=%#v and exp=%#v\n", testCase.options, testCase.expected)
output := testutils.NewTestOutput(t)
logrus.SetOutput(output)
Expand Down Expand Up @@ -595,8 +597,7 @@ func runTestCase(
}
require.NoError(t, err)

warnings := logHook.Drain()
if testCase.expected.logWarning {
if warnings := logHook.Drain(); testCase.expected.logWarning {
assert.NotEmpty(t, warnings)
} else {
assert.Empty(t, warnings)
Expand All @@ -614,6 +615,7 @@ func runTestCase(
}
}

//nolint:paralleltest // see comments in test
func TestConfigConsolidation(t *testing.T) {
// This test and its subtests shouldn't be ran in parallel, since they unfortunately have
// to mess with shared global objects (env vars, variables, the log, ... santa?)
Expand All @@ -623,14 +625,15 @@ func TestConfigConsolidation(t *testing.T) {
defer logrus.SetOutput(os.Stderr)

for tcNum, testCase := range getConfigConsolidationTestCases() {
tcNum, testCase := tcNum, testCase
flagSetInits := testCase.options.cliFlagSetInits
if flagSetInits == nil { // handle the most common case
flagSetInits = mostFlagSets()
}
for fsNum, flagSet := range flagSetInits {
// I want to paralelize this, but I cannot... due to global variables and other
// questionable architectural choices... :|
testCase, flagSet := testCase, flagSet
fsNum, flagSet := fsNum, flagSet
t.Run(
fmt.Sprintf("TestCase#%d_FlagSet#%d", tcNum, fsNum),
func(t *testing.T) { runTestCase(t, testCase, flagSet, &logHook) },
Expand Down
55 changes: 39 additions & 16 deletions cmd/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type testCmdTest struct {
}

func TestConfigCmd(t *testing.T) {
t.Parallel()
testdata := []testCmdData{
{
Name: "Out",
Expand All @@ -75,8 +76,10 @@ func TestConfigCmd(t *testing.T) {

for _, data := range testdata {
t.Run(data.Name, func(t *testing.T) {
t.Parallel()
for _, test := range data.Tests {
t.Run(`"`+test.Name+`"`, func(t *testing.T) {
t.Parallel()
fs := configFlagSet()
fs.AddFlagSet(optionFlagSet())
assert.NoError(t, fs.Parse(test.Args))
Expand All @@ -90,6 +93,7 @@ func TestConfigCmd(t *testing.T) {
}
}

//nolint:paralleltest // this user testutils.SetEnv
func TestConfigEnv(t *testing.T) {
testdata := map[struct{ Name, Key string }]map[string]func(Config){
{"Linger", "K6_LINGER"}: {
Expand Down Expand Up @@ -125,15 +129,19 @@ func TestConfigEnv(t *testing.T) {
}

func TestConfigApply(t *testing.T) {
t.Parallel()
t.Run("Linger", func(t *testing.T) {
t.Parallel()
conf := Config{}.Apply(Config{Linger: null.BoolFrom(true)})
assert.Equal(t, null.BoolFrom(true), conf.Linger)
})
t.Run("NoUsageReport", func(t *testing.T) {
t.Parallel()
conf := Config{}.Apply(Config{NoUsageReport: null.BoolFrom(true)})
assert.Equal(t, null.BoolFrom(true), conf.NoUsageReport)
})
t.Run("Out", func(t *testing.T) {
t.Parallel()
conf := Config{}.Apply(Config{Out: []string{"influxdb"}})
assert.Equal(t, []string{"influxdb"}, conf.Out)

Expand All @@ -152,30 +160,45 @@ func TestDeriveAndValidateConfig(t *testing.T) {
err string
}{
{"defaultOK", Config{}, true, ""},
{"defaultErr", Config{}, false,
"executor default: function 'default' not found in exports"},
{"nonDefaultOK", Config{Options: lib.Options{Scenarios: lib.ScenarioConfigs{
"per_vu_iters": executor.PerVUIterationsConfig{BaseConfig: executor.BaseConfig{
Name: "per_vu_iters", Type: "per-vu-iterations", Exec: null.StringFrom("nonDefault")},
VUs: null.IntFrom(1),
Iterations: null.IntFrom(1),
MaxDuration: types.NullDurationFrom(time.Second),
}}}}, true, "",
{
"defaultErr",
Config{},
false,
"executor default: function 'default' not found in exports",
},
{
"nonDefaultOK", Config{Options: lib.Options{Scenarios: lib.ScenarioConfigs{
"per_vu_iters": executor.PerVUIterationsConfig{
BaseConfig: executor.BaseConfig{
Name: "per_vu_iters", Type: "per-vu-iterations", Exec: null.StringFrom("nonDefault"),
},
VUs: null.IntFrom(1),
Iterations: null.IntFrom(1),
MaxDuration: types.NullDurationFrom(time.Second),
},
}}}, true, "",
},
{"nonDefaultErr", Config{Options: lib.Options{Scenarios: lib.ScenarioConfigs{
"per_vu_iters": executor.PerVUIterationsConfig{BaseConfig: executor.BaseConfig{
Name: "per_vu_iters", Type: "per-vu-iterations", Exec: null.StringFrom("nonDefaultErr")},
VUs: null.IntFrom(1),
Iterations: null.IntFrom(1),
MaxDuration: types.NullDurationFrom(time.Second),
}}}}, false,
{
"nonDefaultErr",
Config{Options: lib.Options{Scenarios: lib.ScenarioConfigs{
"per_vu_iters": executor.PerVUIterationsConfig{
BaseConfig: executor.BaseConfig{
Name: "per_vu_iters", Type: "per-vu-iterations", Exec: null.StringFrom("nonDefaultErr"),
},
VUs: null.IntFrom(1),
Iterations: null.IntFrom(1),
MaxDuration: types.NullDurationFrom(time.Second),
},
}}},
false,
"executor per_vu_iters: function 'nonDefaultErr' not found in exports",
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
_, err := deriveAndValidateConfig(tc.conf,
func(_ string) bool { return tc.isExec })
if tc.err != "" {
Expand Down
10 changes: 5 additions & 5 deletions cmd/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,10 @@ func getConvertCmd() *cobra.Command {
convertCmd.Flags().StringSliceVarP(&skip, "skip", "", []string{}, "skip requests from the given domains")
convertCmd.Flags().UintVarP(&threshold, "batch-threshold", "", 500, "batch request idle time threshold (see example)")
convertCmd.Flags().BoolVarP(&nobatch, "no-batch", "", false, "don't generate batch calls")
convertCmd.Flags().BoolVarP(&enableChecks, "enable-status-code-checks", "", false, "add a status code check for each HTTP response")
convertCmd.Flags().BoolVarP(&returnOnFailedCheck, "return-on-failed-check", "", false, "return from iteration if we get an unexpected response status code")
convertCmd.Flags().BoolVarP(&correlate, "correlate", "", false, "detect values in responses being used in subsequent requests and try adapt the script accordingly (only redirects and JSON values for now)")
convertCmd.Flags().UintVarP(&minSleep, "min-sleep", "", 20, "the minimum amount of seconds to sleep after each iteration")
convertCmd.Flags().UintVarP(&maxSleep, "max-sleep", "", 40, "the maximum amount of seconds to sleep after each iteration")
convertCmd.Flags().BoolVarP(&enableChecks, "enable-status-code-checks", "", false, "add a status code check for each HTTP response") //nolint:lll
convertCmd.Flags().BoolVarP(&returnOnFailedCheck, "return-on-failed-check", "", false, "return from iteration if we get an unexpected response status code") //nolint:lll
convertCmd.Flags().BoolVarP(&correlate, "correlate", "", false, "detect values in responses being used in subsequent requests and try adapt the script accordingly (only redirects and JSON values for now)") //nolint:lll
convertCmd.Flags().UintVarP(&minSleep, "min-sleep", "", 20, "the minimum amount of seconds to sleep after each iteration") //nolint:lll
convertCmd.Flags().UintVarP(&maxSleep, "max-sleep", "", 40, "the maximum amount of seconds to sleep after each iteration") //nolint:lll
return convertCmd
}
1 change: 1 addition & 0 deletions cmd/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export default function() {
}
`

//nolint:paralleltest // a lot of global variables, hopefully convert will just be dropped
func TestIntegrationConvertCmd(t *testing.T) {
t.Run("Correlate", func(t *testing.T) {
harFile, err := filepath.Abs("correlate.har")
Expand Down
2 changes: 1 addition & 1 deletion cmd/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func getInspectCmd(logger *logrus.Logger) *cobra.Command {
if err != nil {
return err
}
fmt.Println(string(data))
fmt.Println(string(data)) //nolint:forbidigo // yes we want to just print it

return nil
},
Expand Down
12 changes: 7 additions & 5 deletions cmd/login_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
"go.k6.io/k6/ui"
)

//nolint:funlen
//nolint:funlen,gocognit
func getLoginCloudCommand(logger logrus.FieldLogger) *cobra.Command {
// loginCloudCommand represents the 'login cloud' command
loginCloudCommand := &cobra.Command{
Expand Down Expand Up @@ -111,12 +111,13 @@ This will set the default token used when just "k6 run -o cloud" is passed.`,
if !term.IsTerminal(int(syscall.Stdin)) { // nolint: unconvert
logger.Warn("Stdin is not a terminal, falling back to plain text input")
}
vals, err := form.Run(os.Stdin, stdout)
var vals map[string]string
vals, err = form.Run(os.Stdin, stdout)
if err != nil {
return err
}
email := vals["Email"].(string)
password := vals["Password"].(string)
email := vals["Email"]
password := vals["Password"]

client := cloudapi.NewClient(
logger,
Expand All @@ -125,7 +126,8 @@ This will set the default token used when just "k6 run -o cloud" is passed.`,
consts.Version,
consolidatedCurrentConfig.Timeout.TimeDuration())

res, err := client.Login(email, password)
var res *cloudapi.LoginResponse
res, err = client.Login(email, password)
if err != nil {
return err
}
Expand Down
9 changes: 5 additions & 4 deletions cmd/login_influxdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"go.k6.io/k6/ui"
)

//nolint:funlen
func getLoginInfluxDBCommand(logger logrus.FieldLogger) *cobra.Command {
// loginInfluxDBCommand represents the 'login influxdb' command
loginInfluxDBCommand := &cobra.Command{
Expand Down Expand Up @@ -100,10 +101,10 @@ This will set the default server used when just "-o influxdb" is passed.`,
return err
}

conf.Addr = null.StringFrom(vals["Addr"].(string))
conf.DB = null.StringFrom(vals["DB"].(string))
conf.Username = null.StringFrom(vals["Username"].(string))
conf.Password = null.StringFrom(vals["Password"].(string))
conf.Addr = null.StringFrom(vals["Addr"])
conf.DB = null.StringFrom(vals["DB"])
conf.Username = null.StringFrom(vals["Username"])
conf.Password = null.StringFrom(vals["Password"])

client, err := influxdb.MakeClient(conf)
if err != nil {
Expand Down
Loading

0 comments on commit 1ed7af9

Please sign in to comment.