Skip to content

Commit

Permalink
Better parsing of the csv output's arguments
Browse files Browse the repository at this point in the history
1. don't use external libraries that we want to remove to do it
2. error out on non existing keys in the argument
3. don't print ot stdout if `file_name` is not provided in the argument
but there is an argument
  • Loading branch information
mstoykov committed Nov 12, 2019
1 parent 030d37d commit 9c73438
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 46 deletions.
44 changes: 17 additions & 27 deletions stats/csv/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@
package csv

import (
"fmt"
"strings"
"time"

"github.com/kubernetes/helm/pkg/strvals"
"github.com/loadimpact/k6/lib/types"
"github.com/mitchellh/mapstructure"
"gopkg.in/guregu/null.v3"
)

Expand All @@ -37,13 +36,6 @@ type Config struct {
SaveInterval types.NullDuration `json:"save_interval" envconfig:"K6_CSV_SAVE_INTERVAL"`
}

// config is a duplicate of ConfigFields as we can not mapstructure.Decode into
// null types so we duplicate the struct with primitive types to Decode into
type config struct {
FileName string `json:"file_name" mapstructure:"file_name" envconfig:"K6_CSV_FILENAME"`
SaveInterval string `json:"save_interval" mapstructure:"save_interval" envconfig:"K6_CSV_SAVE_INTERVAL"`
}

// NewConfig creates a new Config instance with default values for some fields.
func NewConfig() Config {
return Config{
Expand Down Expand Up @@ -73,26 +65,24 @@ func ParseArg(arg string) (Config, error) {
return c, nil
}

params, err := strvals.Parse(arg)

if err != nil {
return c, err
}

if v, ok := params["save_interval"].(string); ok {
err = c.SaveInterval.UnmarshalText([]byte(v))
if err != nil {
return c, err
pairs := strings.Split(arg, ",")
for _, pair := range pairs {
r := strings.SplitN(pair, "=", 2)
if len(r) != 2 {
return c, fmt.Errorf("couldn't parse %q as argument for csv output", arg)
}
switch r[0] {
case "save_interval":
err := c.SaveInterval.UnmarshalText([]byte(r[1]))
if err != nil {
return c, err
}
case "file_name":
c.FileName = null.StringFrom(r[1])
default:
return c, fmt.Errorf("unknown key %q as argument for csv output", r[0])
}
}

var cfg config
err = mapstructure.Decode(params, &cfg)
if err != nil {
return c, err
}

c.FileName = null.StringFrom(cfg.FileName)

return c, nil
}
52 changes: 33 additions & 19 deletions stats/csv/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,32 +75,46 @@ func TestApply(t *testing.T) {
}

func TestParseArg(t *testing.T) {
args := []string{
"test_file.csv",
"file_name=test.csv,save_interval=5s",
}

expected := []Config{
{
FileName: null.StringFrom("test_file.csv"),
SaveInterval: types.NullDurationFrom(1 * time.Second),
cases := map[string]struct {
config Config
expectedErr bool
}{
"test_file.csv": {
config: Config{
FileName: null.StringFrom("test_file.csv"),
SaveInterval: types.NullDurationFrom(1 * time.Second),
},
},
{
FileName: null.StringFrom("test.csv"),
SaveInterval: types.NullDurationFrom(5 * time.Second),
"save_interval=5s": {
config: Config{
SaveInterval: types.NullDurationFrom(5 * time.Second),
},
},
"file_name=test.csv,save_interval=5s": {
config: Config{
FileName: null.StringFrom("test.csv"),
SaveInterval: types.NullDurationFrom(5 * time.Second),
},
},
"filename=test.csv,save_interval=5s": {
expectedErr: true,
},
}

for i := range args {
arg := args[i]
expected := expected[i]
for arg, testCase := range cases {
arg := arg
testCase := testCase

t.Run(expected.FileName.String+"_"+expected.SaveInterval.String(), func(t *testing.T) {
t.Run(arg, func(t *testing.T) {
config, err := ParseArg(arg)

assert.Nil(t, err)
assert.Equal(t, expected.FileName.String, config.FileName.String)
assert.Equal(t, expected.SaveInterval.String(), config.SaveInterval.String())
if testCase.expectedErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
assert.Equal(t, testCase.config.FileName.String, config.FileName.String)
assert.Equal(t, testCase.config.SaveInterval.String(), config.SaveInterval.String())
})
}
}

0 comments on commit 9c73438

Please sign in to comment.