From c0fb7f269fa9a697431f3711686aaafd9a08cd34 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Tue, 18 Oct 2022 09:19:37 +0200 Subject: [PATCH 01/11] deps: upgrade benchttp/engine to v0.2.0 Update the config parsing and overriding logics using the new engine API (using ConfigRepresentation.Unmarshal, removing Config.Override) internal/configfile: - Bind: change signature so it stores into a configparse.Representation instead of runner.Config directly (this allows to call repr.Unmarshal(&cfg) with the file config as base when needed rather than having to bookkeep fields that are set) - Parse: use repr.Unmarshal and refactor internals to use unmarshal-like signatures - Find: make it handle default paths cmd/benchttp: - adapt to new engine and configfile API - refactor --- cmd/benchttp/run.go | 85 +++++---------- internal/configfile/error.go | 4 +- internal/configfile/find.go | 14 ++- internal/configfile/find_test.go | 14 ++- internal/configfile/parse.go | 30 +++--- internal/configfile/parse_test.go | 52 +++++---- internal/configflag/bind.go | 173 +++++++++++++++++++----------- internal/configflag/bind_test.go | 39 ++++--- internal/configflag/body.go | 50 --------- internal/configflag/header.go | 30 ------ internal/configflag/url.go | 29 ----- internal/configflag/which.go | 19 ---- internal/configflag/which_test.go | 52 --------- 13 files changed, 230 insertions(+), 361 deletions(-) delete mode 100644 internal/configflag/body.go delete mode 100644 internal/configflag/header.go delete mode 100644 internal/configflag/url.go delete mode 100644 internal/configflag/which.go delete mode 100644 internal/configflag/which_test.go diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index 066a4ac..5edb37b 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -8,6 +8,7 @@ import ( "io" "os" + "github.com/benchttp/engine/configparse" "github.com/benchttp/engine/runner" "github.com/benchttp/cli/internal/configfile" @@ -27,33 +28,24 @@ type cmdRun struct { // silent is the parsed value for flag -silent silent bool - // config is the runner config resulting from parsing CLI flags. - config runner.Config -} - -// init initializes cmdRun with default values. -func (cmd *cmdRun) init() { - cmd.config = runner.DefaultConfig() - cmd.configFile = configfile.Find([]string{ - "./.benchttp.yml", - "./.benchttp.yaml", - "./.benchttp.json", - }) + // configRepr is the runner config resulting from config flag values + configRepr configparse.Representation } // execute runs the benchttp runner: it parses CLI flags, loads config // from config file and parsed flags, then runs the benchmark and outputs // it according to the config. func (cmd *cmdRun) execute(args []string) error { - cmd.init() + if err := cmd.parseArgs(args); err != nil { + return err + } - // Generate merged config (default < config file < CLI flags) - cfg, err := cmd.makeConfig(args) + config, err := cmd.buildConfig() if err != nil { return err } - report, err := runBenchmark(cfg, cmd.silent) + report, err := runBenchmark(config, cmd.silent) if err != nil { return err } @@ -61,60 +53,31 @@ func (cmd *cmdRun) execute(args []string) error { return renderReport(os.Stdout, report, cmd.silent) } -// parseArgs parses input args as config fields and returns -// a slice of fields that were set by the user. -func (cmd *cmdRun) parseArgs(args []string) []string { - // skip parsing if no flags are provided - if len(args) == 0 { - return []string{} - } - - // config file path - cmd.flagset.StringVar(&cmd.configFile, - "configFile", - cmd.configFile, - "Config file path", - ) - - // silent mode - cmd.flagset.BoolVar(&cmd.silent, - "silent", - false, - "Silent mode", - ) - - // attach config options flags to the flagset - // and bind their value to the config struct - configflag.Bind(cmd.flagset, &cmd.config) - - cmd.flagset.Parse(args) //nolint:errcheck // never occurs due to flag.ExitOnError - - return configflag.Which(cmd.flagset) +func (cmd *cmdRun) parseArgs(args []string) error { + cmd.flagset.StringVar(&cmd.configFile, "configFile", configfile.Find(), "Config file path") + cmd.flagset.BoolVar(&cmd.silent, "silent", false, "Silent mode") + configflag.Bind(cmd.flagset, &cmd.configRepr) + return cmd.flagset.Parse(args) } -// makeConfig returns a runner.ConfigGlobal initialized with config file -// options if found, overridden with CLI options listed in fields -// slice param. -func (cmd *cmdRun) makeConfig(args []string) (cfg runner.Config, err error) { - // Set CLI config from flags and retrieve fields that were set - fields := cmd.parseArgs(args) - - // configFile not set and default ones not found: - // skip the merge and return the cli config - if cmd.configFile == "" { - return cmd.config, cmd.config.Validate() - } +func (cmd *cmdRun) buildConfig() (runner.Config, error) { + // start with default config as base + config := runner.DefaultConfig() - fileConfig, err := configfile.Parse(cmd.configFile) + // override with config file values + err := configfile.Parse(cmd.configFile, &config) if err != nil && !errors.Is(err, configfile.ErrFileNotFound) { // config file is not mandatory: discard ErrFileNotFound. // other errors are critical - return + return config, err } - mergedConfig := cmd.config.WithFields(fields...).Override(fileConfig) + // override with CLI flags values + if err := cmd.configRepr.Unmarshal(&config); err != nil { + return config, err + } - return mergedConfig, mergedConfig.Validate() + return config, nil } func onRecordingProgress(silent bool) func(runner.RecordingProgress) { diff --git a/internal/configfile/error.go b/internal/configfile/error.go index 61fdcf6..c33a7d2 100644 --- a/internal/configfile/error.go +++ b/internal/configfile/error.go @@ -16,10 +16,10 @@ var ( // ErrFileExt signals an unsupported extension for the config file. ErrFileExt = errors.New("invalid extension") - // ErrParse signals an error parsing a retrieved config file. + // ErrFileParse signals an error parsing a retrieved config file. // It is returned if it contains an unexpected field or an unexpected // value for a field. - ErrParse = errors.New("parsing error: invalid config file") + ErrFileParse = errors.New("parsing error: invalid config file") // ErrCircularExtends signals a circular reference in the config file. ErrCircularExtends = errors.New("circular reference detected") diff --git a/internal/configfile/find.go b/internal/configfile/find.go index 755e39a..797052e 100644 --- a/internal/configfile/find.go +++ b/internal/configfile/find.go @@ -2,10 +2,20 @@ package configfile import "os" +var DefaultPaths = []string{ + "./.benchttp.yml", + "./.benchttp.yaml", + "./.benchttp.json", +} + // Find returns the first name tham matches a file path. +// If input paths is empty, it uses DefaultPaths. // If no match is found, it returns an empty string. -func Find(names []string) string { - for _, path := range names { +func Find(paths ...string) string { + if len(paths) == 0 { + paths = DefaultPaths + } + for _, path := range paths { if _, err := os.Stat(path); err == nil { // err IS nil: file exists return path } diff --git a/internal/configfile/find_test.go b/internal/configfile/find_test.go index df6490d..08d8f38 100644 --- a/internal/configfile/find_test.go +++ b/internal/configfile/find_test.go @@ -13,10 +13,18 @@ var ( ) func TestFind(t *testing.T) { - t.Run("return first existing file", func(t *testing.T) { + t.Run("return first existing file form input", func(t *testing.T) { files := []string{badFile, goodFileYML, goodFileJSON} - if got := configfile.Find(files); got != goodFileYML { + if got := configfile.Find(files...); got != goodFileYML { + t.Errorf("did not retrieve good file: exp %s, got %s", goodFileYML, got) + } + }) + + t.Run("return first existing file form defaults", func(t *testing.T) { + configfile.DefaultPaths = []string{badFile, goodFileYML, goodFileJSON} + + if got := configfile.Find(); got != goodFileYML { t.Errorf("did not retrieve good file: exp %s, got %s", goodFileYML, got) } }) @@ -24,7 +32,7 @@ func TestFind(t *testing.T) { t.Run("return empty string when no match", func(t *testing.T) { files := []string{badFile} - if got := configfile.Find(files); got != "" { + if got := configfile.Find(files...); got != "" { t.Errorf("retrieved unexpected file: %s", got) } }) diff --git a/internal/configfile/parse.go b/internal/configfile/parse.go index 5217600..078da3e 100644 --- a/internal/configfile/parse.go +++ b/internal/configfile/parse.go @@ -11,15 +11,16 @@ import ( "github.com/benchttp/cli/internal/errorutil" ) -// Parse parses a benchttp runner config file into a runner.ConfigGlobal -// and returns it or the first non-nil error occurring in the process, -// which can be any of the values declared in the package. -func Parse(filename string) (cfg runner.Config, err error) { - uconfs, err := parseFileRecursive(filename, []configparse.Representation{}, set{}) +// Parse parses given filename as a benchttp runner config file +// into a runner.Config and stores the retrieved values into *dst. +// It returns the first error occurring in the process, which can be +// any of the values declared in the package. +func Parse(filename string, cfg *runner.Config) (err error) { + reprs, err := parseFileRecursive(filename, []configparse.Representation{}, set{}) if err != nil { return } - return parseAndMergeConfigs(uconfs) + return parseAndMergeConfigs(reprs, cfg) } // set is a collection of unique string values. @@ -85,7 +86,7 @@ func parseFile(filename string) (repr configparse.Representation, err error) { } if err = parser.Parse(b, &repr); err != nil { - return repr, errorutil.WithDetails(ErrParse, filename, err) + return repr, errorutil.WithDetails(ErrFileParse, filename, err) } return repr, nil @@ -95,25 +96,20 @@ func parseFile(filename string) (repr configparse.Representation, err error) { // as runner.ConfigGlobal and merging them into a single one. // It returns the merged result or the first non-nil error occurring in the // process. -func parseAndMergeConfigs(reprs []configparse.Representation) (cfg runner.Config, err error) { +func parseAndMergeConfigs(reprs []configparse.Representation, dst *runner.Config) error { if len(reprs) == 0 { // supposedly catched upstream, should not occur - return cfg, errors.New( + return errors.New( "an unacceptable error occurred parsing the config file, " + "please visit https://github.com/benchttp/runner/issues/new " + "and insult us properly", ) } - cfg = runner.DefaultConfig() - for i := len(reprs) - 1; i >= 0; i-- { - repr := reprs[i] - currentConfig, err := configparse.ParseRepresentation(repr) - if err != nil { - return cfg, errorutil.WithDetails(ErrParse, err) + if err := reprs[i].Unmarshal(dst); err != nil { + return errorutil.WithDetails(ErrFileParse, err) } - cfg = currentConfig.Override(cfg) } - return cfg, nil + return nil } diff --git a/internal/configfile/parse_test.go b/internal/configfile/parse_test.go index 06f7de2..978743a 100644 --- a/internal/configfile/parse_test.go +++ b/internal/configfile/parse_test.go @@ -47,12 +47,12 @@ func TestParse(t *testing.T) { { label: "yaml invalid fields", path: configPath("invalid/badfields.yml"), - expErr: configfile.ErrParse, + expErr: configfile.ErrFileParse, }, { label: "json invalid fields", path: configPath("invalid/badfields.json"), - expErr: configfile.ErrParse, + expErr: configfile.ErrFileParse, }, { label: "self reference", @@ -68,7 +68,8 @@ func TestParse(t *testing.T) { for _, tc := range testcases { t.Run(tc.label, func(t *testing.T) { - gotCfg, gotErr := configfile.Parse(tc.path) + cfg := runner.Config{} + gotErr := configfile.Parse(tc.path, &cfg) if gotErr == nil { t.Fatal("exp non-nil error, got nil") @@ -78,8 +79,8 @@ func TestParse(t *testing.T) { t.Errorf("\nexp %v\ngot %v", tc.expErr, gotErr) } - if !reflect.DeepEqual(gotCfg, runner.Config{}) { - t.Errorf("\nexp empty config\ngot %v", gotCfg) + if !reflect.DeepEqual(cfg, runner.Config{}) { + t.Errorf("\nexp empty config\ngot %v", cfg) } }) } @@ -90,8 +91,8 @@ func TestParse(t *testing.T) { expCfg := newExpConfig() fname := configPath("valid/benchttp" + ext) - gotCfg, err := configfile.Parse(fname) - if err != nil { + gotCfg := runner.Config{} + if err := configfile.Parse(fname, &gotCfg); err != nil { // critical error, stop the test t.Fatal(err) } @@ -108,7 +109,7 @@ func TestParse(t *testing.T) { restoreGotCfg := setTempValue(&gotURL.RawQuery, "replaced by test") restoreExpCfg := setTempValue(&expURL.RawQuery, "replaced by test") - if !gotCfg.Equal(expCfg) { + if !reflect.DeepEqual(gotCfg, expCfg) { t.Errorf("unexpected parsed config for %s file:\nexp %v\ngot %v", ext, expCfg, gotCfg) } @@ -117,25 +118,36 @@ func TestParse(t *testing.T) { } }) - t.Run("override default values", func(t *testing.T) { - const ( - expRequests = 0 // default is -1 - expGlobalTimeout = 42 * time.Millisecond - ) + t.Run("override input config", func(t *testing.T) { + cfg := runner.Config{} + cfg.Request.Method = "POST" + cfg.Runner.GlobalTimeout = 10 * time.Millisecond fname := configPath("valid/benchttp-zeros.yml") - cfg, err := configfile.Parse(fname) - if err != nil { + if err := configfile.Parse(fname, &cfg); err != nil { t.Fatal(err) } - if gotRequests := cfg.Runner.Requests; gotRequests != expRequests { - t.Errorf("did not override Requests: exp %d, got %d", expRequests, gotRequests) + const ( + expMethod = "POST" // from input config + expGlobalTimeout = 42 * time.Millisecond // from read file + ) + + if gotMethod := cfg.Request.Method; gotMethod != expMethod { + t.Errorf( + "did not keep input values that are not set: "+ + "exp Request.Method == %s, got %s", + expMethod, gotMethod, + ) } if gotGlobalTimeout := cfg.Runner.GlobalTimeout; gotGlobalTimeout != expGlobalTimeout { - t.Errorf("did not override GlobalTimeout: exp %d, got %d", expGlobalTimeout, gotGlobalTimeout) + t.Errorf( + "did not override input values that are set: "+ + "exp Runner.GlobalTimeout == %v, got %v", + expGlobalTimeout, gotGlobalTimeout, + ) } t.Log(cfg) @@ -161,8 +173,8 @@ func TestParse(t *testing.T) { for _, tc := range testcases { t.Run(tc.label, func(t *testing.T) { - cfg, err := configfile.Parse(tc.cfpath) - if err != nil { + var cfg runner.Config + if err := configfile.Parse(tc.cfpath, &cfg); err != nil { t.Fatal(err) } diff --git a/internal/configflag/bind.go b/internal/configflag/bind.go index 359cae8..e281d61 100644 --- a/internal/configflag/bind.go +++ b/internal/configflag/bind.go @@ -1,76 +1,125 @@ package configflag import ( + "errors" "flag" - "net/http" - "net/url" + "fmt" + "strconv" + "strings" + "github.com/benchttp/engine/configparse" "github.com/benchttp/engine/runner" ) -// Bind reads arguments provided to flagset as config.Fields and binds -// their value to the appropriate fields of given *config.Global. +// Bind reads arguments provided to flagset as config fields +// and binds their value to the appropriate fields of dst. // The provided *flag.Flagset must not have been parsed yet, otherwise // bindings its values would fail. -func Bind(flagset *flag.FlagSet, dst *runner.Config) { - // avoid nil pointer dereferences - if dst.Request.URL == nil { - dst.Request.URL = &url.URL{} - } - if dst.Request.Header == nil { - dst.Request.Header = http.Header{} +func Bind(flagset *flag.FlagSet, dst *configparse.Representation) { + for field, bind := range bindings { + flagset.Func(field, runner.ConfigFieldsUsage[field], bind(dst)) } +} - // request url - flagset.Var(urlValue{url: dst.Request.URL}, - runner.ConfigFieldURL, - runner.ConfigFieldsUsage[runner.ConfigFieldURL], - ) - // request method - flagset.StringVar(&dst.Request.Method, - runner.ConfigFieldMethod, - dst.Request.Method, - runner.ConfigFieldsUsage[runner.ConfigFieldMethod], - ) - // request header - flagset.Var(headerValue{header: &dst.Request.Header}, - runner.ConfigFieldHeader, - runner.ConfigFieldsUsage[runner.ConfigFieldHeader], - ) - // request body - flagset.Var(bodyValue{body: &dst.Request.Body}, - runner.ConfigFieldBody, - runner.ConfigFieldsUsage[runner.ConfigFieldBody], - ) - // requests number - flagset.IntVar(&dst.Runner.Requests, - runner.ConfigFieldRequests, - dst.Runner.Requests, - runner.ConfigFieldsUsage[runner.ConfigFieldRequests], - ) +type ( + repr = configparse.Representation + flagSetter = func(string) error +) - // concurrency - flagset.IntVar(&dst.Runner.Concurrency, - runner.ConfigFieldConcurrency, - dst.Runner.Concurrency, - runner.ConfigFieldsUsage[runner.ConfigFieldConcurrency], - ) - // non-conurrent requests interval - flagset.DurationVar(&dst.Runner.Interval, - runner.ConfigFieldInterval, - dst.Runner.Interval, - runner.ConfigFieldsUsage[runner.ConfigFieldInterval], - ) - // request timeout - flagset.DurationVar(&dst.Runner.RequestTimeout, - runner.ConfigFieldRequestTimeout, - dst.Runner.RequestTimeout, - runner.ConfigFieldsUsage[runner.ConfigFieldRequestTimeout], - ) - // global timeout - flagset.DurationVar(&dst.Runner.GlobalTimeout, - runner.ConfigFieldGlobalTimeout, - dst.Runner.GlobalTimeout, - runner.ConfigFieldsUsage[runner.ConfigFieldGlobalTimeout], - ) +var bindings = map[string]func(*repr) flagSetter{ + runner.ConfigFieldMethod: func(dst *repr) flagSetter { + return func(in string) error { + dst.Request.Method = &in + return nil + } + }, + runner.ConfigFieldURL: func(dst *repr) flagSetter { + return func(in string) error { + dst.Request.URL = &in + return nil + } + }, + runner.ConfigFieldHeader: func(dst *repr) flagSetter { + return func(in string) error { + keyval := strings.SplitN(in, ":", 2) + if len(keyval) != 2 { + return errors.New(`-header: expect format ":"`) + } + key, val := keyval[0], keyval[1] + if dst.Request.Header == nil { + dst.Request.Header = map[string][]string{} + } + dst.Request.Header[key] = append(dst.Request.Header[key], val) + return nil + } + }, + runner.ConfigFieldBody: func(dst *repr) flagSetter { + return func(in string) error { + errFormat := fmt.Errorf(`expect format ":", got %q`, in) + if in == "" { + return errFormat + } + split := strings.SplitN(in, ":", 2) + if len(split) != 2 { + return errFormat + } + btype, bcontent := split[0], split[1] + if bcontent == "" { + return errFormat + } + switch btype { + case "raw": + dst.Request.Body = &struct { + Type string `yaml:"type" json:"type"` + Content string `yaml:"content" json:"content"` + }{ + Type: btype, + Content: bcontent, + } + // case "file": + // // TODO + default: + return fmt.Errorf(`unsupported type: %s (only "raw" accepted)`, btype) + } + return nil + } + }, + runner.ConfigFieldRequests: func(dst *repr) flagSetter { + return func(in string) error { + n, err := strconv.Atoi(in) + if err != nil { + return err + } + dst.Runner.Requests = &n + return nil + } + }, + runner.ConfigFieldConcurrency: func(dst *repr) flagSetter { + return func(in string) error { + n, err := strconv.Atoi(in) + if err != nil { + return err + } + dst.Runner.Concurrency = &n + return nil + } + }, + runner.ConfigFieldInterval: func(dst *repr) flagSetter { + return func(in string) error { + dst.Runner.Interval = &in + return nil + } + }, + runner.ConfigFieldRequestTimeout: func(dst *repr) flagSetter { + return func(in string) error { + dst.Runner.RequestTimeout = &in + return nil + } + }, + runner.ConfigFieldGlobalTimeout: func(dst *repr) flagSetter { + return func(in string) error { + dst.Runner.GlobalTimeout = &in + return nil + } + }, } diff --git a/internal/configflag/bind_test.go b/internal/configflag/bind_test.go index 5c54727..40f0684 100644 --- a/internal/configflag/bind_test.go +++ b/internal/configflag/bind_test.go @@ -7,33 +7,37 @@ import ( "testing" "time" + "github.com/benchttp/engine/configparse" "github.com/benchttp/engine/runner" "github.com/benchttp/cli/internal/configflag" ) func TestBind(t *testing.T) { - t.Run("default to base config", func(t *testing.T) { - flagset := flag.NewFlagSet("run", flag.ExitOnError) + t.Run("default to zero representation", func(t *testing.T) { + flagset := flag.NewFlagSet("", flag.ExitOnError) args := []string{} // no args - cfg := runner.DefaultConfig() - configflag.Bind(flagset, &cfg) + repr := configparse.Representation{} + configflag.Bind(flagset, &repr) if err := flagset.Parse(args); err != nil { t.Fatal(err) // critical error, stop the test } - if exp := runner.DefaultConfig(); !reflect.DeepEqual(cfg, exp) { - t.Errorf("\nexp %#v\ngot %#v", exp, cfg) + exp := configparse.Representation{} + if got := repr; !reflect.DeepEqual(got, exp) { + t.Errorf("\nexp %#v\ngot %#v", exp, got) } }) t.Run("set config with flags values", func(t *testing.T) { - flagset := flag.NewFlagSet("run", flag.ExitOnError) + flagset := flag.NewFlagSet("", flag.ExitOnError) args := []string{ "-method", "POST", "-url", "https://benchttp.app?cool=yes", - "-header", "Content-Type:application/json", + "-header", "API_KEY:abc", + "-header", "Accept:text/html", + "-header", "Accept:application/json", "-body", "raw:hello", "-requests", "1", "-concurrency", "2", @@ -42,8 +46,8 @@ func TestBind(t *testing.T) { "-globalTimeout", "5s", } - cfg := runner.Config{} - configflag.Bind(flagset, &cfg) + repr := configparse.Representation{} + configflag.Bind(flagset, &repr) if err := flagset.Parse(args); err != nil { t.Fatal(err) // critical error, stop the test } @@ -51,8 +55,11 @@ func TestBind(t *testing.T) { exp := runner.Config{ Request: runner.RequestConfig{ Method: "POST", - Header: http.Header{"Content-Type": {"application/json"}}, - Body: runner.RequestBody{Type: "raw", Content: []byte("hello")}, + Header: http.Header{ + "API_KEY": {"abc"}, + "Accept": {"text/html", "application/json"}, + }, + Body: runner.RequestBody{Type: "raw", Content: []byte("hello")}, }.WithURL("https://benchttp.app?cool=yes"), Runner: runner.RecorderConfig{ Requests: 1, @@ -63,8 +70,12 @@ func TestBind(t *testing.T) { }, } - if !reflect.DeepEqual(cfg, exp) { - t.Errorf("\nexp %#v\ngot %#v", exp, cfg) + var got runner.Config + if err := repr.Unmarshal(&got); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, exp) { + t.Errorf("\nexp %#v\ngot %#v", exp, got) } }) } diff --git a/internal/configflag/body.go b/internal/configflag/body.go deleted file mode 100644 index f3f0dfb..0000000 --- a/internal/configflag/body.go +++ /dev/null @@ -1,50 +0,0 @@ -package configflag - -import ( - "fmt" - "strings" - - "github.com/benchttp/engine/runner" -) - -// bodyValue implements flag.Value -type bodyValue struct { - body *runner.RequestBody -} - -// String returns a string representation of the referenced body. -func (v bodyValue) String() string { - return fmt.Sprint(v.body) -} - -// Set reads input string in format "type:content" and sets -// the referenced body accordingly. -// -// Note: only type "raw" is supported at the moment. -func (v bodyValue) Set(raw string) error { - errFormat := fmt.Errorf(`expect format ":", got "%s"`, raw) - - if raw == "" { - return errFormat - } - - split := strings.SplitN(raw, ":", 2) - if len(split) != 2 { - return errFormat - } - - btype, bcontent := split[0], split[1] - if bcontent == "" { - return errFormat - } - - switch btype { - case "raw": - *v.body = runner.NewRequestBody(btype, bcontent) - // case "file": - // // TODO - default: - return fmt.Errorf(`unsupported type: %s (only "raw" accepted)`, btype) - } - return nil -} diff --git a/internal/configflag/header.go b/internal/configflag/header.go deleted file mode 100644 index 232180e..0000000 --- a/internal/configflag/header.go +++ /dev/null @@ -1,30 +0,0 @@ -package configflag - -import ( - "errors" - "fmt" - "net/http" - "strings" -) - -// headerValue implements flag.Value -type headerValue struct { - header *http.Header -} - -// String returns a string representation of the referenced header. -func (v headerValue) String() string { - return fmt.Sprint(v.header) -} - -// Set reads input string in format "key:value" and appends value -// to the key's values of the referenced header. -func (v headerValue) Set(raw string) error { - keyval := strings.SplitN(raw, ":", 2) - if len(keyval) != 2 { - return errors.New(`expect format ":"`) - } - key, val := keyval[0], keyval[1] - (*v.header)[key] = append((*v.header)[key], val) - return nil -} diff --git a/internal/configflag/url.go b/internal/configflag/url.go deleted file mode 100644 index c1eb907..0000000 --- a/internal/configflag/url.go +++ /dev/null @@ -1,29 +0,0 @@ -package configflag - -import ( - "fmt" - "net/url" -) - -// urlValue implements flag.Value -type urlValue struct { - url *url.URL -} - -// String returns a string representation of urlValue.url. -func (v urlValue) String() string { - if v.url == nil { - return "" - } - return v.url.String() -} - -// Set parses input string as a URL and sets the referenced URL accordingly. -func (v urlValue) Set(in string) error { - urlURL, err := url.ParseRequestURI(in) - if err != nil { - return fmt.Errorf(`invalid url: "%s"`, in) - } - *v.url = *urlURL - return nil -} diff --git a/internal/configflag/which.go b/internal/configflag/which.go deleted file mode 100644 index c084ecd..0000000 --- a/internal/configflag/which.go +++ /dev/null @@ -1,19 +0,0 @@ -package configflag - -import ( - "flag" - - "github.com/benchttp/engine/runner" -) - -// Which returns a slice of all config fields set via the CLI -// for the given *flag.FlagSet. -func Which(flagset *flag.FlagSet) []string { - var fields []string - flagset.Visit(func(f *flag.Flag) { - if name := f.Name; runner.IsConfigField(name) { - fields = append(fields, name) - } - }) - return fields -} diff --git a/internal/configflag/which_test.go b/internal/configflag/which_test.go deleted file mode 100644 index 80cf4ae..0000000 --- a/internal/configflag/which_test.go +++ /dev/null @@ -1,52 +0,0 @@ -package configflag_test - -import ( - "flag" - "reflect" - "testing" - - "github.com/benchttp/engine/runner" - - "github.com/benchttp/cli/internal/configflag" -) - -func TestWhich(t *testing.T) { - for _, tc := range []struct { - label string - args []string - exp []string - }{ - { - label: "return all config flags set", - args: []string{ - "-method", "POST", - "-url", "https://benchttp.app?cool=yes", - "-concurrency", "2", - "-requests", "3", - "-requestTimeout", "1s", - "-globalTimeout", "4s", - }, - exp: []string{ - "concurrency", "globalTimeout", "method", - "requestTimeout", "requests", "url", - }, - }, - { - label: "do not return config flags not set", - args: []string{"-requests", "3"}, - exp: []string{"requests"}, - }, - } { - flagset := flag.NewFlagSet("run", flag.ExitOnError) - - configflag.Bind(flagset, &runner.Config{}) - - if err := flagset.Parse(tc.args); err != nil { - t.Fatal(err) // critical error, stop the test - } - - if got := configflag.Which(flagset); !reflect.DeepEqual(got, tc.exp) { - t.Errorf("\nexp %v\ngot %v", tc.exp, got) - } - } -} From 5addce48b603b0b0fef8d0591067c29b9cc36aeb Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 19 Oct 2022 02:52:18 +0200 Subject: [PATCH 02/11] refactor: update after engine changes - Unmarshal -> ParseInto - Removal of runner.ConfigFields - implement testutil.MustMakeRequest --- cmd/benchttp/run.go | 11 ++++-- internal/configfile/parse.go | 2 +- internal/configfile/parse_test.go | 66 +++++++++++++++++++------------ internal/configflag/bind.go | 21 +++++----- internal/configflag/bind_test.go | 41 +++++++------------ internal/configflag/configflag.go | 28 +++++++++++++ internal/render/report_test.go | 13 +++++- internal/testutil/http.go | 15 +++++++ 8 files changed, 127 insertions(+), 70 deletions(-) create mode 100644 internal/configflag/configflag.go create mode 100644 internal/testutil/http.go diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index 5edb37b..a4431bc 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -40,7 +40,7 @@ func (cmd *cmdRun) execute(args []string) error { return err } - config, err := cmd.buildConfig() + config, err := buildConfig(cmd.configFile, cmd.configRepr) if err != nil { return err } @@ -60,12 +60,15 @@ func (cmd *cmdRun) parseArgs(args []string) error { return cmd.flagset.Parse(args) } -func (cmd *cmdRun) buildConfig() (runner.Config, error) { +func buildConfig( + filePath string, + cliConfigRepr configparse.Representation, +) (runner.Config, error) { // start with default config as base config := runner.DefaultConfig() // override with config file values - err := configfile.Parse(cmd.configFile, &config) + err := configfile.Parse(filePath, &config) if err != nil && !errors.Is(err, configfile.ErrFileNotFound) { // config file is not mandatory: discard ErrFileNotFound. // other errors are critical @@ -73,7 +76,7 @@ func (cmd *cmdRun) buildConfig() (runner.Config, error) { } // override with CLI flags values - if err := cmd.configRepr.Unmarshal(&config); err != nil { + if err := cliConfigRepr.ParseInto(&config); err != nil { return config, err } diff --git a/internal/configfile/parse.go b/internal/configfile/parse.go index 078da3e..c54c77e 100644 --- a/internal/configfile/parse.go +++ b/internal/configfile/parse.go @@ -106,7 +106,7 @@ func parseAndMergeConfigs(reprs []configparse.Representation, dst *runner.Config } for i := len(reprs) - 1; i >= 0; i-- { - if err := reprs[i].Unmarshal(dst); err != nil { + if err := reprs[i].ParseInto(dst); err != nil { return errorutil.WithDetails(ErrFileParse, err) } } diff --git a/internal/configfile/parse_test.go b/internal/configfile/parse_test.go index 978743a..0bcae2a 100644 --- a/internal/configfile/parse_test.go +++ b/internal/configfile/parse_test.go @@ -3,6 +3,7 @@ package configfile_test import ( "errors" "fmt" + "io" "net/http" "net/url" "path/filepath" @@ -13,11 +14,12 @@ import ( "github.com/benchttp/engine/runner" "github.com/benchttp/cli/internal/configfile" + "github.com/benchttp/cli/internal/testutil" ) const ( - testdataConfigPath = "./testdata" - testURL = "http://localhost:9999?fib=30&delay=200ms" // value from testdata files + validConfigPath = "./testdata" + validURL = "http://localhost:9999?fib=30&delay=200ms" // value from testdata files ) var supportedExt = []string{ @@ -97,30 +99,20 @@ func TestParse(t *testing.T) { t.Fatal(err) } - expURL, gotURL := expCfg.Request.URL, gotCfg.Request.URL - - // compare *url.URLs separately, as they contain unpredictable values - // they need special checks - if !sameURL(gotURL, expURL) { - t.Errorf("unexpected parsed URL:\nexp %v, got %v", expURL, gotURL) + if sameConfig(gotCfg, runner.Config{}) { + t.Error("received an empty configuration") } - // replace unpredictable values (undetermined query params order) - restoreGotCfg := setTempValue(&gotURL.RawQuery, "replaced by test") - restoreExpCfg := setTempValue(&expURL.RawQuery, "replaced by test") - - if !reflect.DeepEqual(gotCfg, expCfg) { - t.Errorf("unexpected parsed config for %s file:\nexp %v\ngot %v", ext, expCfg, gotCfg) + if !sameConfig(gotCfg, expCfg) { + t.Errorf("unexpected parsed config for %s file:\nexp %#v\ngot %#v", ext, expCfg, gotCfg) } - restoreExpCfg() - restoreGotCfg() } }) t.Run("override input config", func(t *testing.T) { cfg := runner.Config{} - cfg.Request.Method = "POST" + cfg.Request = testutil.MustMakeRequest("POST", "https://overriden.com", nil, nil) cfg.Runner.GlobalTimeout = 10 * time.Millisecond fname := configPath("valid/benchttp-zeros.yml") @@ -200,18 +192,17 @@ func TestParse(t *testing.T) { // newExpConfig returns the expected runner.ConfigConfig result after parsing // one of the config files in testdataConfigPath. func newExpConfig() runner.Config { - u, _ := url.ParseRequestURI(testURL) return runner.Config{ - Request: runner.RequestConfig{ - Method: "POST", - URL: u, - Header: http.Header{ + Request: testutil.MustMakeRequest( + "POST", + validURL, + http.Header{ "key0": []string{"val0", "val1"}, "key1": []string{"val0"}, }, - Body: runner.NewRequestBody("raw", `{"key0":"val0","key1":"val1"}`), - }, - Runner: runner.RecorderConfig{ + []byte(`{"key0":"val0","key1":"val1"}`), + ), + Runner: runner.RunnerConfig{ Requests: 100, Concurrency: 1, Interval: 50 * time.Millisecond, @@ -241,6 +232,15 @@ func newExpConfig() runner.Config { } } +func sameConfig(a, b runner.Config) bool { + if a.Request == nil || b.Request == nil { + return a.Request == nil && b.Request == nil + } + return sameURL(a.Request.URL, b.Request.URL) && + sameHeader(a.Request.Header, b.Request.Header) && + sameBody(a.Request.Body, b.Request.Body) +} + // sameURL returns true if a and b are the same *url.URL, taking into account // the undeterministic nature of their RawQuery. func sameURL(a, b *url.URL) bool { @@ -258,6 +258,20 @@ func sameURL(a, b *url.URL) bool { return reflect.DeepEqual(a, b) } +func sameHeader(a, b http.Header) bool { + return reflect.DeepEqual(a, b) + // if len(a) != len(b) { + // return false + // } + // for k, values := range a { + // if len(values) != len() + // } +} + +func sameBody(a, b io.ReadCloser) bool { + return reflect.DeepEqual(a, b) +} + // setTempValue sets *ptr to val and returns a restore func that sets *ptr // back to its previous value. func setTempValue(ptr *string, val string) (restore func()) { @@ -269,5 +283,5 @@ func setTempValue(ptr *string, val string) (restore func()) { } func configPath(name string) string { - return filepath.Join(testdataConfigPath, name) + return filepath.Join(validConfigPath, name) } diff --git a/internal/configflag/bind.go b/internal/configflag/bind.go index e281d61..586ab23 100644 --- a/internal/configflag/bind.go +++ b/internal/configflag/bind.go @@ -8,7 +8,6 @@ import ( "strings" "github.com/benchttp/engine/configparse" - "github.com/benchttp/engine/runner" ) // Bind reads arguments provided to flagset as config fields @@ -17,7 +16,7 @@ import ( // bindings its values would fail. func Bind(flagset *flag.FlagSet, dst *configparse.Representation) { for field, bind := range bindings { - flagset.Func(field, runner.ConfigFieldsUsage[field], bind(dst)) + flagset.Func(field, flagsUsage[field], bind(dst)) } } @@ -27,19 +26,19 @@ type ( ) var bindings = map[string]func(*repr) flagSetter{ - runner.ConfigFieldMethod: func(dst *repr) flagSetter { + flagMethod: func(dst *repr) flagSetter { return func(in string) error { dst.Request.Method = &in return nil } }, - runner.ConfigFieldURL: func(dst *repr) flagSetter { + flagURL: func(dst *repr) flagSetter { return func(in string) error { dst.Request.URL = &in return nil } }, - runner.ConfigFieldHeader: func(dst *repr) flagSetter { + flagHeader: func(dst *repr) flagSetter { return func(in string) error { keyval := strings.SplitN(in, ":", 2) if len(keyval) != 2 { @@ -53,7 +52,7 @@ var bindings = map[string]func(*repr) flagSetter{ return nil } }, - runner.ConfigFieldBody: func(dst *repr) flagSetter { + flagBody: func(dst *repr) flagSetter { return func(in string) error { errFormat := fmt.Errorf(`expect format ":", got %q`, in) if in == "" { @@ -84,7 +83,7 @@ var bindings = map[string]func(*repr) flagSetter{ return nil } }, - runner.ConfigFieldRequests: func(dst *repr) flagSetter { + flagRequests: func(dst *repr) flagSetter { return func(in string) error { n, err := strconv.Atoi(in) if err != nil { @@ -94,7 +93,7 @@ var bindings = map[string]func(*repr) flagSetter{ return nil } }, - runner.ConfigFieldConcurrency: func(dst *repr) flagSetter { + flagConcurrency: func(dst *repr) flagSetter { return func(in string) error { n, err := strconv.Atoi(in) if err != nil { @@ -104,19 +103,19 @@ var bindings = map[string]func(*repr) flagSetter{ return nil } }, - runner.ConfigFieldInterval: func(dst *repr) flagSetter { + flagInterval: func(dst *repr) flagSetter { return func(in string) error { dst.Runner.Interval = &in return nil } }, - runner.ConfigFieldRequestTimeout: func(dst *repr) flagSetter { + flagRequestTimeout: func(dst *repr) flagSetter { return func(in string) error { dst.Runner.RequestTimeout = &in return nil } }, - runner.ConfigFieldGlobalTimeout: func(dst *repr) flagSetter { + flagGlobalTimeout: func(dst *repr) flagSetter { return func(in string) error { dst.Runner.GlobalTimeout = &in return nil diff --git a/internal/configflag/bind_test.go b/internal/configflag/bind_test.go index 40f0684..6e06a06 100644 --- a/internal/configflag/bind_test.go +++ b/internal/configflag/bind_test.go @@ -2,13 +2,10 @@ package configflag_test import ( "flag" - "net/http" "reflect" "testing" - "time" "github.com/benchttp/engine/configparse" - "github.com/benchttp/engine/runner" "github.com/benchttp/cli/internal/configflag" ) @@ -52,30 +49,22 @@ func TestBind(t *testing.T) { t.Fatal(err) // critical error, stop the test } - exp := runner.Config{ - Request: runner.RequestConfig{ - Method: "POST", - Header: http.Header{ - "API_KEY": {"abc"}, - "Accept": {"text/html", "application/json"}, - }, - Body: runner.RequestBody{Type: "raw", Content: []byte("hello")}, - }.WithURL("https://benchttp.app?cool=yes"), - Runner: runner.RecorderConfig{ - Requests: 1, - Concurrency: 2, - Interval: 3 * time.Second, - RequestTimeout: 4 * time.Second, - GlobalTimeout: 5 * time.Second, - }, - } + switch { + case *repr.Request.Method != "POST", + *repr.Request.URL != "https://benchttp.app?cool=yes", + repr.Request.Header["API_KEY"][0] != "abc", + repr.Request.Header["Accept"][0] != "text/html", + repr.Request.Header["Accept"][1] != "application/json", + repr.Request.Body.Type != "raw", + repr.Request.Body.Content != "hello", - var got runner.Config - if err := repr.Unmarshal(&got); err != nil { - t.Fatal(err) - } - if !reflect.DeepEqual(got, exp) { - t.Errorf("\nexp %#v\ngot %#v", exp, got) + *repr.Runner.Requests != 1, + *repr.Runner.Concurrency != 2, + *repr.Runner.Interval != "3s", + *repr.Runner.RequestTimeout != "4s", + *repr.Runner.GlobalTimeout != "5s": + + t.Error("unexpected parsed flags") } }) } diff --git a/internal/configflag/configflag.go b/internal/configflag/configflag.go new file mode 100644 index 0000000..e841413 --- /dev/null +++ b/internal/configflag/configflag.go @@ -0,0 +1,28 @@ +package configflag + +const ( + flagMethod = "method" + flagURL = "url" + flagHeader = "header" + flagBody = "body" + flagRequests = "requests" + flagConcurrency = "concurrency" + flagInterval = "interval" + flagRequestTimeout = "requestTimeout" + flagGlobalTimeout = "globalTimeout" + flagTests = "tests" +) + +// flagsUsage is a record of all available config flags and their usage. +var flagsUsage = map[string]string{ + flagMethod: "HTTP request method", + flagURL: "HTTP request url", + flagHeader: "HTTP request header", + flagBody: "HTTP request body", + flagRequests: "Number of requests to run, use duration as exit condition if omitted", + flagConcurrency: "Number of connections to run concurrently", + flagInterval: "Minimum duration between two non concurrent requests", + flagRequestTimeout: "Timeout for each HTTP request", + flagGlobalTimeout: "Max duration of test", + flagTests: "Test suite", +} diff --git a/internal/render/report_test.go b/internal/render/report_test.go index 1fad710..3e64cc4 100644 --- a/internal/render/report_test.go +++ b/internal/render/report_test.go @@ -1,6 +1,7 @@ package render_test import ( + "net/http" "testing" "time" @@ -17,7 +18,7 @@ func TestReport_String(t *testing.T) { rep := &runner.Report{ Metrics: metrics, - Metadata: runner.ReportMetadata{ + Metadata: runner.Metadata{ Config: cfg, TotalDuration: duration, }, @@ -44,7 +45,7 @@ func metricsStub() (agg runner.MetricsAggregate, total time.Duration) { func configStub() runner.Config { cfg := runner.Config{} - cfg.Request = cfg.Request.WithURL("https://a.b.com") + cfg.Request = mustMakeRequest("https://a.b.com") cfg.Runner.Requests = -1 return cfg } @@ -67,3 +68,11 @@ Total duration 15000ms t.Errorf("\nexp summary:\n%q\ngot summary:\n%q", expSummary, summary) } } + +func mustMakeRequest(uri string) *http.Request { + req, err := http.NewRequest("GET", uri, nil) + if err != nil { + panic(err) + } + return req +} diff --git a/internal/testutil/http.go b/internal/testutil/http.go new file mode 100644 index 0000000..34b660b --- /dev/null +++ b/internal/testutil/http.go @@ -0,0 +1,15 @@ +package testutil + +import ( + "bytes" + "net/http" +) + +func MustMakeRequest(method, uri string, header http.Header, body []byte) *http.Request { + req, err := http.NewRequest(method, uri, bytes.NewReader(body)) + if err != nil { + panic("testutil.MustMakeRequest: " + err.Error()) + } + req.Header = header + return req +} From 23916b91ffbfaa0cb042e7fa466c3a72a36f9017 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 22 Oct 2022 23:55:17 +0200 Subject: [PATCH 03/11] chore: runner.Config -> runner.Runner --- cmd/benchttp/run.go | 20 ++++++------ internal/configfile/parse.go | 15 +++++---- internal/configfile/parse_test.go | 54 +++++++++++++++---------------- internal/render/report.go | 10 +++--- internal/render/report_test.go | 16 ++++----- 5 files changed, 57 insertions(+), 58 deletions(-) diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index a4431bc..46619f1 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -63,24 +63,24 @@ func (cmd *cmdRun) parseArgs(args []string) error { func buildConfig( filePath string, cliConfigRepr configparse.Representation, -) (runner.Config, error) { - // start with default config as base - config := runner.DefaultConfig() +) (runner.Runner, error) { + // start with default brunner as base + brunner := runner.DefaultRunner() // override with config file values - err := configfile.Parse(filePath, &config) + err := configfile.Parse(filePath, &brunner) if err != nil && !errors.Is(err, configfile.ErrFileNotFound) { // config file is not mandatory: discard ErrFileNotFound. // other errors are critical - return config, err + return brunner, err } // override with CLI flags values - if err := cliConfigRepr.ParseInto(&config); err != nil { - return config, err + if err := cliConfigRepr.ParseInto(&brunner); err != nil { + return brunner, err } - return config, nil + return brunner, nil } func onRecordingProgress(silent bool) func(runner.RecordingProgress) { @@ -97,7 +97,7 @@ func onRecordingProgress(silent bool) func(runner.RecordingProgress) { } } -func runBenchmark(cfg runner.Config, silent bool) (*runner.Report, error) { +func runBenchmark(brunner runner.Runner, silent bool) (*runner.Report, error) { // Prepare graceful shutdown in case of os.Interrupt (Ctrl+C) ctx, cancel := context.WithCancel(context.Background()) go signals.ListenOSInterrupt(cancel) @@ -105,7 +105,7 @@ func runBenchmark(cfg runner.Config, silent bool) (*runner.Report, error) { // Run the benchmark report, err := runner. New(onRecordingProgress(silent)). - Run(ctx, cfg) + Run(ctx, brunner) if err != nil { return report, err } diff --git a/internal/configfile/parse.go b/internal/configfile/parse.go index c54c77e..f5f4cc8 100644 --- a/internal/configfile/parse.go +++ b/internal/configfile/parse.go @@ -11,16 +11,16 @@ import ( "github.com/benchttp/cli/internal/errorutil" ) -// Parse parses given filename as a benchttp runner config file -// into a runner.Config and stores the retrieved values into *dst. +// Parse parses given filename as a benchttp runner configuration +// into a runner.Runner and stores the retrieved values into *dst. // It returns the first error occurring in the process, which can be // any of the values declared in the package. -func Parse(filename string, cfg *runner.Config) (err error) { +func Parse(filename string, dst *runner.Runner) (err error) { reprs, err := parseFileRecursive(filename, []configparse.Representation{}, set{}) if err != nil { return } - return parseAndMergeConfigs(reprs, cfg) + return parseAndMergeConfigs(reprs, dst) } // set is a collection of unique string values. @@ -92,11 +92,12 @@ func parseFile(filename string) (repr configparse.Representation, err error) { return repr, nil } -// parseAndMergeConfigs iterates backwards over uconfs, parsing them -// as runner.ConfigGlobal and merging them into a single one. +// parseAndMergeConfigs iterates backwards over reprs, parses them as +// runner.Runner, merges them successively and finally stores the result +// into dst. // It returns the merged result or the first non-nil error occurring in the // process. -func parseAndMergeConfigs(reprs []configparse.Representation, dst *runner.Config) error { +func parseAndMergeConfigs(reprs []configparse.Representation, dst *runner.Runner) error { if len(reprs) == 0 { // supposedly catched upstream, should not occur return errors.New( "an unacceptable error occurred parsing the config file, " + diff --git a/internal/configfile/parse_test.go b/internal/configfile/parse_test.go index 0bcae2a..7039c47 100644 --- a/internal/configfile/parse_test.go +++ b/internal/configfile/parse_test.go @@ -70,8 +70,8 @@ func TestParse(t *testing.T) { for _, tc := range testcases { t.Run(tc.label, func(t *testing.T) { - cfg := runner.Config{} - gotErr := configfile.Parse(tc.path, &cfg) + brunner := runner.Runner{} + gotErr := configfile.Parse(tc.path, &brunner) if gotErr == nil { t.Fatal("exp non-nil error, got nil") @@ -81,8 +81,8 @@ func TestParse(t *testing.T) { t.Errorf("\nexp %v\ngot %v", tc.expErr, gotErr) } - if !reflect.DeepEqual(cfg, runner.Config{}) { - t.Errorf("\nexp empty config\ngot %v", cfg) + if !reflect.DeepEqual(brunner, runner.Runner{}) { + t.Errorf("\nexp empty config\ngot %v", brunner) } }) } @@ -93,13 +93,13 @@ func TestParse(t *testing.T) { expCfg := newExpConfig() fname := configPath("valid/benchttp" + ext) - gotCfg := runner.Config{} + gotCfg := runner.Runner{} if err := configfile.Parse(fname, &gotCfg); err != nil { // critical error, stop the test t.Fatal(err) } - if sameConfig(gotCfg, runner.Config{}) { + if sameConfig(gotCfg, runner.Runner{}) { t.Error("received an empty configuration") } @@ -111,13 +111,13 @@ func TestParse(t *testing.T) { }) t.Run("override input config", func(t *testing.T) { - cfg := runner.Config{} - cfg.Request = testutil.MustMakeRequest("POST", "https://overriden.com", nil, nil) - cfg.Runner.GlobalTimeout = 10 * time.Millisecond + brunner := runner.Runner{} + brunner.Request = testutil.MustMakeRequest("POST", "https://overriden.com", nil, nil) + brunner.GlobalTimeout = 10 * time.Millisecond fname := configPath("valid/benchttp-zeros.yml") - if err := configfile.Parse(fname, &cfg); err != nil { + if err := configfile.Parse(fname, &brunner); err != nil { t.Fatal(err) } @@ -126,7 +126,7 @@ func TestParse(t *testing.T) { expGlobalTimeout = 42 * time.Millisecond // from read file ) - if gotMethod := cfg.Request.Method; gotMethod != expMethod { + if gotMethod := brunner.Request.Method; gotMethod != expMethod { t.Errorf( "did not keep input values that are not set: "+ "exp Request.Method == %s, got %s", @@ -134,7 +134,7 @@ func TestParse(t *testing.T) { ) } - if gotGlobalTimeout := cfg.Runner.GlobalTimeout; gotGlobalTimeout != expGlobalTimeout { + if gotGlobalTimeout := brunner.GlobalTimeout; gotGlobalTimeout != expGlobalTimeout { t.Errorf( "did not override input values that are set: "+ "exp Runner.GlobalTimeout == %v, got %v", @@ -142,7 +142,7 @@ func TestParse(t *testing.T) { ) } - t.Log(cfg) + t.Log(brunner) }) t.Run("extend config files", func(t *testing.T) { @@ -165,8 +165,8 @@ func TestParse(t *testing.T) { for _, tc := range testcases { t.Run(tc.label, func(t *testing.T) { - var cfg runner.Config - if err := configfile.Parse(tc.cfpath, &cfg); err != nil { + var brunner runner.Runner + if err := configfile.Parse(tc.cfpath, &brunner); err != nil { t.Fatal(err) } @@ -175,11 +175,11 @@ func TestParse(t *testing.T) { expURL = fmt.Sprintf("http://%s.config", tc.cfname) ) - if gotMethod := cfg.Request.Method; gotMethod != expMethod { + if gotMethod := brunner.Request.Method; gotMethod != expMethod { t.Errorf("method: exp %s, got %s", expMethod, gotMethod) } - if gotURL := cfg.Request.URL.String(); gotURL != expURL { + if gotURL := brunner.Request.URL.String(); gotURL != expURL { t.Errorf("url: exp %s, got %s", expURL, gotURL) } }) @@ -191,8 +191,8 @@ func TestParse(t *testing.T) { // newExpConfig returns the expected runner.ConfigConfig result after parsing // one of the config files in testdataConfigPath. -func newExpConfig() runner.Config { - return runner.Config{ +func newExpConfig() runner.Runner { + return runner.Runner{ Request: testutil.MustMakeRequest( "POST", validURL, @@ -202,13 +202,13 @@ func newExpConfig() runner.Config { }, []byte(`{"key0":"val0","key1":"val1"}`), ), - Runner: runner.RunnerConfig{ - Requests: 100, - Concurrency: 1, - Interval: 50 * time.Millisecond, - RequestTimeout: 2 * time.Second, - GlobalTimeout: 60 * time.Second, - }, + + Requests: 100, + Concurrency: 1, + Interval: 50 * time.Millisecond, + RequestTimeout: 2 * time.Second, + GlobalTimeout: 60 * time.Second, + Tests: []runner.TestCase{ { Name: "minimum response time", @@ -232,7 +232,7 @@ func newExpConfig() runner.Config { } } -func sameConfig(a, b runner.Config) bool { +func sameConfig(a, b runner.Runner) bool { if a.Request == nil || b.Request == nil { return a.Request == nil && b.Request == nil } diff --git a/internal/render/report.go b/internal/render/report.go index d5f996f..4d16517 100644 --- a/internal/render/report.go +++ b/internal/render/report.go @@ -37,15 +37,13 @@ func ReportSummaryString(rep *runner.Report) string { return fmt.Sprintf("%d/%s", n, maxString) } - var ( - m = rep.Metrics - cfg = rep.Metadata.Config - ) + m := rep.Metrics + r := rep.Metadata.Runner b.WriteString(ansi.Bold("→ Summary")) b.WriteString("\n") - b.WriteString(line("Endpoint", cfg.Request.URL)) - b.WriteString(line("Requests", formatRequests(len(m.Records), cfg.Runner.Requests))) + b.WriteString(line("Endpoint", r.Request.URL)) + b.WriteString(line("Requests", formatRequests(len(m.Records), r.Requests))) b.WriteString(line("Errors", len(m.RequestFailures))) b.WriteString(line("Min response time", msString(m.ResponseTimes.Min))) b.WriteString(line("Max response time", msString(m.ResponseTimes.Max))) diff --git a/internal/render/report_test.go b/internal/render/report_test.go index 3e64cc4..a8bc5f3 100644 --- a/internal/render/report_test.go +++ b/internal/render/report_test.go @@ -14,12 +14,12 @@ import ( func TestReport_String(t *testing.T) { t.Run("returns metrics summary", func(t *testing.T) { metrics, duration := metricsStub() - cfg := configStub() + runner := runnerStub() rep := &runner.Report{ Metrics: metrics, - Metadata: runner.Metadata{ - Config: cfg, + Metadata: benchttp.Metadata{ + Runner: runner, TotalDuration: duration, }, } @@ -43,11 +43,11 @@ func metricsStub() (agg runner.MetricsAggregate, total time.Duration) { }, 15 * time.Second } -func configStub() runner.Config { - cfg := runner.Config{} - cfg.Request = mustMakeRequest("https://a.b.com") - cfg.Runner.Requests = -1 - return cfg +func runnerStub() runner.Runner { + brunner := runner.Runner{} + brunner.Request = mustMakeRequest("https://a.b.com") + brunner.Requests = -1 + return brunner } func checkSummary(t *testing.T, summary string) { From c3415816f30743ac213f1a0fda82ed649e87886e Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 16:57:48 +0200 Subject: [PATCH 04/11] chore: runner.New() -> runner.Runner{} --- cmd/benchttp/run.go | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index 46619f1..de30a99 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -83,6 +83,23 @@ func buildConfig( return brunner, nil } +func runBenchmark(brunner runner.Runner, silent bool) (*runner.Report, error) { + // Prepare graceful shutdown in case of os.Interrupt (Ctrl+C) + ctx, cancel := context.WithCancel(context.Background()) + go signals.ListenOSInterrupt(cancel) + + // Stream progress to stdout + brunner.OnProgress = onRecordingProgress(silent) + + // Run the benchmark + report, err := brunner.Run(ctx) + if err != nil { + return report, err + } + + return report, nil +} + func onRecordingProgress(silent bool) func(runner.RecordingProgress) { if silent { return func(runner.RecordingProgress) {} @@ -97,22 +114,6 @@ func onRecordingProgress(silent bool) func(runner.RecordingProgress) { } } -func runBenchmark(brunner runner.Runner, silent bool) (*runner.Report, error) { - // Prepare graceful shutdown in case of os.Interrupt (Ctrl+C) - ctx, cancel := context.WithCancel(context.Background()) - go signals.ListenOSInterrupt(cancel) - - // Run the benchmark - report, err := runner. - New(onRecordingProgress(silent)). - Run(ctx, brunner) - if err != nil { - return report, err - } - - return report, nil -} - func renderReport(w io.Writer, report *runner.Report, silent bool) error { writeIfNotSilent := output.ConditionalWriter{Writer: w}.If(!silent) From 9a4ed83ae8eca562c9060cd170544bed7d73bca9 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 18:56:01 +0200 Subject: [PATCH 05/11] chore: apply sdk module name changes --- cmd/benchttp/run.go | 34 ++++++++++++------------ go.mod | 2 +- internal/configfile/parse.go | 8 +++--- internal/configfile/parse_test.go | 44 +++++++++++++++---------------- internal/configfile/parser.go | 2 +- internal/configflag/bind.go | 2 +- internal/configflag/bind_test.go | 2 +- internal/render/progress.go | 18 ++++++------- internal/render/report.go | 6 ++--- internal/render/report_test.go | 20 +++++++------- internal/render/testsuite.go | 6 ++--- 11 files changed, 72 insertions(+), 72 deletions(-) diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index de30a99..3765758 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -8,8 +8,8 @@ import ( "io" "os" - "github.com/benchttp/engine/configparse" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" + "github.com/benchttp/sdk/configparse" "github.com/benchttp/cli/internal/configfile" "github.com/benchttp/cli/internal/configflag" @@ -63,36 +63,36 @@ func (cmd *cmdRun) parseArgs(args []string) error { func buildConfig( filePath string, cliConfigRepr configparse.Representation, -) (runner.Runner, error) { - // start with default brunner as base - brunner := runner.DefaultRunner() +) (benchttp.Runner, error) { + // start with default runner as base + runner := benchttp.DefaultRunner() // override with config file values - err := configfile.Parse(filePath, &brunner) + err := configfile.Parse(filePath, &runner) if err != nil && !errors.Is(err, configfile.ErrFileNotFound) { // config file is not mandatory: discard ErrFileNotFound. // other errors are critical - return brunner, err + return runner, err } // override with CLI flags values - if err := cliConfigRepr.ParseInto(&brunner); err != nil { - return brunner, err + if err := cliConfigRepr.ParseInto(&runner); err != nil { + return runner, err } - return brunner, nil + return runner, nil } -func runBenchmark(brunner runner.Runner, silent bool) (*runner.Report, error) { +func runBenchmark(runner benchttp.Runner, silent bool) (*benchttp.Report, error) { // Prepare graceful shutdown in case of os.Interrupt (Ctrl+C) ctx, cancel := context.WithCancel(context.Background()) go signals.ListenOSInterrupt(cancel) // Stream progress to stdout - brunner.OnProgress = onRecordingProgress(silent) + runner.OnProgress = onRecordingProgress(silent) // Run the benchmark - report, err := brunner.Run(ctx) + report, err := runner.Run(ctx) if err != nil { return report, err } @@ -100,21 +100,21 @@ func runBenchmark(brunner runner.Runner, silent bool) (*runner.Report, error) { return report, nil } -func onRecordingProgress(silent bool) func(runner.RecordingProgress) { +func onRecordingProgress(silent bool) func(benchttp.RecordingProgress) { if silent { - return func(runner.RecordingProgress) {} + return func(benchttp.RecordingProgress) {} } // hack: write a blank line as render.Progress always // erases the previous line fmt.Println() - return func(progress runner.RecordingProgress) { + return func(progress benchttp.RecordingProgress) { render.Progress(os.Stdout, progress) //nolint: errcheck } } -func renderReport(w io.Writer, report *runner.Report, silent bool) error { +func renderReport(w io.Writer, report *benchttp.Report, silent bool) error { writeIfNotSilent := output.ConditionalWriter{Writer: w}.If(!silent) if _, err := render.ReportSummary(writeIfNotSilent, report); err != nil { diff --git a/go.mod b/go.mod index b66e1f3..e4e1eb9 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/benchttp/cli go 1.17 -require github.com/benchttp/engine v0.1.0 +require github.com/benchttp/sdk v0.1.0 require ( golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 // indirect diff --git a/internal/configfile/parse.go b/internal/configfile/parse.go index f5f4cc8..0120481 100644 --- a/internal/configfile/parse.go +++ b/internal/configfile/parse.go @@ -5,8 +5,8 @@ import ( "os" "path/filepath" - "github.com/benchttp/engine/configparse" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" + "github.com/benchttp/sdk/configparse" "github.com/benchttp/cli/internal/errorutil" ) @@ -15,7 +15,7 @@ import ( // into a runner.Runner and stores the retrieved values into *dst. // It returns the first error occurring in the process, which can be // any of the values declared in the package. -func Parse(filename string, dst *runner.Runner) (err error) { +func Parse(filename string, dst *benchttp.Runner) (err error) { reprs, err := parseFileRecursive(filename, []configparse.Representation{}, set{}) if err != nil { return @@ -97,7 +97,7 @@ func parseFile(filename string) (repr configparse.Representation, err error) { // into dst. // It returns the merged result or the first non-nil error occurring in the // process. -func parseAndMergeConfigs(reprs []configparse.Representation, dst *runner.Runner) error { +func parseAndMergeConfigs(reprs []configparse.Representation, dst *benchttp.Runner) error { if len(reprs) == 0 { // supposedly catched upstream, should not occur return errors.New( "an unacceptable error occurred parsing the config file, " + diff --git a/internal/configfile/parse_test.go b/internal/configfile/parse_test.go index 7039c47..3720544 100644 --- a/internal/configfile/parse_test.go +++ b/internal/configfile/parse_test.go @@ -11,7 +11,7 @@ import ( "testing" "time" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" "github.com/benchttp/cli/internal/configfile" "github.com/benchttp/cli/internal/testutil" @@ -70,8 +70,8 @@ func TestParse(t *testing.T) { for _, tc := range testcases { t.Run(tc.label, func(t *testing.T) { - brunner := runner.Runner{} - gotErr := configfile.Parse(tc.path, &brunner) + runner := benchttp.Runner{} + gotErr := configfile.Parse(tc.path, &runner) if gotErr == nil { t.Fatal("exp non-nil error, got nil") @@ -81,8 +81,8 @@ func TestParse(t *testing.T) { t.Errorf("\nexp %v\ngot %v", tc.expErr, gotErr) } - if !reflect.DeepEqual(brunner, runner.Runner{}) { - t.Errorf("\nexp empty config\ngot %v", brunner) + if !reflect.DeepEqual(runner, benchttp.Runner{}) { + t.Errorf("\nexp empty config\ngot %v", runner) } }) } @@ -93,13 +93,13 @@ func TestParse(t *testing.T) { expCfg := newExpConfig() fname := configPath("valid/benchttp" + ext) - gotCfg := runner.Runner{} + gotCfg := benchttp.Runner{} if err := configfile.Parse(fname, &gotCfg); err != nil { // critical error, stop the test t.Fatal(err) } - if sameConfig(gotCfg, runner.Runner{}) { + if sameConfig(gotCfg, benchttp.Runner{}) { t.Error("received an empty configuration") } @@ -111,13 +111,13 @@ func TestParse(t *testing.T) { }) t.Run("override input config", func(t *testing.T) { - brunner := runner.Runner{} - brunner.Request = testutil.MustMakeRequest("POST", "https://overriden.com", nil, nil) - brunner.GlobalTimeout = 10 * time.Millisecond + runner := benchttp.Runner{} + runner.Request = testutil.MustMakeRequest("POST", "https://overriden.com", nil, nil) + runner.GlobalTimeout = 10 * time.Millisecond fname := configPath("valid/benchttp-zeros.yml") - if err := configfile.Parse(fname, &brunner); err != nil { + if err := configfile.Parse(fname, &runner); err != nil { t.Fatal(err) } @@ -126,7 +126,7 @@ func TestParse(t *testing.T) { expGlobalTimeout = 42 * time.Millisecond // from read file ) - if gotMethod := brunner.Request.Method; gotMethod != expMethod { + if gotMethod := runner.Request.Method; gotMethod != expMethod { t.Errorf( "did not keep input values that are not set: "+ "exp Request.Method == %s, got %s", @@ -134,7 +134,7 @@ func TestParse(t *testing.T) { ) } - if gotGlobalTimeout := brunner.GlobalTimeout; gotGlobalTimeout != expGlobalTimeout { + if gotGlobalTimeout := runner.GlobalTimeout; gotGlobalTimeout != expGlobalTimeout { t.Errorf( "did not override input values that are set: "+ "exp Runner.GlobalTimeout == %v, got %v", @@ -142,7 +142,7 @@ func TestParse(t *testing.T) { ) } - t.Log(brunner) + t.Log(runner) }) t.Run("extend config files", func(t *testing.T) { @@ -165,8 +165,8 @@ func TestParse(t *testing.T) { for _, tc := range testcases { t.Run(tc.label, func(t *testing.T) { - var brunner runner.Runner - if err := configfile.Parse(tc.cfpath, &brunner); err != nil { + var runner benchttp.Runner + if err := configfile.Parse(tc.cfpath, &runner); err != nil { t.Fatal(err) } @@ -175,11 +175,11 @@ func TestParse(t *testing.T) { expURL = fmt.Sprintf("http://%s.config", tc.cfname) ) - if gotMethod := brunner.Request.Method; gotMethod != expMethod { + if gotMethod := runner.Request.Method; gotMethod != expMethod { t.Errorf("method: exp %s, got %s", expMethod, gotMethod) } - if gotURL := brunner.Request.URL.String(); gotURL != expURL { + if gotURL := runner.Request.URL.String(); gotURL != expURL { t.Errorf("url: exp %s, got %s", expURL, gotURL) } }) @@ -191,8 +191,8 @@ func TestParse(t *testing.T) { // newExpConfig returns the expected runner.ConfigConfig result after parsing // one of the config files in testdataConfigPath. -func newExpConfig() runner.Runner { - return runner.Runner{ +func newExpConfig() benchttp.Runner { + return benchttp.Runner{ Request: testutil.MustMakeRequest( "POST", validURL, @@ -209,7 +209,7 @@ func newExpConfig() runner.Runner { RequestTimeout: 2 * time.Second, GlobalTimeout: 60 * time.Second, - Tests: []runner.TestCase{ + Tests: []benchttp.TestCase{ { Name: "minimum response time", Field: "ResponseTimes.Min", @@ -232,7 +232,7 @@ func newExpConfig() runner.Runner { } } -func sameConfig(a, b runner.Runner) bool { +func sameConfig(a, b benchttp.Runner) bool { if a.Request == nil || b.Request == nil { return a.Request == nil && b.Request == nil } diff --git a/internal/configfile/parser.go b/internal/configfile/parser.go index c50876e..943fc1e 100644 --- a/internal/configfile/parser.go +++ b/internal/configfile/parser.go @@ -3,7 +3,7 @@ package configfile import ( "errors" - "github.com/benchttp/engine/configparse" + "github.com/benchttp/sdk/configparse" ) type extension string diff --git a/internal/configflag/bind.go b/internal/configflag/bind.go index 586ab23..5e59343 100644 --- a/internal/configflag/bind.go +++ b/internal/configflag/bind.go @@ -7,7 +7,7 @@ import ( "strconv" "strings" - "github.com/benchttp/engine/configparse" + "github.com/benchttp/sdk/configparse" ) // Bind reads arguments provided to flagset as config fields diff --git a/internal/configflag/bind_test.go b/internal/configflag/bind_test.go index 6e06a06..cbab44a 100644 --- a/internal/configflag/bind_test.go +++ b/internal/configflag/bind_test.go @@ -5,7 +5,7 @@ import ( "reflect" "testing" - "github.com/benchttp/engine/configparse" + "github.com/benchttp/sdk/configparse" "github.com/benchttp/cli/internal/configflag" ) diff --git a/internal/render/progress.go b/internal/render/progress.go index 2495e88..22a526e 100644 --- a/internal/render/progress.go +++ b/internal/render/progress.go @@ -6,14 +6,14 @@ import ( "strconv" "strings" - "github.com/benchttp/engine/runner" + benchttp "github.com/benchttp/sdk/benchttp" "github.com/benchttp/cli/internal/render/ansi" ) // Progress renders a fancy representation of a runner.RecordingProgress // and writes the result to w. -func Progress(w io.Writer, p runner.RecordingProgress) (int, error) { +func Progress(w io.Writer, p benchttp.RecordingProgress) (int, error) { return fmt.Fprint(w, progressString(p)) } @@ -21,7 +21,7 @@ func Progress(w io.Writer, p runner.RecordingProgress) (int, error) { // for a fancy display in a CLI: // // RUNNING ◼︎◼︎◼︎◼︎◼︎◼︎◼︎◼︎◼︎◼︎ 50% | 50/100 requests | 27s timeout -func progressString(p runner.RecordingProgress) string { +func progressString(p benchttp.RecordingProgress) string { var ( countdown = p.Timeout - p.Elapsed reqmax = strconv.Itoa(p.MaxCount) @@ -68,20 +68,20 @@ func renderTimeline(pctdone int) string { // renderStatus returns a string representing the status, // depending on whether the run is done or not and the value // of its context error. -func renderStatus(status runner.RecordingStatus) string { +func renderStatus(status benchttp.RecordingStatus) string { styled := statusStyle(status) return styled(string(status)) } -func statusStyle(status runner.RecordingStatus) ansi.StyleFunc { +func statusStyle(status benchttp.RecordingStatus) ansi.StyleFunc { switch status { - case runner.StatusRunning: + case benchttp.StatusRunning: return ansi.Yellow - case runner.StatusDone: + case benchttp.StatusDone: return ansi.Green - case runner.StatusCanceled: + case benchttp.StatusCanceled: return ansi.Red - case runner.StatusTimeout: + case benchttp.StatusTimeout: return ansi.Cyan } return ansi.Grey // should not occur diff --git a/internal/render/report.go b/internal/render/report.go index 4d16517..cf688a9 100644 --- a/internal/render/report.go +++ b/internal/render/report.go @@ -7,17 +7,17 @@ import ( "strings" "time" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" "github.com/benchttp/cli/internal/render/ansi" ) -func ReportSummary(w io.Writer, rep *runner.Report) (int, error) { +func ReportSummary(w io.Writer, rep *benchttp.Report) (int, error) { return w.Write([]byte(ReportSummaryString(rep))) } // String returns a default summary of the Report as a string. -func ReportSummaryString(rep *runner.Report) string { +func ReportSummaryString(rep *benchttp.Report) string { var b strings.Builder line := func(name string, value interface{}) string { diff --git a/internal/render/report_test.go b/internal/render/report_test.go index a8bc5f3..993348d 100644 --- a/internal/render/report_test.go +++ b/internal/render/report_test.go @@ -5,7 +5,7 @@ import ( "testing" "time" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" "github.com/benchttp/cli/internal/render" "github.com/benchttp/cli/internal/render/ansi" @@ -16,7 +16,7 @@ func TestReport_String(t *testing.T) { metrics, duration := metricsStub() runner := runnerStub() - rep := &runner.Report{ + rep := &benchttp.Report{ Metrics: metrics, Metadata: benchttp.Metadata{ Runner: runner, @@ -29,13 +29,13 @@ func TestReport_String(t *testing.T) { // helpers -func metricsStub() (agg runner.MetricsAggregate, total time.Duration) { - return runner.MetricsAggregate{ +func metricsStub() (agg benchttp.MetricsAggregate, total time.Duration) { + return benchttp.MetricsAggregate{ RequestFailures: make([]struct { Reason string }, 1), Records: make([]struct{ ResponseTime time.Duration }, 3), - ResponseTimes: runner.MetricsTimeStats{ + ResponseTimes: benchttp.MetricsTimeStats{ Min: 4 * time.Second, Max: 6 * time.Second, Mean: 5 * time.Second, @@ -43,11 +43,11 @@ func metricsStub() (agg runner.MetricsAggregate, total time.Duration) { }, 15 * time.Second } -func runnerStub() runner.Runner { - brunner := runner.Runner{} - brunner.Request = mustMakeRequest("https://a.b.com") - brunner.Requests = -1 - return brunner +func runnerStub() benchttp.Runner { + runner := benchttp.Runner{} + runner.Request = mustMakeRequest("https://a.b.com") + runner.Requests = -1 + return runner } func checkSummary(t *testing.T, summary string) { diff --git a/internal/render/testsuite.go b/internal/render/testsuite.go index 648ad17..d9fca71 100644 --- a/internal/render/testsuite.go +++ b/internal/render/testsuite.go @@ -4,17 +4,17 @@ import ( "io" "strings" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" "github.com/benchttp/cli/internal/render/ansi" ) -func TestSuite(w io.Writer, suite runner.TestSuiteResults) (int, error) { +func TestSuite(w io.Writer, suite benchttp.TestSuiteResults) (int, error) { return w.Write([]byte(TestSuiteString(suite))) } // String returns a default summary of the Report as a string. -func TestSuiteString(suite runner.TestSuiteResults) string { +func TestSuiteString(suite benchttp.TestSuiteResults) string { if len(suite.Results) == 0 { return "" } From 1c3d6085bb1c9d13aa720854c0e0046a270b2fc6 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Thu, 3 Nov 2022 08:40:41 +0100 Subject: [PATCH 06/11] chore: use benchttp/sdk/configio over /configfile --- cmd/benchttp/run.go | 15 +++++++-------- internal/configflag/bind.go | 6 +++--- internal/configflag/bind_test.go | 8 ++++---- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index 3765758..eca8955 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -9,9 +9,8 @@ import ( "os" "github.com/benchttp/sdk/benchttp" - "github.com/benchttp/sdk/configparse" + "github.com/benchttp/sdk/configio" - "github.com/benchttp/cli/internal/configfile" "github.com/benchttp/cli/internal/configflag" "github.com/benchttp/cli/internal/output" "github.com/benchttp/cli/internal/render" @@ -29,7 +28,7 @@ type cmdRun struct { silent bool // configRepr is the runner config resulting from config flag values - configRepr configparse.Representation + configRepr configio.Representation } // execute runs the benchttp runner: it parses CLI flags, loads config @@ -54,7 +53,7 @@ func (cmd *cmdRun) execute(args []string) error { } func (cmd *cmdRun) parseArgs(args []string) error { - cmd.flagset.StringVar(&cmd.configFile, "configFile", configfile.Find(), "Config file path") + cmd.flagset.StringVar(&cmd.configFile, "configFile", configio.FindFile(), "Config file path") cmd.flagset.BoolVar(&cmd.silent, "silent", false, "Silent mode") configflag.Bind(cmd.flagset, &cmd.configRepr) return cmd.flagset.Parse(args) @@ -62,21 +61,21 @@ func (cmd *cmdRun) parseArgs(args []string) error { func buildConfig( filePath string, - cliConfigRepr configparse.Representation, + cliConfigRepr configio.Representation, ) (benchttp.Runner, error) { // start with default runner as base runner := benchttp.DefaultRunner() // override with config file values - err := configfile.Parse(filePath, &runner) - if err != nil && !errors.Is(err, configfile.ErrFileNotFound) { + err := configio.UnmarshalFile(filePath, &runner) + if err != nil && !errors.Is(err, configio.ErrFileNotFound) { // config file is not mandatory: discard ErrFileNotFound. // other errors are critical return runner, err } // override with CLI flags values - if err := cliConfigRepr.ParseInto(&runner); err != nil { + if err := cliConfigRepr.Into(&runner); err != nil { return runner, err } diff --git a/internal/configflag/bind.go b/internal/configflag/bind.go index 5e59343..a45b648 100644 --- a/internal/configflag/bind.go +++ b/internal/configflag/bind.go @@ -7,21 +7,21 @@ import ( "strconv" "strings" - "github.com/benchttp/sdk/configparse" + "github.com/benchttp/sdk/configio" ) // Bind reads arguments provided to flagset as config fields // and binds their value to the appropriate fields of dst. // The provided *flag.Flagset must not have been parsed yet, otherwise // bindings its values would fail. -func Bind(flagset *flag.FlagSet, dst *configparse.Representation) { +func Bind(flagset *flag.FlagSet, dst *configio.Representation) { for field, bind := range bindings { flagset.Func(field, flagsUsage[field], bind(dst)) } } type ( - repr = configparse.Representation + repr = configio.Representation flagSetter = func(string) error ) diff --git a/internal/configflag/bind_test.go b/internal/configflag/bind_test.go index cbab44a..896d5c8 100644 --- a/internal/configflag/bind_test.go +++ b/internal/configflag/bind_test.go @@ -5,7 +5,7 @@ import ( "reflect" "testing" - "github.com/benchttp/sdk/configparse" + "github.com/benchttp/sdk/configio" "github.com/benchttp/cli/internal/configflag" ) @@ -15,13 +15,13 @@ func TestBind(t *testing.T) { flagset := flag.NewFlagSet("", flag.ExitOnError) args := []string{} // no args - repr := configparse.Representation{} + repr := configio.Representation{} configflag.Bind(flagset, &repr) if err := flagset.Parse(args); err != nil { t.Fatal(err) // critical error, stop the test } - exp := configparse.Representation{} + exp := configio.Representation{} if got := repr; !reflect.DeepEqual(got, exp) { t.Errorf("\nexp %#v\ngot %#v", exp, got) } @@ -43,7 +43,7 @@ func TestBind(t *testing.T) { "-globalTimeout", "5s", } - repr := configparse.Representation{} + repr := configio.Representation{} configflag.Bind(flagset, &repr) if err := flagset.Parse(args); err != nil { t.Fatal(err) // critical error, stop the test From 39664152de60c8b73f7c9e626e3b9878ae8253bc Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Thu, 3 Nov 2022 08:40:57 +0100 Subject: [PATCH 07/11] chore: remove package configifile --- internal/configfile/error.go | 26 -- internal/configfile/find.go | 24 -- internal/configfile/find_test.go | 39 --- internal/configfile/parse.go | 116 ------- internal/configfile/parse_test.go | 287 ------------------ internal/configfile/parser.go | 35 --- .../testdata/extends/extends-circular-0.yml | 1 - .../testdata/extends/extends-circular-1.yml | 1 - .../testdata/extends/extends-circular-2.yml | 1 - .../extends/extends-circular-self.yml | 1 - .../testdata/extends/extends-valid-child.yml | 4 - .../testdata/extends/extends-valid-parent.yml | 3 - .../nest-0/nest-1/extends-valid-nested.yml | 4 - .../configfile/testdata/invalid/badext.yams | 2 - .../testdata/invalid/badfields.json | 7 - .../configfile/testdata/invalid/badfields.yml | 4 - .../testdata/valid/benchttp-zeros.yml | 3 - .../configfile/testdata/valid/benchttp.json | 44 --- .../configfile/testdata/valid/benchttp.yaml | 35 --- .../configfile/testdata/valid/benchttp.yml | 32 -- 20 files changed, 669 deletions(-) delete mode 100644 internal/configfile/error.go delete mode 100644 internal/configfile/find.go delete mode 100644 internal/configfile/find_test.go delete mode 100644 internal/configfile/parse.go delete mode 100644 internal/configfile/parse_test.go delete mode 100644 internal/configfile/parser.go delete mode 100644 internal/configfile/testdata/extends/extends-circular-0.yml delete mode 100644 internal/configfile/testdata/extends/extends-circular-1.yml delete mode 100644 internal/configfile/testdata/extends/extends-circular-2.yml delete mode 100644 internal/configfile/testdata/extends/extends-circular-self.yml delete mode 100644 internal/configfile/testdata/extends/extends-valid-child.yml delete mode 100644 internal/configfile/testdata/extends/extends-valid-parent.yml delete mode 100644 internal/configfile/testdata/extends/nest-0/nest-1/extends-valid-nested.yml delete mode 100644 internal/configfile/testdata/invalid/badext.yams delete mode 100644 internal/configfile/testdata/invalid/badfields.json delete mode 100644 internal/configfile/testdata/invalid/badfields.yml delete mode 100644 internal/configfile/testdata/valid/benchttp-zeros.yml delete mode 100644 internal/configfile/testdata/valid/benchttp.json delete mode 100644 internal/configfile/testdata/valid/benchttp.yaml delete mode 100644 internal/configfile/testdata/valid/benchttp.yml diff --git a/internal/configfile/error.go b/internal/configfile/error.go deleted file mode 100644 index c33a7d2..0000000 --- a/internal/configfile/error.go +++ /dev/null @@ -1,26 +0,0 @@ -package configfile - -import ( - "errors" -) - -var ( - // ErrFileNotFound signals a config file not found. - ErrFileNotFound = errors.New("file not found") - - // ErrFileRead signals an error trying to read a config file. - // It can be due to a corrupted file or an invalid permission - // for instance. - ErrFileRead = errors.New("invalid file") - - // ErrFileExt signals an unsupported extension for the config file. - ErrFileExt = errors.New("invalid extension") - - // ErrFileParse signals an error parsing a retrieved config file. - // It is returned if it contains an unexpected field or an unexpected - // value for a field. - ErrFileParse = errors.New("parsing error: invalid config file") - - // ErrCircularExtends signals a circular reference in the config file. - ErrCircularExtends = errors.New("circular reference detected") -) diff --git a/internal/configfile/find.go b/internal/configfile/find.go deleted file mode 100644 index 797052e..0000000 --- a/internal/configfile/find.go +++ /dev/null @@ -1,24 +0,0 @@ -package configfile - -import "os" - -var DefaultPaths = []string{ - "./.benchttp.yml", - "./.benchttp.yaml", - "./.benchttp.json", -} - -// Find returns the first name tham matches a file path. -// If input paths is empty, it uses DefaultPaths. -// If no match is found, it returns an empty string. -func Find(paths ...string) string { - if len(paths) == 0 { - paths = DefaultPaths - } - for _, path := range paths { - if _, err := os.Stat(path); err == nil { // err IS nil: file exists - return path - } - } - return "" -} diff --git a/internal/configfile/find_test.go b/internal/configfile/find_test.go deleted file mode 100644 index 08d8f38..0000000 --- a/internal/configfile/find_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package configfile_test - -import ( - "testing" - - "github.com/benchttp/cli/internal/configfile" -) - -var ( - goodFileYML = configPath("valid/benchttp.yml") - goodFileJSON = configPath("valid/benchttp.json") - badFile = configPath("does-not-exist.json") -) - -func TestFind(t *testing.T) { - t.Run("return first existing file form input", func(t *testing.T) { - files := []string{badFile, goodFileYML, goodFileJSON} - - if got := configfile.Find(files...); got != goodFileYML { - t.Errorf("did not retrieve good file: exp %s, got %s", goodFileYML, got) - } - }) - - t.Run("return first existing file form defaults", func(t *testing.T) { - configfile.DefaultPaths = []string{badFile, goodFileYML, goodFileJSON} - - if got := configfile.Find(); got != goodFileYML { - t.Errorf("did not retrieve good file: exp %s, got %s", goodFileYML, got) - } - }) - - t.Run("return empty string when no match", func(t *testing.T) { - files := []string{badFile} - - if got := configfile.Find(files...); got != "" { - t.Errorf("retrieved unexpected file: %s", got) - } - }) -} diff --git a/internal/configfile/parse.go b/internal/configfile/parse.go deleted file mode 100644 index 0120481..0000000 --- a/internal/configfile/parse.go +++ /dev/null @@ -1,116 +0,0 @@ -package configfile - -import ( - "errors" - "os" - "path/filepath" - - "github.com/benchttp/sdk/benchttp" - "github.com/benchttp/sdk/configparse" - - "github.com/benchttp/cli/internal/errorutil" -) - -// Parse parses given filename as a benchttp runner configuration -// into a runner.Runner and stores the retrieved values into *dst. -// It returns the first error occurring in the process, which can be -// any of the values declared in the package. -func Parse(filename string, dst *benchttp.Runner) (err error) { - reprs, err := parseFileRecursive(filename, []configparse.Representation{}, set{}) - if err != nil { - return - } - return parseAndMergeConfigs(reprs, dst) -} - -// set is a collection of unique string values. -type set map[string]bool - -// add adds v to the receiver. If v is already set, it returns a non-nil -// error instead. -func (s set) add(v string) error { - if _, exists := s[v]; exists { - return errors.New("value already set") - } - s[v] = true - return nil -} - -// parseFileRecursive parses a config file and its parent found from key -// "extends" recursively until the root config file is reached. -// It returns the list of all parsed configs or the first non-nil error -// occurring in the process. -func parseFileRecursive( - filename string, - reprs []configparse.Representation, - seen set, -) ([]configparse.Representation, error) { - // avoid infinite recursion caused by circular reference - if err := seen.add(filename); err != nil { - return reprs, ErrCircularExtends - } - - // parse current file, append parsed config - repr, err := parseFile(filename) - if err != nil { - return reprs, err - } - reprs = append(reprs, repr) - - // root config reached: stop now and return the parsed configs - if repr.Extends == nil { - return reprs, nil - } - - // config has parent: resolve its path and parse it recursively - parentPath := filepath.Join(filepath.Dir(filename), *repr.Extends) - return parseFileRecursive(parentPath, reprs, seen) -} - -// parseFile parses a single config file and returns the result as an -// configparse.Representation and an appropriate error predeclared in the package. -func parseFile(filename string) (repr configparse.Representation, err error) { - b, err := os.ReadFile(filename) - switch { - case err == nil: - case errors.Is(err, os.ErrNotExist): - return repr, errorutil.WithDetails(ErrFileNotFound, filename) - default: - return repr, errorutil.WithDetails(ErrFileRead, filename, err) - } - - ext := extension(filepath.Ext(filename)) - parser, err := newParser(ext) - if err != nil { - return repr, errorutil.WithDetails(ErrFileExt, ext, err) - } - - if err = parser.Parse(b, &repr); err != nil { - return repr, errorutil.WithDetails(ErrFileParse, filename, err) - } - - return repr, nil -} - -// parseAndMergeConfigs iterates backwards over reprs, parses them as -// runner.Runner, merges them successively and finally stores the result -// into dst. -// It returns the merged result or the first non-nil error occurring in the -// process. -func parseAndMergeConfigs(reprs []configparse.Representation, dst *benchttp.Runner) error { - if len(reprs) == 0 { // supposedly catched upstream, should not occur - return errors.New( - "an unacceptable error occurred parsing the config file, " + - "please visit https://github.com/benchttp/runner/issues/new " + - "and insult us properly", - ) - } - - for i := len(reprs) - 1; i >= 0; i-- { - if err := reprs[i].ParseInto(dst); err != nil { - return errorutil.WithDetails(ErrFileParse, err) - } - } - - return nil -} diff --git a/internal/configfile/parse_test.go b/internal/configfile/parse_test.go deleted file mode 100644 index 3720544..0000000 --- a/internal/configfile/parse_test.go +++ /dev/null @@ -1,287 +0,0 @@ -package configfile_test - -import ( - "errors" - "fmt" - "io" - "net/http" - "net/url" - "path/filepath" - "reflect" - "testing" - "time" - - "github.com/benchttp/sdk/benchttp" - - "github.com/benchttp/cli/internal/configfile" - "github.com/benchttp/cli/internal/testutil" -) - -const ( - validConfigPath = "./testdata" - validURL = "http://localhost:9999?fib=30&delay=200ms" // value from testdata files -) - -var supportedExt = []string{ - ".yml", - ".yaml", - ".json", -} - -// TestParse ensures the config file is open, read, and correctly parsed. -func TestParse(t *testing.T) { - t.Run("return file errors early", func(t *testing.T) { - testcases := []struct { - label string - path string - expErr error - }{ - { - label: "not found", - path: configPath("invalid/bad path"), - expErr: configfile.ErrFileNotFound, - }, - { - label: "unsupported extension", - path: configPath("invalid/badext.yams"), - expErr: configfile.ErrFileExt, - }, - { - label: "yaml invalid fields", - path: configPath("invalid/badfields.yml"), - expErr: configfile.ErrFileParse, - }, - { - label: "json invalid fields", - path: configPath("invalid/badfields.json"), - expErr: configfile.ErrFileParse, - }, - { - label: "self reference", - path: configPath("extends/extends-circular-self.yml"), - expErr: configfile.ErrCircularExtends, - }, - { - label: "circular reference", - path: configPath("extends/extends-circular-0.yml"), - expErr: configfile.ErrCircularExtends, - }, - } - - for _, tc := range testcases { - t.Run(tc.label, func(t *testing.T) { - runner := benchttp.Runner{} - gotErr := configfile.Parse(tc.path, &runner) - - if gotErr == nil { - t.Fatal("exp non-nil error, got nil") - } - - if !errors.Is(gotErr, tc.expErr) { - t.Errorf("\nexp %v\ngot %v", tc.expErr, gotErr) - } - - if !reflect.DeepEqual(runner, benchttp.Runner{}) { - t.Errorf("\nexp empty config\ngot %v", runner) - } - }) - } - }) - - t.Run("happy path for all extensions", func(t *testing.T) { - for _, ext := range supportedExt { - expCfg := newExpConfig() - fname := configPath("valid/benchttp" + ext) - - gotCfg := benchttp.Runner{} - if err := configfile.Parse(fname, &gotCfg); err != nil { - // critical error, stop the test - t.Fatal(err) - } - - if sameConfig(gotCfg, benchttp.Runner{}) { - t.Error("received an empty configuration") - } - - if !sameConfig(gotCfg, expCfg) { - t.Errorf("unexpected parsed config for %s file:\nexp %#v\ngot %#v", ext, expCfg, gotCfg) - } - - } - }) - - t.Run("override input config", func(t *testing.T) { - runner := benchttp.Runner{} - runner.Request = testutil.MustMakeRequest("POST", "https://overriden.com", nil, nil) - runner.GlobalTimeout = 10 * time.Millisecond - - fname := configPath("valid/benchttp-zeros.yml") - - if err := configfile.Parse(fname, &runner); err != nil { - t.Fatal(err) - } - - const ( - expMethod = "POST" // from input config - expGlobalTimeout = 42 * time.Millisecond // from read file - ) - - if gotMethod := runner.Request.Method; gotMethod != expMethod { - t.Errorf( - "did not keep input values that are not set: "+ - "exp Request.Method == %s, got %s", - expMethod, gotMethod, - ) - } - - if gotGlobalTimeout := runner.GlobalTimeout; gotGlobalTimeout != expGlobalTimeout { - t.Errorf( - "did not override input values that are set: "+ - "exp Runner.GlobalTimeout == %v, got %v", - expGlobalTimeout, gotGlobalTimeout, - ) - } - - t.Log(runner) - }) - - t.Run("extend config files", func(t *testing.T) { - testcases := []struct { - label string - cfname string - cfpath string - }{ - { - label: "same directory", - cfname: "child", - cfpath: configPath("extends/extends-valid-child.yml"), - }, - { - label: "nested directory", - cfname: "nested", - cfpath: configPath("extends/nest-0/nest-1/extends-valid-nested.yml"), - }, - } - - for _, tc := range testcases { - t.Run(tc.label, func(t *testing.T) { - var runner benchttp.Runner - if err := configfile.Parse(tc.cfpath, &runner); err != nil { - t.Fatal(err) - } - - var ( - expMethod = "POST" - expURL = fmt.Sprintf("http://%s.config", tc.cfname) - ) - - if gotMethod := runner.Request.Method; gotMethod != expMethod { - t.Errorf("method: exp %s, got %s", expMethod, gotMethod) - } - - if gotURL := runner.Request.URL.String(); gotURL != expURL { - t.Errorf("url: exp %s, got %s", expURL, gotURL) - } - }) - } - }) -} - -// helpers - -// newExpConfig returns the expected runner.ConfigConfig result after parsing -// one of the config files in testdataConfigPath. -func newExpConfig() benchttp.Runner { - return benchttp.Runner{ - Request: testutil.MustMakeRequest( - "POST", - validURL, - http.Header{ - "key0": []string{"val0", "val1"}, - "key1": []string{"val0"}, - }, - []byte(`{"key0":"val0","key1":"val1"}`), - ), - - Requests: 100, - Concurrency: 1, - Interval: 50 * time.Millisecond, - RequestTimeout: 2 * time.Second, - GlobalTimeout: 60 * time.Second, - - Tests: []benchttp.TestCase{ - { - Name: "minimum response time", - Field: "ResponseTimes.Min", - Predicate: "GT", - Target: 80 * time.Millisecond, - }, - { - Name: "maximum response time", - Field: "ResponseTimes.Max", - Predicate: "LTE", - Target: 120 * time.Millisecond, - }, - { - Name: "100% availability", - Field: "RequestFailureCount", - Predicate: "EQ", - Target: 0, - }, - }, - } -} - -func sameConfig(a, b benchttp.Runner) bool { - if a.Request == nil || b.Request == nil { - return a.Request == nil && b.Request == nil - } - return sameURL(a.Request.URL, b.Request.URL) && - sameHeader(a.Request.Header, b.Request.Header) && - sameBody(a.Request.Body, b.Request.Body) -} - -// sameURL returns true if a and b are the same *url.URL, taking into account -// the undeterministic nature of their RawQuery. -func sameURL(a, b *url.URL) bool { - // check query params equality via Query() rather than RawQuery - if !reflect.DeepEqual(a.Query(), b.Query()) { - return false - } - - // temporarily set RawQuery to a determined value - for _, u := range []*url.URL{a, b} { - defer setTempValue(&u.RawQuery, "replaced by test")() - } - - // we can now rely on deep equality check - return reflect.DeepEqual(a, b) -} - -func sameHeader(a, b http.Header) bool { - return reflect.DeepEqual(a, b) - // if len(a) != len(b) { - // return false - // } - // for k, values := range a { - // if len(values) != len() - // } -} - -func sameBody(a, b io.ReadCloser) bool { - return reflect.DeepEqual(a, b) -} - -// setTempValue sets *ptr to val and returns a restore func that sets *ptr -// back to its previous value. -func setTempValue(ptr *string, val string) (restore func()) { - previousValue := *ptr - *ptr = val - return func() { - *ptr = previousValue - } -} - -func configPath(name string) string { - return filepath.Join(validConfigPath, name) -} diff --git a/internal/configfile/parser.go b/internal/configfile/parser.go deleted file mode 100644 index 943fc1e..0000000 --- a/internal/configfile/parser.go +++ /dev/null @@ -1,35 +0,0 @@ -package configfile - -import ( - "errors" - - "github.com/benchttp/sdk/configparse" -) - -type extension string - -const ( - extYML extension = ".yml" - extYAML extension = ".yaml" - extJSON extension = ".json" -) - -// configParser exposes a method parse to read bytes as a raw config. -type configParser interface { - // parse parses a raw bytes input as a raw config and stores - // the resulting value into dst. - Parse(in []byte, dst *configparse.Representation) error -} - -// newParser returns an appropriate parser according to ext, or a non-nil -// error if ext is not an expected extension. -func newParser(ext extension) (configParser, error) { - switch ext { - case extYML, extYAML: - return configparse.YAMLParser{}, nil - case extJSON: - return configparse.JSONParser{}, nil - default: - return nil, errors.New("unsupported config format") - } -} diff --git a/internal/configfile/testdata/extends/extends-circular-0.yml b/internal/configfile/testdata/extends/extends-circular-0.yml deleted file mode 100644 index 55c7ac7..0000000 --- a/internal/configfile/testdata/extends/extends-circular-0.yml +++ /dev/null @@ -1 +0,0 @@ -extends: ./extends-circular-1.yml diff --git a/internal/configfile/testdata/extends/extends-circular-1.yml b/internal/configfile/testdata/extends/extends-circular-1.yml deleted file mode 100644 index f451260..0000000 --- a/internal/configfile/testdata/extends/extends-circular-1.yml +++ /dev/null @@ -1 +0,0 @@ -extends: ./extends-circular-2.yml diff --git a/internal/configfile/testdata/extends/extends-circular-2.yml b/internal/configfile/testdata/extends/extends-circular-2.yml deleted file mode 100644 index b862fa4..0000000 --- a/internal/configfile/testdata/extends/extends-circular-2.yml +++ /dev/null @@ -1 +0,0 @@ -extends: ./extends-circular-0.yml diff --git a/internal/configfile/testdata/extends/extends-circular-self.yml b/internal/configfile/testdata/extends/extends-circular-self.yml deleted file mode 100644 index 2fa66ac..0000000 --- a/internal/configfile/testdata/extends/extends-circular-self.yml +++ /dev/null @@ -1 +0,0 @@ -extends: ./extends-circular-self.yml diff --git a/internal/configfile/testdata/extends/extends-valid-child.yml b/internal/configfile/testdata/extends/extends-valid-child.yml deleted file mode 100644 index a344080..0000000 --- a/internal/configfile/testdata/extends/extends-valid-child.yml +++ /dev/null @@ -1,4 +0,0 @@ -extends: ./extends-valid-parent.yml - -request: - url: http://child.config diff --git a/internal/configfile/testdata/extends/extends-valid-parent.yml b/internal/configfile/testdata/extends/extends-valid-parent.yml deleted file mode 100644 index 7f3b136..0000000 --- a/internal/configfile/testdata/extends/extends-valid-parent.yml +++ /dev/null @@ -1,3 +0,0 @@ -request: - method: POST - url: http://parent.config diff --git a/internal/configfile/testdata/extends/nest-0/nest-1/extends-valid-nested.yml b/internal/configfile/testdata/extends/nest-0/nest-1/extends-valid-nested.yml deleted file mode 100644 index 7810890..0000000 --- a/internal/configfile/testdata/extends/nest-0/nest-1/extends-valid-nested.yml +++ /dev/null @@ -1,4 +0,0 @@ -extends: ../../extends-valid-parent.yml - -request: - url: http://nested.config diff --git a/internal/configfile/testdata/invalid/badext.yams b/internal/configfile/testdata/invalid/badext.yams deleted file mode 100644 index f7aba57..0000000 --- a/internal/configfile/testdata/invalid/badext.yams +++ /dev/null @@ -1,2 +0,0 @@ -request: - url: https://benchttp.app diff --git a/internal/configfile/testdata/invalid/badfields.json b/internal/configfile/testdata/invalid/badfields.json deleted file mode 100644 index 9b45468..0000000 --- a/internal/configfile/testdata/invalid/badfields.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "runner": { - "requests": [123], - "concurrency": "123" - }, - "notafield": 123 -} diff --git a/internal/configfile/testdata/invalid/badfields.yml b/internal/configfile/testdata/invalid/badfields.yml deleted file mode 100644 index 6899cd8..0000000 --- a/internal/configfile/testdata/invalid/badfields.yml +++ /dev/null @@ -1,4 +0,0 @@ -runner: - requests: [123] # error: invalid type for field requests - concurrency: "123" # error: invalid type for field concurrency -notafield: 123 # error: field does not exist diff --git a/internal/configfile/testdata/valid/benchttp-zeros.yml b/internal/configfile/testdata/valid/benchttp-zeros.yml deleted file mode 100644 index 38b1bdb..0000000 --- a/internal/configfile/testdata/valid/benchttp-zeros.yml +++ /dev/null @@ -1,3 +0,0 @@ -runner: - requests: 0 - globalTimeout: 42ms diff --git a/internal/configfile/testdata/valid/benchttp.json b/internal/configfile/testdata/valid/benchttp.json deleted file mode 100644 index 714506f..0000000 --- a/internal/configfile/testdata/valid/benchttp.json +++ /dev/null @@ -1,44 +0,0 @@ -{ - "request": { - "method": "POST", - "url": "http://localhost:9999?delay=200ms", - "queryParams": { - "fib": "30" - }, - "header": { - "key0": ["val0", "val1"], - "key1": ["val0"] - }, - "body": { - "type": "raw", - "content": "{\"key0\":\"val0\",\"key1\":\"val1\"}" - } - }, - "runner": { - "requests": 100, - "concurrency": 1, - "interval": "50ms", - "requestTimeout": "2s", - "globalTimeout": "60s" - }, - "tests": [ - { - "name": "minimum response time", - "field": "ResponseTimes.Min", - "predicate": "GT", - "target": "80ms" - }, - { - "name": "maximum response time", - "field": "ResponseTimes.Max", - "predicate": "LTE", - "target": "120ms" - }, - { - "name": "100% availability", - "field": "RequestFailureCount", - "predicate": "EQ", - "target": "0" - } - ] -} diff --git a/internal/configfile/testdata/valid/benchttp.yaml b/internal/configfile/testdata/valid/benchttp.yaml deleted file mode 100644 index 2ee790c..0000000 --- a/internal/configfile/testdata/valid/benchttp.yaml +++ /dev/null @@ -1,35 +0,0 @@ -x-custom: &data - method: POST - url: http://localhost:9999?delay=200ms - -request: - <<: *data - queryParams: - fib: 30 - header: - key0: [val0, val1] - key1: [val0] - body: - type: raw - content: '{"key0":"val0","key1":"val1"}' - -runner: - requests: 100 - concurrency: 1 - interval: 50ms - requestTimeout: 2s - globalTimeout: 60s - -tests: - - name: minimum response time - field: ResponseTimes.Min - predicate: GT - target: 80ms - - name: maximum response time - field: ResponseTimes.Max - predicate: LTE - target: 120ms - - name: 100% availability - field: RequestFailureCount - predicate: EQ - target: 0 diff --git a/internal/configfile/testdata/valid/benchttp.yml b/internal/configfile/testdata/valid/benchttp.yml deleted file mode 100644 index 27a2fc9..0000000 --- a/internal/configfile/testdata/valid/benchttp.yml +++ /dev/null @@ -1,32 +0,0 @@ -request: - method: POST - url: http://localhost:9999?delay=200ms - queryParams: - fib: 30 - header: - key0: [val0, val1] - key1: [val0] - body: - type: raw - content: '{"key0":"val0","key1":"val1"}' - -runner: - requests: 100 - concurrency: 1 - interval: 50ms - requestTimeout: 2s - globalTimeout: 60s - -tests: - - name: minimum response time - field: ResponseTimes.Min - predicate: GT - target: 80ms - - name: maximum response time - field: ResponseTimes.Max - predicate: LTE - target: 120ms - - name: 100% availability - field: RequestFailureCount - predicate: EQ - target: 0 From 6777f4ad6e8138d4bd65491083880af585661e0c Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 5 Nov 2022 13:03:35 +0100 Subject: [PATCH 08/11] refactor: use configio.Builder --- cmd/benchttp/run.go | 22 ++++------ internal/configflag/bind.go | 81 ++++++++++++++++++++++--------------- 2 files changed, 56 insertions(+), 47 deletions(-) diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index eca8955..7f7a447 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -21,14 +21,10 @@ import ( type cmdRun struct { flagset *flag.FlagSet - // configFile is the parsed value for flag -configFile - configFile string + configFile string // parsed value for flag -configFile + silent bool // parsed value for flag -silent - // silent is the parsed value for flag -silent - silent bool - - // configRepr is the runner config resulting from config flag values - configRepr configio.Representation + builder configio.Builder } // execute runs the benchttp runner: it parses CLI flags, loads config @@ -39,7 +35,7 @@ func (cmd *cmdRun) execute(args []string) error { return err } - config, err := buildConfig(cmd.configFile, cmd.configRepr) + config, err := buildConfig(cmd.builder, cmd.configFile) if err != nil { return err } @@ -55,15 +51,15 @@ func (cmd *cmdRun) execute(args []string) error { func (cmd *cmdRun) parseArgs(args []string) error { cmd.flagset.StringVar(&cmd.configFile, "configFile", configio.FindFile(), "Config file path") cmd.flagset.BoolVar(&cmd.silent, "silent", false, "Silent mode") - configflag.Bind(cmd.flagset, &cmd.configRepr) + configflag.Bind(cmd.flagset, &cmd.builder) return cmd.flagset.Parse(args) } func buildConfig( + b configio.Builder, filePath string, - cliConfigRepr configio.Representation, ) (benchttp.Runner, error) { - // start with default runner as base + // use default runner as a base runner := benchttp.DefaultRunner() // override with config file values @@ -75,9 +71,7 @@ func buildConfig( } // override with CLI flags values - if err := cliConfigRepr.Into(&runner); err != nil { - return runner, err - } + b.Mutate(&runner) return runner, nil } diff --git a/internal/configflag/bind.go b/internal/configflag/bind.go index a45b648..2793c0d 100644 --- a/internal/configflag/bind.go +++ b/internal/configflag/bind.go @@ -1,11 +1,16 @@ package configflag import ( + "bytes" "errors" "flag" "fmt" + "io" + "net/http" + "net/url" "strconv" "strings" + "time" "github.com/benchttp/sdk/configio" ) @@ -14,45 +19,49 @@ import ( // and binds their value to the appropriate fields of dst. // The provided *flag.Flagset must not have been parsed yet, otherwise // bindings its values would fail. -func Bind(flagset *flag.FlagSet, dst *configio.Representation) { +func Bind(flagset *flag.FlagSet, dst *configio.Builder) { for field, bind := range bindings { flagset.Func(field, flagsUsage[field], bind(dst)) } } -type ( - repr = configio.Representation - flagSetter = func(string) error -) +type setter = func(string) error -var bindings = map[string]func(*repr) flagSetter{ - flagMethod: func(dst *repr) flagSetter { +var bindings = map[string]func(*configio.Builder) setter{ + flagMethod: func(b *configio.Builder) setter { return func(in string) error { - dst.Request.Method = &in + b.SetRequestMethod(in) return nil } }, - flagURL: func(dst *repr) flagSetter { + flagURL: func(b *configio.Builder) setter { return func(in string) error { - dst.Request.URL = &in + u, err := url.ParseRequestURI(in) + if err != nil { + return err + } + b.SetRequestURL(u) return nil } }, - flagHeader: func(dst *repr) flagSetter { + flagHeader: func(b *configio.Builder) setter { return func(in string) error { keyval := strings.SplitN(in, ":", 2) if len(keyval) != 2 { return errors.New(`-header: expect format ":"`) } key, val := keyval[0], keyval[1] - if dst.Request.Header == nil { - dst.Request.Header = map[string][]string{} - } - dst.Request.Header[key] = append(dst.Request.Header[key], val) + b.SetRequestHeaderFunc(func(h http.Header) http.Header { + if h == nil { + h = http.Header{} + } + h[key] = append(h[key], val) + return h + }) return nil } }, - flagBody: func(dst *repr) flagSetter { + flagBody: func(b *configio.Builder) setter { return func(in string) error { errFormat := fmt.Errorf(`expect format ":", got %q`, in) if in == "" { @@ -68,13 +77,7 @@ var bindings = map[string]func(*repr) flagSetter{ } switch btype { case "raw": - dst.Request.Body = &struct { - Type string `yaml:"type" json:"type"` - Content string `yaml:"content" json:"content"` - }{ - Type: btype, - Content: bcontent, - } + b.SetRequestBody(io.NopCloser(bytes.NewBufferString(bcontent))) // case "file": // // TODO default: @@ -83,41 +86,53 @@ var bindings = map[string]func(*repr) flagSetter{ return nil } }, - flagRequests: func(dst *repr) flagSetter { + flagRequests: func(b *configio.Builder) setter { return func(in string) error { n, err := strconv.Atoi(in) if err != nil { return err } - dst.Runner.Requests = &n + b.SetRequests(n) return nil } }, - flagConcurrency: func(dst *repr) flagSetter { + flagConcurrency: func(b *configio.Builder) setter { return func(in string) error { n, err := strconv.Atoi(in) if err != nil { return err } - dst.Runner.Concurrency = &n + b.SetConcurrency(n) return nil } }, - flagInterval: func(dst *repr) flagSetter { + flagInterval: func(b *configio.Builder) setter { return func(in string) error { - dst.Runner.Interval = &in + d, err := time.ParseDuration(in) + if err != nil { + return err + } + b.SetInterval(d) return nil } }, - flagRequestTimeout: func(dst *repr) flagSetter { + flagRequestTimeout: func(b *configio.Builder) setter { return func(in string) error { - dst.Runner.RequestTimeout = &in + d, err := time.ParseDuration(in) + if err != nil { + return err + } + b.SetRequestTimeout(d) return nil } }, - flagGlobalTimeout: func(dst *repr) flagSetter { + flagGlobalTimeout: func(b *configio.Builder) setter { return func(in string) error { - dst.Runner.GlobalTimeout = &in + d, err := time.ParseDuration(in) + if err != nil { + return err + } + b.SetGlobalTimeout(d) return nil } }, From af98e2bf7574722091e404390e8012f8a45aa45b Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 5 Nov 2022 13:05:03 +0100 Subject: [PATCH 09/11] test(configflag): refactor tests - use benchttptest.AssertEqualRunners --- go.mod | 1 + go.sum | 13 +----- internal/configflag/bind_test.go | 69 +++++++++++++++++++------------- 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/go.mod b/go.mod index e4e1eb9..ae56c7a 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.17 require github.com/benchttp/sdk v0.1.0 require ( + github.com/google/go-cmp v0.5.9 // indirect golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 281e466..cb4a9ff 100644 --- a/go.sum +++ b/go.sum @@ -1,14 +1,5 @@ -github.com/benchttp/engine v0.0.0-20221008174504-d1162e9ac007 h1:h6ON0tn3i/83eUMoXq0FHxBMLYIA/OIli3m9KYkcxPI= -github.com/benchttp/engine v0.0.0-20221008174504-d1162e9ac007/go.mod h1:FRfUnUjoL1s0aHVGlrxB3pdPAEDLNCnWh6cVOur24hM= -github.com/benchttp/engine v0.1.0 h1:FpQOwHklBITuRd7B/AGKqr0mAmbXgTwzQiPHlhUktbQ= -github.com/benchttp/engine v0.1.0/go.mod h1:FRfUnUjoL1s0aHVGlrxB3pdPAEDLNCnWh6cVOur24hM= -github.com/drykit-go/cond v0.1.0 h1:y7MNxREQLT83vGfcfSKjyFPLC/ZDjYBNp6KuaVVjOg4= -github.com/drykit-go/cond v0.1.0/go.mod h1:7MXBFjjaB5ZCEB8Q4w2euNOaWuTqf7NjOFZAyV1Jpfg= -github.com/drykit-go/strcase v0.2.0/go.mod h1:cWK0/az2f09UPIbJ42Sb8Iqdv01uENrFX+XXKGjPo+8= -github.com/drykit-go/testx v0.1.0/go.mod h1:qGXb49a8CzQ82crBeCVW8R3kGU1KRgWHnI+Q6CNVbz8= -github.com/drykit-go/testx v1.2.0 h1:UsH+tFd24z3Xu+mwvwPY+9eBEg9CUyMsUeMYyUprG0o= -github.com/drykit-go/testx v1.2.0/go.mod h1:qTzXJgnAg8n31woklBzNTaWzLMJrnFk93x/aeaIpc20= -github.com/joho/godotenv v1.4.0/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= +github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= +github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 h1:uVc8UZUe6tr40fFVnUP5Oj+veunVezqYl9z7DYw9xzw= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/internal/configflag/bind_test.go b/internal/configflag/bind_test.go index 896d5c8..a817ad8 100644 --- a/internal/configflag/bind_test.go +++ b/internal/configflag/bind_test.go @@ -1,37 +1,40 @@ package configflag_test import ( + "bytes" "flag" - "reflect" + "io" + "net/http" + "net/url" "testing" + "time" + "github.com/benchttp/sdk/benchttp" + "github.com/benchttp/sdk/benchttptest" "github.com/benchttp/sdk/configio" "github.com/benchttp/cli/internal/configflag" ) func TestBind(t *testing.T) { - t.Run("default to zero representation", func(t *testing.T) { + t.Run("default to zero runner", func(t *testing.T) { flagset := flag.NewFlagSet("", flag.ExitOnError) args := []string{} // no args - repr := configio.Representation{} - configflag.Bind(flagset, &repr) + b := configio.Builder{} + configflag.Bind(flagset, &b) if err := flagset.Parse(args); err != nil { t.Fatal(err) // critical error, stop the test } - exp := configio.Representation{} - if got := repr; !reflect.DeepEqual(got, exp) { - t.Errorf("\nexp %#v\ngot %#v", exp, got) - } + benchttptest.AssertEqualRunners(t, benchttp.Runner{}, b.Runner()) }) t.Run("set config with flags values", func(t *testing.T) { flagset := flag.NewFlagSet("", flag.ExitOnError) args := []string{ "-method", "POST", - "-url", "https://benchttp.app?cool=yes", + "-url", "https://example.com?a=b", "-header", "API_KEY:abc", "-header", "Accept:text/html", "-header", "Accept:application/json", @@ -43,28 +46,38 @@ func TestBind(t *testing.T) { "-globalTimeout", "5s", } - repr := configio.Representation{} - configflag.Bind(flagset, &repr) + b := configio.Builder{} + configflag.Bind(flagset, &b) if err := flagset.Parse(args); err != nil { t.Fatal(err) // critical error, stop the test } - switch { - case *repr.Request.Method != "POST", - *repr.Request.URL != "https://benchttp.app?cool=yes", - repr.Request.Header["API_KEY"][0] != "abc", - repr.Request.Header["Accept"][0] != "text/html", - repr.Request.Header["Accept"][1] != "application/json", - repr.Request.Body.Type != "raw", - repr.Request.Body.Content != "hello", - - *repr.Runner.Requests != 1, - *repr.Runner.Concurrency != 2, - *repr.Runner.Interval != "3s", - *repr.Runner.RequestTimeout != "4s", - *repr.Runner.GlobalTimeout != "5s": - - t.Error("unexpected parsed flags") - } + benchttptest.AssertEqualRunners(t, + benchttp.Runner{ + Request: &http.Request{ + Method: "POST", + URL: mustParseURL("https://example.com?a=b"), + Header: http.Header{ + "API_KEY": []string{"abc"}, + "Accept": []string{"text/html", "application/json"}, + }, + Body: io.NopCloser(bytes.NewBufferString("hello")), + }, + Requests: 1, + Concurrency: 2, + Interval: 3 * time.Second, + RequestTimeout: 4 * time.Second, + GlobalTimeout: 5 * time.Second, + }, + b.Runner(), + ) }) } + +func mustParseURL(v string) *url.URL { + u, err := url.ParseRequestURI(v) + if err != nil { + panic("mustParseURL: " + err.Error()) + } + return u +} From 7028625028ebc7084523719890d88c15ad3d9545 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 1 Apr 2023 10:07:39 +0200 Subject: [PATCH 10/11] chore: benchttp/engine import --- cmd/benchttp/run.go | 4 ++-- go.mod | 2 +- internal/configflag/bind.go | 2 +- internal/configflag/bind_test.go | 6 +++--- internal/render/progress.go | 2 +- internal/render/report.go | 2 +- internal/render/report_test.go | 2 +- internal/render/testsuite.go | 2 +- 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index 7f7a447..6c2bd68 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -8,8 +8,8 @@ import ( "io" "os" - "github.com/benchttp/sdk/benchttp" - "github.com/benchttp/sdk/configio" + "github.com/benchttp/engine/benchttp" + "github.com/benchttp/engine/configio" "github.com/benchttp/cli/internal/configflag" "github.com/benchttp/cli/internal/output" diff --git a/go.mod b/go.mod index ae56c7a..c05d705 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/benchttp/cli go 1.17 -require github.com/benchttp/sdk v0.1.0 +require github.com/benchttp/engine v0.1.0 require ( github.com/google/go-cmp v0.5.9 // indirect diff --git a/internal/configflag/bind.go b/internal/configflag/bind.go index 2793c0d..2cfb717 100644 --- a/internal/configflag/bind.go +++ b/internal/configflag/bind.go @@ -12,7 +12,7 @@ import ( "strings" "time" - "github.com/benchttp/sdk/configio" + "github.com/benchttp/engine/configio" ) // Bind reads arguments provided to flagset as config fields diff --git a/internal/configflag/bind_test.go b/internal/configflag/bind_test.go index a817ad8..f9e9fd4 100644 --- a/internal/configflag/bind_test.go +++ b/internal/configflag/bind_test.go @@ -9,9 +9,9 @@ import ( "testing" "time" - "github.com/benchttp/sdk/benchttp" - "github.com/benchttp/sdk/benchttptest" - "github.com/benchttp/sdk/configio" + "github.com/benchttp/engine/benchttp" + "github.com/benchttp/engine/benchttptest" + "github.com/benchttp/engine/configio" "github.com/benchttp/cli/internal/configflag" ) diff --git a/internal/render/progress.go b/internal/render/progress.go index 22a526e..4f5c673 100644 --- a/internal/render/progress.go +++ b/internal/render/progress.go @@ -6,7 +6,7 @@ import ( "strconv" "strings" - benchttp "github.com/benchttp/sdk/benchttp" + "github.com/benchttp/engine/benchttp" "github.com/benchttp/cli/internal/render/ansi" ) diff --git a/internal/render/report.go b/internal/render/report.go index cf688a9..ced37b7 100644 --- a/internal/render/report.go +++ b/internal/render/report.go @@ -7,7 +7,7 @@ import ( "strings" "time" - "github.com/benchttp/sdk/benchttp" + "github.com/benchttp/engine/benchttp" "github.com/benchttp/cli/internal/render/ansi" ) diff --git a/internal/render/report_test.go b/internal/render/report_test.go index 993348d..3771798 100644 --- a/internal/render/report_test.go +++ b/internal/render/report_test.go @@ -5,7 +5,7 @@ import ( "testing" "time" - "github.com/benchttp/sdk/benchttp" + "github.com/benchttp/engine/benchttp" "github.com/benchttp/cli/internal/render" "github.com/benchttp/cli/internal/render/ansi" diff --git a/internal/render/testsuite.go b/internal/render/testsuite.go index d9fca71..185965c 100644 --- a/internal/render/testsuite.go +++ b/internal/render/testsuite.go @@ -4,7 +4,7 @@ import ( "io" "strings" - "github.com/benchttp/sdk/benchttp" + "github.com/benchttp/engine/benchttp" "github.com/benchttp/cli/internal/render/ansi" ) From 8586996540d147971399bc3d7de86d1edab69052 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 1 Apr 2023 10:47:34 +0200 Subject: [PATCH 11/11] deps: bump benchttp/engine to v0.2.0 --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c05d705..1bd77a2 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/benchttp/cli go 1.17 -require github.com/benchttp/engine v0.1.0 +require github.com/benchttp/engine v0.2.0 require ( github.com/google/go-cmp v0.5.9 // indirect diff --git a/go.sum b/go.sum index cb4a9ff..f5b6e2d 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/benchttp/engine v0.2.0 h1:r3zKs7ZOgRC0uDsHabD76sIoiF57MwGl0ouhFGpbu+A= +github.com/benchttp/engine v0.2.0/go.mod h1:70imLQ2ONTEMGcbJimOirjz57uw3aix9RI8yZVZxLdc= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 h1:uVc8UZUe6tr40fFVnUP5Oj+veunVezqYl9z7DYw9xzw=