From 6d19e615a3526704dcdf99c0e248d6812488d89a Mon Sep 17 00:00:00 2001 From: Nedyalko Andreev Date: Tue, 12 Mar 2019 15:13:41 +0200 Subject: [PATCH] Improve config consolidation, default values and tests --- cmd/config.go | 4 +- cmd/config_consolidation_test.go | 66 +++++++++++++++----------- lib/scheduler/base_config.go | 4 +- lib/scheduler/constant_arrival_rate.go | 2 +- lib/scheduler/constant_looping_vus.go | 11 +++-- lib/scheduler/schedulers_test.go | 4 +- 6 files changed, 52 insertions(+), 39 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 31e28a53d35..ba2e696624d 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -194,7 +194,7 @@ func buildExecutionConfig(conf Config) (Config, error) { log.Warnf("Specifying both duration and iterations is deprecated and won't be supported in the future k6 versions") } - if conf.Stages != nil { + if len(conf.Stages) > 0 { // stages isn't nil (not set) and isn't explicitly set to empty //TODO: make this an executionConflictConfigError in the next version log.Warnf("Specifying both duration and stages is deprecated and won't be supported in the future k6 versions") } @@ -214,7 +214,7 @@ func buildExecutionConfig(conf Config) (Config, error) { result.Execution = scheduler.ConfigMap{lib.DefaultSchedulerName: ds} } - case conf.Stages != nil: + case len(conf.Stages) > 0: // stages isn't nil (not set) and isn't explicitly set to empty if conf.Iterations.Valid { //TODO: make this an executionConflictConfigError in the next version log.Warnf("Specifying both iterations and stages is deprecated and won't be supported in the future k6 versions") diff --git a/cmd/config_consolidation_test.go b/cmd/config_consolidation_test.go index 82a619ce5f0..5f5451e4f52 100644 --- a/cmd/config_consolidation_test.go +++ b/cmd/config_consolidation_test.go @@ -84,21 +84,21 @@ func verifyOneIterPerOneVU(t *testing.T, c Config) { assert.Equal(t, null.NewInt(1, false), perVuIters.VUs) } -func verifySharedIters(vus, iters int64) func(t *testing.T, c Config) { +func verifySharedIters(vus, iters null.Int) func(t *testing.T, c Config) { return func(t *testing.T, c Config) { sched := c.Execution[lib.DefaultSchedulerName] require.NotEmpty(t, sched) require.IsType(t, scheduler.SharedIteationsConfig{}, sched) sharedIterConfig, ok := sched.(scheduler.SharedIteationsConfig) require.True(t, ok) - assert.Equal(t, null.NewInt(vus, true), sharedIterConfig.VUs) - assert.Equal(t, null.NewInt(iters, true), sharedIterConfig.Iterations) - assert.Equal(t, null.NewInt(vus, true), c.VUs) - assert.Equal(t, null.NewInt(iters, true), c.Iterations) + assert.Equal(t, vus, sharedIterConfig.VUs) + assert.Equal(t, iters, sharedIterConfig.Iterations) + assert.Equal(t, vus, c.VUs) + assert.Equal(t, iters, c.Iterations) } } -func verifyConstLoopingVUs(vus int64, duration time.Duration) func(t *testing.T, c Config) { +func verifyConstLoopingVUs(vus null.Int, duration time.Duration) func(t *testing.T, c Config) { return func(t *testing.T, c Config) { sched := c.Execution[lib.DefaultSchedulerName] require.NotEmpty(t, sched) @@ -106,14 +106,14 @@ func verifyConstLoopingVUs(vus int64, duration time.Duration) func(t *testing.T, clvc, ok := sched.(scheduler.ConstantLoopingVUsConfig) require.True(t, ok) assert.Equal(t, null.NewBool(true, false), clvc.Interruptible) - assert.Equal(t, null.NewInt(vus, true), clvc.VUs) + assert.Equal(t, vus, clvc.VUs) assert.Equal(t, types.NullDurationFrom(duration), clvc.Duration) - assert.Equal(t, null.NewInt(vus, true), c.VUs) + assert.Equal(t, vus, c.VUs) assert.Equal(t, types.NullDurationFrom(duration), c.Duration) } } -func verifyVarLoopingVUs(startVus int64, startVUsSet bool, stages []scheduler.Stage) func(t *testing.T, c Config) { +func verifyVarLoopingVUs(startVus null.Int, stages []scheduler.Stage) func(t *testing.T, c Config) { return func(t *testing.T, c Config) { sched := c.Execution[lib.DefaultSchedulerName] require.NotEmpty(t, sched) @@ -121,8 +121,8 @@ func verifyVarLoopingVUs(startVus int64, startVUsSet bool, stages []scheduler.St clvc, ok := sched.(scheduler.VariableLoopingVUsConfig) require.True(t, ok) assert.Equal(t, null.NewBool(true, false), clvc.Interruptible) - assert.Equal(t, null.NewInt(startVus, startVUsSet), clvc.StartVUs) - assert.Equal(t, null.NewInt(startVus, startVUsSet), c.VUs) + assert.Equal(t, startVus, clvc.StartVUs) + assert.Equal(t, startVus, c.VUs) assert.Equal(t, stages, clvc.Stages) assert.Len(t, c.Stages, len(stages)) for i, s := range stages { @@ -241,6 +241,7 @@ type configConsolidationTestCase struct { } func getConfigConsolidationTestCases() []configConsolidationTestCase { + I := null.IntFrom // shortcut for "Valid" (i.e. user-specified) ints // This is a function, because some of these test cases actually need for the init() functions // to be executed, since they depend on defaultConfigFilePath return []configConsolidationTestCase{ @@ -253,20 +254,21 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase { {opts{cli: []string{"--execution", ""}}, exp{cliParseError: true}, nil}, {opts{cli: []string{"--stage", "10:20s"}}, exp{cliReadError: true}, nil}, // Check if CLI shortcuts generate correct execution values - {opts{cli: []string{"--vus", "1", "--iterations", "5"}}, exp{}, verifySharedIters(1, 5)}, - {opts{cli: []string{"-u", "2", "-i", "6"}}, exp{}, verifySharedIters(2, 6)}, - {opts{cli: []string{"-u", "3", "-d", "30s"}}, exp{}, verifyConstLoopingVUs(3, 30*time.Second)}, - {opts{cli: []string{"-u", "4", "--duration", "60s"}}, exp{}, verifyConstLoopingVUs(4, 1*time.Minute)}, + {opts{cli: []string{"--vus", "1", "--iterations", "5"}}, exp{}, verifySharedIters(I(1), I(5))}, + {opts{cli: []string{"-u", "2", "-i", "6"}}, exp{}, verifySharedIters(I(2), I(6))}, + {opts{cli: []string{"-d", "123s"}}, exp{}, verifyConstLoopingVUs(null.NewInt(1, false), 123*time.Second)}, + {opts{cli: []string{"-u", "3", "-d", "30s"}}, exp{}, verifyConstLoopingVUs(I(3), 30*time.Second)}, + {opts{cli: []string{"-u", "4", "--duration", "60s"}}, exp{}, verifyConstLoopingVUs(I(4), 1*time.Minute)}, { opts{cli: []string{"--stage", "20s:10", "-s", "3m:5"}}, exp{}, - verifyVarLoopingVUs(1, false, buildStages(20, 10, 180, 5)), + verifyVarLoopingVUs(null.NewInt(1, false), buildStages(20, 10, 180, 5)), }, { opts{cli: []string{"-s", "1m6s:5", "--vus", "10"}}, exp{}, - verifyVarLoopingVUs(10, true, buildStages(66, 5)), + verifyVarLoopingVUs(null.NewInt(10, true), buildStages(66, 5)), }, // This should get a validation error since VUs are more than the shared iterations - {opts{cli: []string{"--vus", "10", "-i", "6"}}, exp{validationErrors: true}, verifySharedIters(10, 6)}, + {opts{cli: []string{"--vus", "10", "-i", "6"}}, exp{validationErrors: true}, verifySharedIters(I(10), I(6))}, // These should emit a warning //TODO: in next version, those should be an error {opts{cli: []string{"-u", "1", "-i", "6", "-d", "10s"}}, exp{logWarning: true}, nil}, @@ -282,18 +284,18 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase { }, {opts{fs: defaultConfig(`{"execution": {}}`)}, exp{logWarning: true}, verifyOneIterPerOneVU}, // Test if environment variable shortcuts are working as expected - {opts{env: []string{"K6_VUS=5", "K6_ITERATIONS=15"}}, exp{}, verifySharedIters(5, 15)}, - {opts{env: []string{"K6_VUS=10", "K6_DURATION=20s"}}, exp{}, verifyConstLoopingVUs(10, 20*time.Second)}, + {opts{env: []string{"K6_VUS=5", "K6_ITERATIONS=15"}}, exp{}, verifySharedIters(I(5), I(15))}, + {opts{env: []string{"K6_VUS=10", "K6_DURATION=20s"}}, exp{}, verifyConstLoopingVUs(I(10), 20*time.Second)}, { opts{env: []string{"K6_STAGES=2m30s:11,1h1m:100"}}, exp{}, - verifyVarLoopingVUs(1, false, buildStages(150, 11, 3660, 100)), + verifyVarLoopingVUs(null.NewInt(1, false), buildStages(150, 11, 3660, 100)), }, { opts{env: []string{"K6_STAGES=100s:100,0m30s:0", "K6_VUS=0"}}, exp{}, - verifyVarLoopingVUs(0, true, buildStages(100, 100, 30, 0)), + verifyVarLoopingVUs(null.NewInt(0, true), buildStages(100, 100, 30, 0)), }, // Test if JSON configs work as expected - {opts{fs: defaultConfig(`{"iterations": 77, "vus": 7}`)}, exp{}, verifySharedIters(7, 77)}, + {opts{fs: defaultConfig(`{"iterations": 77, "vus": 7}`)}, exp{}, verifySharedIters(I(7), I(77))}, {opts{fs: defaultConfig(`wrong-json`)}, exp{consolidationError: true}, nil}, {opts{fs: getFS(nil), cli: []string{"--config", "/my/config.file"}}, exp{consolidationError: true}, nil}, @@ -302,7 +304,15 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase { opts{ fs: getFS([]file{{"/my/config.file", `{"vus": 8, "duration": "2m"}`}}), cli: []string{"--config", "/my/config.file"}, - }, exp{}, verifyConstLoopingVUs(8, 120*time.Second), + }, exp{}, verifyConstLoopingVUs(I(8), 120*time.Second), + }, + { + opts{ + fs: defaultConfig(`{"stages": [{"duration": "20s", "target": 20}], "vus": 10}`), + env: []string{"K6_DURATION=15s"}, + cli: []string{"--stage", ""}, + }, + exp{}, verifyConstLoopingVUs(I(10), 15*time.Second), }, { opts{ @@ -310,7 +320,7 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase { cli: []string{"--iterations", "5"}, }, //TODO: this shouldn't be a warning in the next version, but the result will be different - exp{logWarning: true}, verifyConstLoopingVUs(5, 50*time.Second), + exp{logWarning: true}, verifyConstLoopingVUs(I(5), 50*time.Second), }, { opts{ @@ -318,7 +328,7 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase { runner: &lib.Options{VUs: null.IntFrom(5)}, }, exp{}, - verifyVarLoopingVUs(5, true, buildStages(20, 10)), + verifyVarLoopingVUs(null.NewInt(5, true), buildStages(20, 10)), }, { opts{ @@ -327,7 +337,7 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase { env: []string{"K6_VUS=15", "K6_ITERATIONS=15"}, }, exp{logWarning: true}, //TODO: this won't be a warning in the next version, but the result will be different - verifyVarLoopingVUs(15, true, buildStages(20, 10)), + verifyVarLoopingVUs(null.NewInt(15, true), buildStages(20, 10)), }, { opts{ @@ -337,7 +347,7 @@ func getConfigConsolidationTestCases() []configConsolidationTestCase { cli: []string{"--stage", "44s:44", "-s", "55s:55"}, }, exp{}, - verifyVarLoopingVUs(33, true, buildStages(44, 44, 55, 55)), + verifyVarLoopingVUs(null.NewInt(33, true), buildStages(44, 44, 55, 55)), }, //TODO: test the future full overwriting of the duration/iterations/stages/execution options diff --git a/lib/scheduler/base_config.go b/lib/scheduler/base_config.go index efba8cb1c3a..73a4c22a4b4 100644 --- a/lib/scheduler/base_config.go +++ b/lib/scheduler/base_config.go @@ -77,8 +77,8 @@ func (bc BaseConfig) Validate() (errors []error) { errors = append(errors, fmt.Errorf("exec value cannot be empty")) } // The actually reasonable checks: - if bc.StartTime.Valid && bc.StartTime.Duration < 0 { - errors = append(errors, fmt.Errorf("scheduler start time should be positive")) + if bc.StartTime.Duration < 0 { + errors = append(errors, fmt.Errorf("scheduler start time can't be negative")) } iterTimeout := time.Duration(bc.IterationTimeout.Duration) if iterTimeout < 0 || iterTimeout > maxIterationTimeout { diff --git a/lib/scheduler/constant_arrival_rate.go b/lib/scheduler/constant_arrival_rate.go index 0234c3a3465..ed53297e924 100644 --- a/lib/scheduler/constant_arrival_rate.go +++ b/lib/scheduler/constant_arrival_rate.go @@ -69,7 +69,7 @@ func (carc ConstantArrivalRateConfig) Validate() []error { if !carc.Rate.Valid { errors = append(errors, fmt.Errorf("the iteration rate isn't specified")) } else if carc.Rate.Int64 <= 0 { - errors = append(errors, fmt.Errorf("the iteration rate should be positive")) + errors = append(errors, fmt.Errorf("the iteration rate should be more than 0")) } if time.Duration(carc.TimeUnit.Duration) <= 0 { diff --git a/lib/scheduler/constant_looping_vus.go b/lib/scheduler/constant_looping_vus.go index dd67e9cdf17..a6293cb1b17 100644 --- a/lib/scheduler/constant_looping_vus.go +++ b/lib/scheduler/constant_looping_vus.go @@ -51,7 +51,10 @@ type ConstantLoopingVUsConfig struct { // NewConstantLoopingVUsConfig returns a ConstantLoopingVUsConfig with default values func NewConstantLoopingVUsConfig(name string) ConstantLoopingVUsConfig { - return ConstantLoopingVUsConfig{BaseConfig: NewBaseConfig(name, constantLoopingVUsType, false)} + return ConstantLoopingVUsConfig{ + BaseConfig: NewBaseConfig(name, constantLoopingVUsType, false), + VUs: null.NewInt(1, false), + } } // Make sure we implement the Config interface @@ -60,10 +63,8 @@ var _ Config = &ConstantLoopingVUsConfig{} // Validate makes sure all options are configured and valid func (lcv ConstantLoopingVUsConfig) Validate() []error { errors := lcv.BaseConfig.Validate() - if !lcv.VUs.Valid { - errors = append(errors, fmt.Errorf("the number of VUs isn't specified")) - } else if lcv.VUs.Int64 < 0 { - errors = append(errors, fmt.Errorf("the number of VUs shouldn't be negative")) + if lcv.VUs.Int64 <= 0 { + errors = append(errors, fmt.Errorf("the number of VUs should be more than 0")) } if !lcv.Duration.Valid { diff --git a/lib/scheduler/schedulers_test.go b/lib/scheduler/schedulers_test.go index cbecca214f4..11c07453f64 100644 --- a/lib/scheduler/schedulers_test.go +++ b/lib/scheduler/schedulers_test.go @@ -76,10 +76,12 @@ var configMapTestCases = []configMapTestCase{ assert.Equal(t, int64(10), cm["someKey"].GetMaxVUs()) assert.Empty(t, cm["someKey"].Validate()) }}, + {`{"aname": {"type": "constant-looping-vus", "duration": "60s"}}`, false, false, nil}, {`{"": {"type": "constant-looping-vus", "vus": 10, "duration": "60s"}}`, false, true, nil}, {`{"aname": {"type": "constant-looping-vus"}}`, false, true, nil}, + {`{"aname": {"type": "constant-looping-vus", "vus": 0.5}}`, true, false, nil}, {`{"aname": {"type": "constant-looping-vus", "vus": 10}}`, false, true, nil}, - {`{"aname": {"type": "constant-looping-vus", "duration": "60s"}}`, false, true, nil}, + {`{"aname": {"type": "constant-looping-vus", "vus": 0, "duration": "60s"}}`, false, true, nil}, {`{"aname": {"type": "constant-looping-vus", "vus": -1, "duration": "60s"}}`, false, true, nil}, {`{"aname": {"type": "constant-looping-vus", "vus": 10, "duration": "0s"}}`, false, true, nil}, {`{"aname": {"type": "constant-looping-vus", "vus": 10, "duration": "10s", "startTime": "-10s"}}`, false, true, nil},