From 0c7243bb26e8ea581de56a36c5357f11163c5f80 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 15 Oct 2022 18:19:20 +0200 Subject: [PATCH 01/25] refactor(configparse): implement Representation.Unmarshal - ParseRepresentation -> UnmarshalRepresentation: change signature so it matches json.Unmarshal style - implement Representation.Unmarshal: alias to UnmarshalRepresentation - update JSON accordingly --- configparse/json.go | 9 ++++---- configparse/parse.go | 55 +++++++++++++++++--------------------------- 2 files changed, 25 insertions(+), 39 deletions(-) diff --git a/configparse/json.go b/configparse/json.go index ed052c9..6313a49 100644 --- a/configparse/json.go +++ b/configparse/json.go @@ -7,16 +7,15 @@ import ( // JSON reads input bytes as JSON and unmarshals it into a runner.ConfigGlobal. func JSON(in []byte) (runner.Config, error) { parser := JSONParser{} - - var repr Representation + repr := Representation{} if err := parser.Parse(in, &repr); err != nil { return runner.Config{}, err } - cfg, err := ParseRepresentation(repr) - if err != nil { + cfg := runner.DefaultConfig() + if err := repr.Unmarshal(&cfg); err != nil { return runner.Config{}, err } - return cfg.Override(runner.DefaultConfig()), nil + return cfg, nil } diff --git a/configparse/parse.go b/configparse/parse.go index 66761c7..f735708 100644 --- a/configparse/parse.go +++ b/configparse/parse.go @@ -44,32 +44,27 @@ type Representation struct { } `yaml:"tests" json:"tests"` } -// ParseRepresentation parses an input raw config as a runner.ConfigGlobal and returns -// a parsed Config or the first non-nil error occurring in the process. -func ParseRepresentation(repr Representation) (runner.Config, error) { //nolint:gocognit // acceptable complexity for a parsing func - cfg := runner.Config{} - assignedFields := []string{} - - addField := func(field string) { - assignedFields = append(assignedFields, field) - } - - abort := func(err error) (runner.Config, error) { - return runner.Config{}, err - } +// Unmarshal parses the Representation receiver as a runner.Config +// and stores any non-nil field value into the corresponding field +// of dst. +func (repr Representation) Unmarshal(dst *runner.Config) error { + return UnmarshalRepresentation(repr, dst) +} +// Unmarshal parses the given Representation as a runner.Config +// and stores any non-nil field value into the corresponding field +// of dst. +func UnmarshalRepresentation(repr Representation, cfg *runner.Config) error { //nolint:gocognit // acceptable complexity for a parsing func if method := repr.Request.Method; method != nil { cfg.Request.Method = *method - addField(runner.ConfigFieldMethod) } if rawURL := repr.Request.URL; rawURL != nil { - parsedURL, err := parseAndBuildURL(*repr.Request.URL, repr.Request.QueryParams) + parsedURL, err := parseAndBuildURL(*rawURL, repr.Request.QueryParams) if err != nil { - return abort(err) + return err } cfg.Request.URL = parsedURL - addField(runner.ConfigFieldURL) } if header := repr.Request.Header; header != nil { @@ -78,7 +73,6 @@ func ParseRepresentation(repr Representation) (runner.Config, error) { //nolint: httpHeader[key] = val } cfg.Request.Header = httpHeader - addField(runner.ConfigFieldHeader) } if body := repr.Request.Body; body != nil { @@ -86,49 +80,43 @@ func ParseRepresentation(repr Representation) (runner.Config, error) { //nolint: Type: body.Type, Content: []byte(body.Content), } - addField(runner.ConfigFieldBody) } if requests := repr.Runner.Requests; requests != nil { cfg.Runner.Requests = *requests - addField(runner.ConfigFieldRequests) } if concurrency := repr.Runner.Concurrency; concurrency != nil { cfg.Runner.Concurrency = *concurrency - addField(runner.ConfigFieldConcurrency) } if interval := repr.Runner.Interval; interval != nil { parsedInterval, err := parseOptionalDuration(*interval) if err != nil { - return abort(err) + return err } cfg.Runner.Interval = parsedInterval - addField(runner.ConfigFieldInterval) } if requestTimeout := repr.Runner.RequestTimeout; requestTimeout != nil { parsedTimeout, err := parseOptionalDuration(*requestTimeout) if err != nil { - return abort(err) + return err } cfg.Runner.RequestTimeout = parsedTimeout - addField(runner.ConfigFieldRequestTimeout) } if globalTimeout := repr.Runner.GlobalTimeout; globalTimeout != nil { parsedGlobalTimeout, err := parseOptionalDuration(*globalTimeout) if err != nil { - return abort(err) + return err } cfg.Runner.GlobalTimeout = parsedGlobalTimeout - addField(runner.ConfigFieldGlobalTimeout) } testSuite := repr.Tests if len(testSuite) == 0 { - return cfg.WithFields(assignedFields...), nil + return nil } cases := make([]runner.TestCase, len(testSuite)) @@ -143,22 +131,22 @@ func ParseRepresentation(repr Representation) (runner.Config, error) { //nolint: fieldPath("predicate"): t.Predicate, fieldPath("target"): t.Target, }); err != nil { - return abort(err) + return err } field := runner.MetricsField(*t.Field) if err := field.Validate(); err != nil { - return abort(fmt.Errorf("%s: %s", fieldPath("field"), err)) + return fmt.Errorf("%s: %s", fieldPath("field"), err) } predicate := runner.TestPredicate(*t.Predicate) if err := predicate.Validate(); err != nil { - return abort(fmt.Errorf("%s: %s", fieldPath("predicate"), err)) + return fmt.Errorf("%s: %s", fieldPath("predicate"), err) } target, err := parseMetricValue(field, fmt.Sprint(t.Target)) if err != nil { - return abort(fmt.Errorf("%s: %s", fieldPath("target"), err)) + return fmt.Errorf("%s: %s", fieldPath("target"), err) } cases[i] = runner.TestCase{ @@ -169,9 +157,8 @@ func ParseRepresentation(repr Representation) (runner.Config, error) { //nolint: } } cfg.Tests = cases - addField(runner.ConfigFieldTests) - return cfg.WithFields(assignedFields...), nil + return nil } // helpers From 1d18415495711f5c5e414ac13dd597f5aa0d4ac4 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 15 Oct 2022 18:47:50 +0200 Subject: [PATCH 02/25] feat: remove Config methods that became irrelevant - remove Config.Override, Config.WithFields, Config.Equal - remove related tests - update tests relying on removed Config methods --- configparse/json_test.go | 48 +++---- runner/internal/config/config.go | 97 +------------- runner/internal/config/config_test.go | 176 -------------------------- 3 files changed, 20 insertions(+), 301 deletions(-) diff --git a/configparse/json_test.go b/configparse/json_test.go index 142c98d..adc2105 100644 --- a/configparse/json_test.go +++ b/configparse/json_test.go @@ -3,7 +3,6 @@ package configparse_test import ( "encoding/json" "errors" - "net/url" "testing" "github.com/benchttp/engine/configparse" @@ -18,18 +17,18 @@ func TestJSON(t *testing.T) { } testcases := []struct { - name string - input []byte - expConfig runner.Config - expError error + name string + input []byte + isValidConfig func(runner.Config) bool + expError error }{ { name: "returns error if input json has bad keys", input: baseInput.assign(object{ "badkey": "marcel-patulacci", }).json(), - expConfig: runner.Config{}, - expError: errors.New(`invalid field ("badkey"): does not exist`), + isValidConfig: func(cfg runner.Config) bool { return true }, + expError: errors.New(`invalid field ("badkey"): does not exist`), }, { name: "returns error if input json has bad values", @@ -38,24 +37,23 @@ func TestJSON(t *testing.T) { "concurrency": "bad value", // want int }, }).json(), - expConfig: runner.Config{}, - expError: errors.New(`wrong type for field runner.concurrency: want int, got string`), + isValidConfig: func(runner.Config) bool { return true }, + expError: errors.New(`wrong type for field runner.concurrency: want int, got string`), }, { name: "unmarshals JSON config and merges it with default", input: baseInput.assign(object{ "runner": object{"concurrency": 3}, }).json(), - expConfig: runner.Config{ - Request: runner.RequestConfig{ - URL: mustParseURL("https://example.com"), - }, - Runner: runner.RecorderConfig{ - Concurrency: 3, - }, - }. - WithFields("url", "concurrency"). - Override(runner.DefaultConfig()), + isValidConfig: func(cfg runner.Config) bool { + defaultConfig := runner.DefaultConfig() + + isInputValueParsed := cfg.Runner.Concurrency == 3 + isMergedWithDefault := cfg.Request.Method == defaultConfig.Request.Method && + cfg.Runner.GlobalTimeout == defaultConfig.Runner.GlobalTimeout + + return isInputValueParsed && isMergedWithDefault + }, expError: nil, }, } @@ -63,8 +61,8 @@ func TestJSON(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { gotConfig, gotError := configparse.JSON(tc.input) - if !gotConfig.Equal(tc.expConfig) { - t.Errorf("unexpected config:\nexp %+v\ngot %+v", tc.expConfig, gotConfig) + if !tc.isValidConfig(gotConfig) { + t.Errorf("unexpected config:\n%+v", gotConfig) } if !sameErrors(gotError, tc.expError) { @@ -95,14 +93,6 @@ func (o object) assign(other object) object { return newObject } -func mustParseURL(rawURL string) *url.URL { - u, err := url.Parse(rawURL) - if err != nil { - panic(err) - } - return u -} - func sameErrors(a, b error) bool { if a == nil && b == nil { return true diff --git a/runner/internal/config/config.go b/runner/internal/config/config.go index 4076333..5a6b56a 100644 --- a/runner/internal/config/config.go +++ b/runner/internal/config/config.go @@ -7,7 +7,6 @@ import ( "fmt" "net/http" "net/url" - "reflect" "time" "github.com/benchttp/engine/runner/internal/tests" @@ -79,34 +78,12 @@ type Runner struct { GlobalTimeout time.Duration } -type set map[string]struct{} - -func (set set) add(values ...string) { - for _, v := range values { - set[v] = struct{}{} - } -} - // Global represents the global configuration of the runner. // It must be validated using Global.Validate before usage. type Global struct { Request Request Runner Runner - - Tests []tests.Case - - assignedFields set -} - -// WithField returns a new Global with the input fields marked as set. -// Accepted options are limited to existing Fields, other values are -// silently ignored. -func (cfg Global) WithFields(fields ...string) Global { - if cfg.assignedFields == nil { - cfg.assignedFields = set{} - } - cfg.assignedFields.add(fields...) - return cfg + Tests []tests.Case } // String implements fmt.Stringer. It returns an indented JSON representation @@ -116,78 +93,6 @@ func (cfg Global) String() string { return string(b) } -// Equal returns true if cfg and c are equal configurations. -func (cfg Global) Equal(c Global) bool { - cfg.assignedFields = nil - c.assignedFields = nil - return reflect.DeepEqual(cfg, c) -} - -// Override returns a new Config by overriding the values of base -// with the values from the Config receiver. -// Only fields previously specified by the receiver via Config.WithFields -// are replaced. -// All other values from base are preserved. -// -// The following example is equivalent to defaultConfig with the concurrency -// value from myConfig: -// -// myConfig. -// WithFields(FieldConcurrency). -// Override(defaultConfig) -// -// The following example is equivalent to defaultConfig, as no field as been -// tagged via WithFields by the receiver: -// -// myConfig.Override(defaultConfig) -func (cfg Global) Override(base Global) Global { - for field := range cfg.assignedFields { - switch field { - case FieldMethod: - base.Request.Method = cfg.Request.Method - case FieldURL: - base.Request.URL = cfg.Request.URL - case FieldHeader: - base.overrideHeader(cfg.Request.Header) - case FieldBody: - base.Request.Body = cfg.Request.Body - case FieldRequests: - base.Runner.Requests = cfg.Runner.Requests - case FieldConcurrency: - base.Runner.Concurrency = cfg.Runner.Concurrency - case FieldInterval: - base.Runner.Interval = cfg.Runner.Interval - case FieldRequestTimeout: - base.Runner.RequestTimeout = cfg.Runner.RequestTimeout - case FieldGlobalTimeout: - base.Runner.GlobalTimeout = cfg.Runner.GlobalTimeout - case FieldTests: - base.Tests = cfg.Tests - } - } - return base -} - -// overrideHeader overrides cfg's Request.Header with the values from newHeader. -// For every key in newHeader: -// -// - If it's not present in cfg.Request.Header, it is added. -// -// - If it's already present in cfg.Request.Header, the value is replaced. -// -// - All other keys in cfg.Request.Header are left untouched. -func (cfg *Global) overrideHeader(newHeader http.Header) { - if newHeader == nil { - return - } - if cfg.Request.Header == nil { - cfg.Request.Header = http.Header{} - } - for k, v := range newHeader { - cfg.Request.Header[k] = v - } -} - // Validate returns a non-nil InvalidConfigError if any of its fields // does not meet the requirements. func (cfg Global) Validate() error { //nolint:gocognit diff --git a/runner/internal/config/config_test.go b/runner/internal/config/config_test.go index 6d43ebe..6218614 100644 --- a/runner/internal/config/config_test.go +++ b/runner/internal/config/config_test.go @@ -8,7 +8,6 @@ import ( "net/url" "reflect" "testing" - "time" "github.com/benchttp/engine/runner/internal/config" ) @@ -70,144 +69,6 @@ func TestGlobal_Validate(t *testing.T) { }) } -func TestGlobal_Override(t *testing.T) { - t.Run("do not override unspecified fields", func(t *testing.T) { - baseCfg := config.Global{} - nextCfg := config.Global{ - Request: config.Request{ - Body: config.RequestBody{}, - }.WithURL("http://a.b?p=2"), - Runner: config.Runner{ - Requests: 1, - Concurrency: 2, - RequestTimeout: 3 * time.Second, - GlobalTimeout: 4 * time.Second, - }, - } - - if gotCfg := nextCfg.Override(baseCfg); !gotCfg.Equal(baseCfg) { - t.Errorf("overrode unexpected fields:\nexp %#v\ngot %#v", baseCfg, gotCfg) - } - }) - - t.Run("override specified fields", func(t *testing.T) { - fields := []string{ - config.FieldMethod, - config.FieldURL, - config.FieldRequests, - config.FieldConcurrency, - config.FieldRequestTimeout, - config.FieldGlobalTimeout, - config.FieldBody, - } - - baseCfg := config.Global{} - nextCfg := config.Global{ - Request: config.Request{ - Body: validBody, - }.WithURL("http://a.b?p=2"), - Runner: config.Runner{ - Requests: 1, - Concurrency: 2, - RequestTimeout: 3 * time.Second, - GlobalTimeout: 4 * time.Second, - }, - }.WithFields(fields...) - - if gotCfg := nextCfg.Override(baseCfg); !gotCfg.Equal(nextCfg) { - t.Errorf("did not override expected fields:\nexp %v\ngot %v", nextCfg, gotCfg) - t.Log(fields) - } - }) - - t.Run("override header selectively", func(t *testing.T) { - testcases := []struct { - label string - oldHeader http.Header - newHeader http.Header - expHeader http.Header - }{ - { - label: "erase overridden keys", - oldHeader: http.Header{"key": []string{"oldval"}}, - newHeader: http.Header{"key": []string{"newval"}}, - expHeader: http.Header{"key": []string{"newval"}}, - }, - { - label: "do not erase not overridden keys", - oldHeader: http.Header{"key": []string{"oldval"}}, - newHeader: http.Header{}, - expHeader: http.Header{"key": []string{"oldval"}}, - }, - { - label: "add new keys", - oldHeader: http.Header{"key0": []string{"oldval"}}, - newHeader: http.Header{"key1": []string{"newval"}}, - expHeader: http.Header{ - "key0": []string{"oldval"}, - "key1": []string{"newval"}, - }, - }, - { - label: "erase only overridden keys", - oldHeader: http.Header{ - "key0": []string{"oldval0", "oldval1"}, - "key1": []string{"oldval0", "oldval1"}, - }, - newHeader: http.Header{ - "key1": []string{"newval0", "newval1"}, - "key2": []string{"newval0", "newval1"}, - }, - expHeader: http.Header{ - "key0": []string{"oldval0", "oldval1"}, - "key1": []string{"newval0", "newval1"}, - "key2": []string{"newval0", "newval1"}, - }, - }, - { - label: "nil new header does nothing", - oldHeader: http.Header{"key": []string{"val"}}, - newHeader: nil, - expHeader: http.Header{"key": []string{"val"}}, - }, - { - label: "replace nil old header", - oldHeader: nil, - newHeader: http.Header{"key": []string{"val"}}, - expHeader: http.Header{"key": []string{"val"}}, - }, - { - label: "nil over nil is nil", - oldHeader: nil, - newHeader: nil, - expHeader: nil, - }, - } - - for _, tc := range testcases { - t.Run(tc.label, func(t *testing.T) { - baseCfg := config.Global{ - Request: config.Request{ - Header: tc.oldHeader, - }, - } - - nextCfg := config.Global{ - Request: config.Request{ - Header: tc.newHeader, - }, - }.WithFields(config.FieldHeader) - - gotCfg := nextCfg.Override(baseCfg) - - if gotHeader := gotCfg.Request.Header; !reflect.DeepEqual(gotHeader, tc.expHeader) { - t.Errorf("\nexp %#v\ngot %#v", tc.expHeader, gotHeader) - } - }) - } - }) -} - func TestRequest_WithURL(t *testing.T) { t.Run("set empty url if invalid", func(t *testing.T) { cfg := config.Global{Request: config.Request{}.WithURL("abc")} @@ -297,43 +158,6 @@ func TestRequest_Value(t *testing.T) { }) } -func TestGlobal_Equal(t *testing.T) { - t.Run("returns false for different configs", func(t *testing.T) { - base := config.Default() - diff := base - diff.Runner.Requests = base.Runner.Requests + 1 - - if base.Equal(diff) { - t.Error("exp unequal configs") - } - }) - - t.Run("ignores set fields", func(t *testing.T) { - base := config.Default() - same := base.WithFields(config.FieldRequests) - - if !base.Equal(same) { - t.Error("exp equal configs") - } - }) - - t.Run("does not alter configs", func(t *testing.T) { - baseA := config.Default().WithFields(config.FieldRequests) - copyA := config.Default().WithFields(config.FieldRequests) - baseB := config.Default().WithFields(config.FieldURL) - copyB := config.Default().WithFields(config.FieldURL) - - baseA.Equal(baseB) - - if !reflect.DeepEqual(baseA, copyA) { - t.Error("altered receiver config") - } - if !reflect.DeepEqual(baseB, copyB) { - t.Error("altered parameter config") - } - }) -} - // helpers // findErrorOrFail fails t if no error in src matches msg. From 8757bfbcbd07c2c6a15693a652f46f714cd1462d Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Tue, 18 Oct 2022 23:51:39 +0200 Subject: [PATCH 03/25] chore: remove unused IsConfigField --- runner/internal/config/field.go | 33 ---------------------------- runner/internal/config/field_test.go | 24 -------------------- runner/runner.go | 17 ++------------ 3 files changed, 2 insertions(+), 72 deletions(-) delete mode 100644 runner/internal/config/field.go delete mode 100644 runner/internal/config/field_test.go diff --git a/runner/internal/config/field.go b/runner/internal/config/field.go deleted file mode 100644 index c23c831..0000000 --- a/runner/internal/config/field.go +++ /dev/null @@ -1,33 +0,0 @@ -package config - -const ( - FieldMethod = "method" - FieldURL = "url" - FieldHeader = "header" - FieldBody = "body" - FieldRequests = "requests" - FieldConcurrency = "concurrency" - FieldInterval = "interval" - FieldRequestTimeout = "requestTimeout" - FieldGlobalTimeout = "globalTimeout" - FieldTests = "tests" -) - -// FieldsUsage is a record of all available config fields and their usage. -var FieldsUsage = map[string]string{ - FieldMethod: "HTTP request method", - FieldURL: "HTTP request url", - FieldHeader: "HTTP request header", - FieldBody: "HTTP request body", - FieldRequests: "Number of requests to run, use duration as exit condition if omitted", - FieldConcurrency: "Number of connections to run concurrently", - FieldInterval: "Minimum duration between two non concurrent requests", - FieldRequestTimeout: "Timeout for each HTTP request", - FieldGlobalTimeout: "Max duration of test", - FieldTests: "Test suite", -} - -func IsField(v string) bool { - _, exists := FieldsUsage[v] - return exists -} diff --git a/runner/internal/config/field_test.go b/runner/internal/config/field_test.go deleted file mode 100644 index ad53a23..0000000 --- a/runner/internal/config/field_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package config_test - -import ( - "testing" - - "github.com/drykit-go/testx" - - "github.com/benchttp/engine/runner/internal/config" -) - -func TestIsField(t *testing.T) { - testx.Table(config.IsField).Cases([]testx.Case{ - {In: config.FieldMethod, Exp: true}, - {In: config.FieldURL, Exp: true}, - {In: config.FieldHeader, Exp: true}, - {In: config.FieldBody, Exp: true}, - {In: config.FieldRequests, Exp: true}, - {In: config.FieldConcurrency, Exp: true}, - {In: config.FieldInterval, Exp: true}, - {In: config.FieldRequestTimeout, Exp: true}, - {In: config.FieldGlobalTimeout, Exp: true}, - {In: "notafield", Exp: false}, - }).Run(t) -} diff --git a/runner/runner.go b/runner/runner.go index a68db93..38817c5 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -41,24 +41,11 @@ const ( StatusCanceled = recorder.StatusCanceled StatusTimeout = recorder.StatusTimeout StatusDone = recorder.StatusDone - - ConfigFieldMethod = config.FieldMethod - ConfigFieldURL = config.FieldURL - ConfigFieldHeader = config.FieldHeader - ConfigFieldBody = config.FieldBody - ConfigFieldRequests = config.FieldRequests - ConfigFieldConcurrency = config.FieldConcurrency - ConfigFieldInterval = config.FieldInterval - ConfigFieldRequestTimeout = config.FieldRequestTimeout - ConfigFieldGlobalTimeout = config.FieldGlobalTimeout - ConfigFieldTests = config.FieldTests ) var ( - DefaultConfig = config.Default - ConfigFieldsUsage = config.FieldsUsage - NewRequestBody = config.NewRequestBody - IsConfigField = config.IsField + DefaultConfig = config.Default + NewRequestBody = config.NewRequestBody ErrCanceled = recorder.ErrCanceled ) From 763ffd871a080462d8866e2173308d420e230f6c Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Tue, 18 Oct 2022 23:52:19 +0200 Subject: [PATCH 04/25] refactor(configparse): rename Representation.Unmarshal Unmarshal -> ParseInto --- configparse/json.go | 2 +- configparse/parse.go | 34 ++++++++++++++-------------------- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/configparse/json.go b/configparse/json.go index 6313a49..1a1d718 100644 --- a/configparse/json.go +++ b/configparse/json.go @@ -13,7 +13,7 @@ func JSON(in []byte) (runner.Config, error) { } cfg := runner.DefaultConfig() - if err := repr.Unmarshal(&cfg); err != nil { + if err := repr.ParseInto(&cfg); err != nil { return runner.Config{}, err } diff --git a/configparse/parse.go b/configparse/parse.go index f735708..2b14215 100644 --- a/configparse/parse.go +++ b/configparse/parse.go @@ -10,10 +10,11 @@ import ( "github.com/benchttp/engine/runner" ) -// Representation is a raw data model for runner config files. +// Representation is a raw data model for formatted runner config (json, yaml). // It serves as a receiver for unmarshaling processes and for that reason // its types are kept simple (certain types are incompatible with certain // unmarshalers). +// It exposes a method Unmarshal to convert its values into a runner.Config. type Representation struct { Extends *string `yaml:"extends" json:"extends"` @@ -44,19 +45,12 @@ type Representation struct { } `yaml:"tests" json:"tests"` } -// Unmarshal parses the Representation receiver as a runner.Config +// ParseInto parses the Representation receiver as a runner.Config // and stores any non-nil field value into the corresponding field // of dst. -func (repr Representation) Unmarshal(dst *runner.Config) error { - return UnmarshalRepresentation(repr, dst) -} - -// Unmarshal parses the given Representation as a runner.Config -// and stores any non-nil field value into the corresponding field -// of dst. -func UnmarshalRepresentation(repr Representation, cfg *runner.Config) error { //nolint:gocognit // acceptable complexity for a parsing func +func (repr Representation) ParseInto(dst *runner.Config) error { //nolint:gocognit // acceptable complexity for a parsing func if method := repr.Request.Method; method != nil { - cfg.Request.Method = *method + dst.Request.Method = *method } if rawURL := repr.Request.URL; rawURL != nil { @@ -64,7 +58,7 @@ func UnmarshalRepresentation(repr Representation, cfg *runner.Config) error { // if err != nil { return err } - cfg.Request.URL = parsedURL + dst.Request.URL = parsedURL } if header := repr.Request.Header; header != nil { @@ -72,22 +66,22 @@ func UnmarshalRepresentation(repr Representation, cfg *runner.Config) error { // for key, val := range header { httpHeader[key] = val } - cfg.Request.Header = httpHeader + dst.Request.Header = httpHeader } if body := repr.Request.Body; body != nil { - cfg.Request.Body = runner.RequestBody{ + dst.Request.Body = runner.RequestBody{ Type: body.Type, Content: []byte(body.Content), } } if requests := repr.Runner.Requests; requests != nil { - cfg.Runner.Requests = *requests + dst.Runner.Requests = *requests } if concurrency := repr.Runner.Concurrency; concurrency != nil { - cfg.Runner.Concurrency = *concurrency + dst.Runner.Concurrency = *concurrency } if interval := repr.Runner.Interval; interval != nil { @@ -95,7 +89,7 @@ func UnmarshalRepresentation(repr Representation, cfg *runner.Config) error { // if err != nil { return err } - cfg.Runner.Interval = parsedInterval + dst.Runner.Interval = parsedInterval } if requestTimeout := repr.Runner.RequestTimeout; requestTimeout != nil { @@ -103,7 +97,7 @@ func UnmarshalRepresentation(repr Representation, cfg *runner.Config) error { // if err != nil { return err } - cfg.Runner.RequestTimeout = parsedTimeout + dst.Runner.RequestTimeout = parsedTimeout } if globalTimeout := repr.Runner.GlobalTimeout; globalTimeout != nil { @@ -111,7 +105,7 @@ func UnmarshalRepresentation(repr Representation, cfg *runner.Config) error { // if err != nil { return err } - cfg.Runner.GlobalTimeout = parsedGlobalTimeout + dst.Runner.GlobalTimeout = parsedGlobalTimeout } testSuite := repr.Tests @@ -156,7 +150,7 @@ func UnmarshalRepresentation(repr Representation, cfg *runner.Config) error { // Target: target, } } - cfg.Tests = cases + dst.Tests = cases return nil } From 6ecb641bdb41d551c8d70d142784b30a0e36a3bf Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Tue, 18 Oct 2022 23:53:01 +0200 Subject: [PATCH 05/25] deps: remove unused dependencies - run go mod tidy --- go.mod | 3 --- go.sum | 6 ------ 2 files changed, 9 deletions(-) diff --git a/go.mod b/go.mod index 4de6a8f..23faa4d 100644 --- a/go.mod +++ b/go.mod @@ -3,9 +3,6 @@ module github.com/benchttp/engine go 1.17 require ( - github.com/drykit-go/testx v1.2.0 golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 gopkg.in/yaml.v3 v3.0.1 ) - -require github.com/drykit-go/cond v0.1.0 // indirect diff --git a/go.sum b/go.sum index 12452b0..12011ba 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,3 @@ -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= 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= From a99624ad4f3a0e313adc86e21012339361ea5267 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 19 Oct 2022 00:59:42 +0200 Subject: [PATCH 06/25] refactor(runner): use *http.Request for Config.Request --- configparse/parse.go | 44 +++++++-- runner/internal/config/config.go | 67 +------------- runner/internal/config/config_test.go | 123 ++------------------------ runner/internal/config/default.go | 17 ++-- runner/runner.go | 13 +-- 5 files changed, 59 insertions(+), 205 deletions(-) diff --git a/configparse/parse.go b/configparse/parse.go index 2b14215..0ee8fbd 100644 --- a/configparse/parse.go +++ b/configparse/parse.go @@ -1,7 +1,10 @@ package configparse import ( + "bytes" + "errors" "fmt" + "io" "net/http" "net/url" "strconv" @@ -48,17 +51,29 @@ type Representation struct { // ParseInto parses the Representation receiver as a runner.Config // and stores any non-nil field value into the corresponding field // of dst. -func (repr Representation) ParseInto(dst *runner.Config) error { //nolint:gocognit // acceptable complexity for a parsing func +func (repr Representation) ParseInto(dst *runner.Config) error { + if err := repr.parseRequestInto(dst); err != nil { + return err + } + if err := repr.parseRunnerInto(dst); err != nil { + return err + } + return repr.parseTestsInto(dst) +} + +func (repr Representation) parseRequestInto(dst *runner.Config) error { + req := &http.Request{} + if method := repr.Request.Method; method != nil { - dst.Request.Method = *method + req.Method = *method } if rawURL := repr.Request.URL; rawURL != nil { parsedURL, err := parseAndBuildURL(*rawURL, repr.Request.QueryParams) if err != nil { - return err + return fmt.Errorf(`configparse: invalid url: %q`, *rawURL) } - dst.Request.URL = parsedURL + req.URL = parsedURL } if header := repr.Request.Header; header != nil { @@ -66,16 +81,23 @@ func (repr Representation) ParseInto(dst *runner.Config) error { //nolint:gocogn for key, val := range header { httpHeader[key] = val } - dst.Request.Header = httpHeader + req.Header = httpHeader } if body := repr.Request.Body; body != nil { - dst.Request.Body = runner.RequestBody{ - Type: body.Type, - Content: []byte(body.Content), + switch body.Type { + case "raw": + req.Body = io.NopCloser(bytes.NewReader([]byte(body.Content))) + default: + return errors.New(`configparse: request.body.type: only "raw" accepted`) } } + *dst.Request = *req + return nil +} + +func (repr Representation) parseRunnerInto(dst *runner.Config) error { if requests := repr.Runner.Requests; requests != nil { dst.Runner.Requests = *requests } @@ -108,6 +130,10 @@ func (repr Representation) ParseInto(dst *runner.Config) error { //nolint:gocogn dst.Runner.GlobalTimeout = parsedGlobalTimeout } + return nil +} + +func (repr Representation) parseTestsInto(dst *runner.Config) error { testSuite := repr.Tests if len(testSuite) == 0 { return nil @@ -150,8 +176,8 @@ func (repr Representation) ParseInto(dst *runner.Config) error { //nolint:gocogn Target: target, } } - dst.Tests = cases + dst.Tests = cases return nil } diff --git a/runner/internal/config/config.go b/runner/internal/config/config.go index 5a6b56a..efce833 100644 --- a/runner/internal/config/config.go +++ b/runner/internal/config/config.go @@ -1,74 +1,15 @@ package config import ( - "bytes" "encoding/json" "errors" "fmt" "net/http" - "net/url" "time" "github.com/benchttp/engine/runner/internal/tests" ) -// RequestBody represents a request body associated with a type. -// The type affects the way the content is processed. -// If Type == "file", Content is read as a filepath to be resolved. -// If Type == "raw", Content is attached as-is. -// -// Note: only "raw" is supported at the moment. -type RequestBody struct { - Type string - Content []byte -} - -// NewRequestBody returns a Body initialized with the given type and content. -// For now, the only valid value for type is "raw". -func NewRequestBody(typ, content string) RequestBody { - return RequestBody{Type: typ, Content: []byte(content)} -} - -// Request contains the confing options relative to a single request. -type Request struct { - Method string - URL *url.URL - Header http.Header - Body RequestBody -} - -// Value generates a *http.Request based on Request and returns it -// or any non-nil error that occurred. -func (r Request) Value() (*http.Request, error) { - if r.URL == nil { - return nil, errors.New("empty url") - } - rawURL := r.URL.String() - if _, err := url.ParseRequestURI(rawURL); err != nil { - return nil, errors.New("bad url") - } - - req, err := http.NewRequest(r.Method, rawURL, bytes.NewReader(r.Body.Content)) - if err != nil { - return nil, err - } - req.Header = r.Header - return req, nil -} - -// WithURL sets the current Request with the parsed *url.URL from rawURL -// and returns it. Any errors is discarded as a Config can be invalid -// until Config.Validate is called. The url is always non-nil. -func (r Request) WithURL(rawURL string) Request { - // ignore err: a Config can be invalid at this point - urlURL, _ := url.ParseRequestURI(rawURL) - if urlURL == nil { - urlURL = &url.URL{} - } - r.URL = urlURL - return r -} - // Runner contains options relative to the runner. type Runner struct { Requests int @@ -81,7 +22,7 @@ type Runner struct { // Global represents the global configuration of the runner. // It must be validated using Global.Validate before usage. type Global struct { - Request Request + Request *http.Request Runner Runner Tests []tests.Case } @@ -101,10 +42,8 @@ func (cfg Global) Validate() error { //nolint:gocognit errs = append(errs, err) } - if cfg.Request.URL == nil { - appendError(errors.New("url: missing")) - } else if _, err := url.ParseRequestURI(cfg.Request.URL.String()); err != nil { - appendError(fmt.Errorf("url (%q): invalid", cfg.Request.URL.String())) + if cfg.Request == nil { + appendError(errors.New("unexpected nil request")) } if cfg.Runner.Requests < 1 && cfg.Runner.Requests != -1 { diff --git a/runner/internal/config/config_test.go b/runner/internal/config/config_test.go index 6218614..a8d3c20 100644 --- a/runner/internal/config/config_test.go +++ b/runner/internal/config/config_test.go @@ -1,25 +1,17 @@ package config_test import ( - "bytes" "errors" - "io" "net/http" - "net/url" - "reflect" "testing" "github.com/benchttp/engine/runner/internal/config" ) -var validBody = config.NewRequestBody("raw", `{"key0": "val0", "key1": "val1"}`) - func TestGlobal_Validate(t *testing.T) { t.Run("return nil if config is valid", func(t *testing.T) { cfg := config.Global{ - Request: config.Request{ - Body: validBody, - }.WithURL("https://github.com/benchttp/"), + Request: validRequest(), Runner: config.Runner{ Requests: 5, Concurrency: 5, @@ -35,9 +27,7 @@ func TestGlobal_Validate(t *testing.T) { t.Run("return cumulated errors if config is invalid", func(t *testing.T) { cfg := config.Global{ - Request: config.Request{ - Body: config.RequestBody{}, - }.WithURL("abc"), + Request: nil, Runner: config.Runner{ Requests: -5, Concurrency: -5, @@ -58,7 +48,7 @@ func TestGlobal_Validate(t *testing.T) { } errs := errInvalid.Errors - findErrorOrFail(t, errs, `url (""): invalid`) + findErrorOrFail(t, errs, `unexpected nil request`) findErrorOrFail(t, errs, `requests (-5): want >= 0`) findErrorOrFail(t, errs, `concurrency (-5): want > 0 and <= requests (-5)`) findErrorOrFail(t, errs, `interval (-5): want >= 0`) @@ -69,97 +59,16 @@ func TestGlobal_Validate(t *testing.T) { }) } -func TestRequest_WithURL(t *testing.T) { - t.Run("set empty url if invalid", func(t *testing.T) { - cfg := config.Global{Request: config.Request{}.WithURL("abc")} - if got := cfg.Request.URL; !reflect.DeepEqual(got, &url.URL{}) { - t.Errorf("exp empty *url.URL, got %v", got) - } - }) - - t.Run("set parsed url", func(t *testing.T) { - var ( - rawURL = "http://benchttp.app?cool=true" - expURL, _ = url.ParseRequestURI(rawURL) - gotURL = config.Request{}.WithURL(rawURL).URL - ) - - if !reflect.DeepEqual(gotURL, expURL) { - t.Errorf("\nexp %v\ngot %v", expURL, gotURL) - } - }) -} - -func TestRequest_Value(t *testing.T) { - testcases := []struct { - label string - in config.Request - expMsg string - }{ - { - label: "return error if url is empty", - in: config.Request{}, - expMsg: "empty url", - }, - { - label: "return error if url is invalid", - in: config.Request{URL: &url.URL{Scheme: ""}}, - expMsg: "bad url", - }, - { - label: "return error if NewRequest fails", - in: config.Request{Method: "é", URL: &url.URL{Scheme: "http"}}, - expMsg: `net/http: invalid method "é"`, - }, - } - - for _, tc := range testcases { - t.Run(tc.label, func(t *testing.T) { - gotReq, gotErr := tc.in.Value() - if gotErr == nil { - t.Fatal("exp error, got nil") - } - - if gotMsg := gotErr.Error(); gotMsg != tc.expMsg { - t.Errorf("\nexp %q\ngot %q", tc.expMsg, gotMsg) - } +// helpers - if gotReq != nil { - t.Errorf("exp nil, got %v", gotReq) - } - }) +func validRequest() *http.Request { + req, err := http.NewRequest("GET", "https://a.b#c?d=e&f=g", nil) + if err != nil { + panic(err) } - - t.Run("return request with added headers", func(t *testing.T) { - in := config.Request{ - Method: "POST", - Header: http.Header{"key": []string{"val"}}, - Body: config.RequestBody{Content: []byte("abc")}, - }.WithURL("http://a.b") - - expReq, err := http.NewRequest( - in.Method, - in.URL.String(), - bytes.NewReader(in.Body.Content), - ) - if err != nil { - t.Fatal(err) - } - expReq.Header = in.Header - - gotReq, gotErr := in.Value() - if gotErr != nil { - t.Fatal(err) - } - - if !sameRequests(gotReq, expReq) { - t.Errorf("\nexp %#v\ngot %#v", expReq, gotReq) - } - }) + return req } -// helpers - // findErrorOrFail fails t if no error in src matches msg. func findErrorOrFail(t *testing.T, src []error, msg string) { t.Helper() @@ -170,17 +79,3 @@ func findErrorOrFail(t *testing.T, src []error, msg string) { } t.Errorf("missing error: %v", msg) } - -func sameRequests(a, b *http.Request) bool { - if a == nil || b == nil { - return a == b - } - - ab, _ := io.ReadAll(a.Body) - bb, _ := io.ReadAll(b.Body) - - return a.Method == b.Method && - a.URL.String() == b.URL.String() && - bytes.Equal(ab, bb) && - reflect.DeepEqual(a.Header, b.Header) -} diff --git a/runner/internal/config/default.go b/runner/internal/config/default.go index b5299de..9bae34f 100644 --- a/runner/internal/config/default.go +++ b/runner/internal/config/default.go @@ -1,18 +1,13 @@ package config import ( + "fmt" "net/http" - "net/url" "time" ) var defaultConfig = Global{ - Request: Request{ - Method: "GET", - URL: &url.URL{}, - Header: http.Header{}, - Body: RequestBody{}, - }, + Request: defaultRequest(), Runner: Runner{ Concurrency: 10, Requests: 100, @@ -26,3 +21,11 @@ var defaultConfig = Global{ func Default() Global { return defaultConfig } + +func defaultRequest() *http.Request { + req, err := http.NewRequest("GET", "", nil) + if err != nil { + panic(fmt.Sprintf("benchttp/runner: %s", err)) + } + return req +} diff --git a/runner/runner.go b/runner/runner.go index 38817c5..10467bc 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -13,8 +13,6 @@ import ( type ( Config = config.Global - RequestConfig = config.Request - RequestBody = config.RequestBody RecorderConfig = config.Runner InvalidConfigError = config.InvalidConfigError @@ -44,8 +42,7 @@ const ( ) var ( - DefaultConfig = config.Default - NewRequestBody = config.NewRequestBody + DefaultConfig = config.Default ErrCanceled = recorder.ErrCanceled ) @@ -65,19 +62,13 @@ func (r *Runner) Run(ctx context.Context, cfg config.Global) (*Report, error) { return nil, err } - // Generate http request from input config - rq, err := cfg.Request.Value() - if err != nil { - return nil, err - } - // Create and attach request recorder r.recorder = recorder.New(recorderConfig(cfg, r.onRecordingProgress)) startTime := time.Now() // Run request recorder - records, err := r.recorder.Record(ctx, rq) + records, err := r.recorder.Record(ctx, cfg.Request) if err != nil { return nil, err } From afe07de411d0a8309a2b8e3b0143cf8825d609e9 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 19 Oct 2022 01:16:58 +0200 Subject: [PATCH 07/25] refactor(runner): flatten packages runner, config, report --- configparse/json.go | 2 +- runner/{internal/config => }/config.go | 18 ++++++++--------- runner/{internal/config => }/config_test.go | 16 +++++++-------- runner/{internal/config => }/default.go | 10 +++++----- runner/{internal/config => }/error.go | 4 ++-- runner/{internal/config => }/error_test.go | 6 +++--- runner/{internal/report => }/report.go | 11 +++++------ runner/runner.go | 22 ++++----------------- 8 files changed, 37 insertions(+), 52 deletions(-) rename runner/{internal/config => }/config.go (81%) rename runner/{internal/config => }/config_test.go (86%) rename runner/{internal/config => }/default.go (72%) rename runner/{internal/config => }/error.go (85%) rename runner/{internal/config => }/error_test.go (79%) rename runner/{internal/report => }/report.go (80%) diff --git a/configparse/json.go b/configparse/json.go index 1a1d718..7297947 100644 --- a/configparse/json.go +++ b/configparse/json.go @@ -4,7 +4,7 @@ import ( "github.com/benchttp/engine/runner" ) -// JSON reads input bytes as JSON and unmarshals it into a runner.ConfigGlobal. +// JSON reads input bytes as JSON and unmarshals it into a runner.Config. func JSON(in []byte) (runner.Config, error) { parser := JSONParser{} repr := Representation{} diff --git a/runner/internal/config/config.go b/runner/config.go similarity index 81% rename from runner/internal/config/config.go rename to runner/config.go index efce833..30158bb 100644 --- a/runner/internal/config/config.go +++ b/runner/config.go @@ -1,4 +1,4 @@ -package config +package runner import ( "encoding/json" @@ -10,8 +10,8 @@ import ( "github.com/benchttp/engine/runner/internal/tests" ) -// Runner contains options relative to the runner. -type Runner struct { +// RunnerConfig contains options relative to the runner. +type RunnerConfig struct { Requests int Concurrency int Interval time.Duration @@ -19,24 +19,24 @@ type Runner struct { GlobalTimeout time.Duration } -// Global represents the global configuration of the runner. -// It must be validated using Global.Validate before usage. -type Global struct { +// Config represents the global configuration of the runner. +// It must be validated using Config.Validate before usage. +type Config struct { Request *http.Request - Runner Runner + Runner RunnerConfig Tests []tests.Case } // String implements fmt.Stringer. It returns an indented JSON representation // of Config for debugging purposes. -func (cfg Global) String() string { +func (cfg Config) String() string { b, _ := json.MarshalIndent(cfg, "", " ") return string(b) } // Validate returns a non-nil InvalidConfigError if any of its fields // does not meet the requirements. -func (cfg Global) Validate() error { //nolint:gocognit +func (cfg Config) Validate() error { //nolint:gocognit errs := []error{} appendError := func(err error) { errs = append(errs, err) diff --git a/runner/internal/config/config_test.go b/runner/config_test.go similarity index 86% rename from runner/internal/config/config_test.go rename to runner/config_test.go index a8d3c20..c3a2734 100644 --- a/runner/internal/config/config_test.go +++ b/runner/config_test.go @@ -1,18 +1,18 @@ -package config_test +package runner_test import ( "errors" "net/http" "testing" - "github.com/benchttp/engine/runner/internal/config" + "github.com/benchttp/engine/runner" ) -func TestGlobal_Validate(t *testing.T) { +func TestConfig_Validate(t *testing.T) { t.Run("return nil if config is valid", func(t *testing.T) { - cfg := config.Global{ + cfg := runner.Config{ Request: validRequest(), - Runner: config.Runner{ + Runner: runner.RunnerConfig{ Requests: 5, Concurrency: 5, Interval: 5, @@ -26,9 +26,9 @@ func TestGlobal_Validate(t *testing.T) { }) t.Run("return cumulated errors if config is invalid", func(t *testing.T) { - cfg := config.Global{ + cfg := runner.Config{ Request: nil, - Runner: config.Runner{ + Runner: runner.RunnerConfig{ Requests: -5, Concurrency: -5, Interval: -5, @@ -42,7 +42,7 @@ func TestGlobal_Validate(t *testing.T) { t.Fatal("invalid configuration considered valid") } - var errInvalid *config.InvalidConfigError + var errInvalid *runner.InvalidConfigError if !errors.As(err, &errInvalid) { t.Fatalf("unexpected error type: %T", err) } diff --git a/runner/internal/config/default.go b/runner/default.go similarity index 72% rename from runner/internal/config/default.go rename to runner/default.go index 9bae34f..84b896d 100644 --- a/runner/internal/config/default.go +++ b/runner/default.go @@ -1,4 +1,4 @@ -package config +package runner import ( "fmt" @@ -6,9 +6,9 @@ import ( "time" ) -var defaultConfig = Global{ +var defaultConfig = Config{ Request: defaultRequest(), - Runner: Runner{ + Runner: RunnerConfig{ Concurrency: 10, Requests: 100, Interval: 0 * time.Second, @@ -17,8 +17,8 @@ var defaultConfig = Global{ }, } -// Default returns a default config that is safe to use. -func Default() Global { +// DefaultConfig returns a default config that is safe to use. +func DefaultConfig() Config { return defaultConfig } diff --git a/runner/internal/config/error.go b/runner/error.go similarity index 85% rename from runner/internal/config/error.go rename to runner/error.go index a69cf82..145f72c 100644 --- a/runner/internal/config/error.go +++ b/runner/error.go @@ -1,8 +1,8 @@ -package config +package runner import "strings" -// InvalidConfigError is the errors returned by Global.Validate +// InvalidConfigError is the errors returned by Config.Validate // when values are missing or invalid. type InvalidConfigError struct { Errors []error diff --git a/runner/internal/config/error_test.go b/runner/error_test.go similarity index 79% rename from runner/internal/config/error_test.go rename to runner/error_test.go index 8ff4421..f8c3af2 100644 --- a/runner/internal/config/error_test.go +++ b/runner/error_test.go @@ -1,14 +1,14 @@ -package config_test +package runner_test import ( "errors" "testing" - "github.com/benchttp/engine/runner/internal/config" + "github.com/benchttp/engine/runner" ) func TestInvalidConfigError_Error(t *testing.T) { - e := config.InvalidConfigError{ + e := runner.InvalidConfigError{ Errors: []error{ errors.New("error 0"), errors.New("error 1\nwith new line"), diff --git a/runner/internal/report/report.go b/runner/report.go similarity index 80% rename from runner/internal/report/report.go rename to runner/report.go index 8f972c9..cd75bf9 100644 --- a/runner/internal/report/report.go +++ b/runner/report.go @@ -1,9 +1,8 @@ -package report +package runner import ( "time" - "github.com/benchttp/engine/runner/internal/config" "github.com/benchttp/engine/runner/internal/metrics" "github.com/benchttp/engine/runner/internal/tests" ) @@ -17,14 +16,14 @@ type Report struct { // Metadata contains contextual information about a run. type Metadata struct { - Config config.Global + Config Config FinishedAt time.Time TotalDuration time.Duration } -// New returns an initialized *Report. -func New( - cfg config.Global, +// newReport returns an initialized *Report. +func newReport( + cfg Config, d time.Duration, m metrics.Aggregate, t tests.SuiteResult, diff --git a/runner/runner.go b/runner/runner.go index 10467bc..74db536 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -4,23 +4,15 @@ import ( "context" "time" - "github.com/benchttp/engine/runner/internal/config" "github.com/benchttp/engine/runner/internal/metrics" "github.com/benchttp/engine/runner/internal/recorder" - "github.com/benchttp/engine/runner/internal/report" "github.com/benchttp/engine/runner/internal/tests" ) type ( - Config = config.Global - RecorderConfig = config.Runner - InvalidConfigError = config.InvalidConfigError - RecordingProgress = recorder.Progress RecordingStatus = recorder.Status - Report = report.Report - MetricsAggregate = metrics.Aggregate MetricsField = metrics.Field MetricsValue = metrics.Value @@ -30,8 +22,6 @@ type ( TestPredicate = tests.Predicate TestSuiteResults = tests.SuiteResult TestCaseResult = tests.CaseResult - - ReportMetadata = report.Metadata ) const ( @@ -41,11 +31,7 @@ const ( StatusDone = recorder.StatusDone ) -var ( - DefaultConfig = config.Default - - ErrCanceled = recorder.ErrCanceled -) +var ErrCanceled = recorder.ErrCanceled type Runner struct { recorder *recorder.Recorder @@ -56,7 +42,7 @@ func New(onRecordingProgress func(RecordingProgress)) *Runner { return &Runner{onRecordingProgress: onRecordingProgress} } -func (r *Runner) Run(ctx context.Context, cfg config.Global) (*Report, error) { +func (r *Runner) Run(ctx context.Context, cfg Config) (*Report, error) { // Validate input config if err := cfg.Validate(); err != nil { return nil, err @@ -79,7 +65,7 @@ func (r *Runner) Run(ctx context.Context, cfg config.Global) (*Report, error) { testResults := tests.Run(agg, cfg.Tests) - return report.New(cfg, duration, agg, testResults), nil + return newReport(cfg, duration, agg, testResults), nil } // Progress returns the current progress of the recording. @@ -94,7 +80,7 @@ func (r *Runner) Progress() RecordingProgress { // recorderConfig returns a runner.RequesterConfig generated from cfg. func recorderConfig( - cfg config.Global, + cfg Config, onRecordingProgress func(recorder.Progress), ) recorder.Config { return recorder.Config{ From 5b2168be20f2143844a8d82dc2148ac18fc157e2 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 19 Oct 2022 02:53:10 +0200 Subject: [PATCH 08/25] refactor(configparse): minor param renaming --- configparse/parse.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/configparse/parse.go b/configparse/parse.go index 0ee8fbd..101c1b1 100644 --- a/configparse/parse.go +++ b/configparse/parse.go @@ -62,10 +62,12 @@ func (repr Representation) ParseInto(dst *runner.Config) error { } func (repr Representation) parseRequestInto(dst *runner.Config) error { - req := &http.Request{} + if dst.Request == nil { + dst.Request = &http.Request{} + } if method := repr.Request.Method; method != nil { - req.Method = *method + dst.Request.Method = *method } if rawURL := repr.Request.URL; rawURL != nil { @@ -73,7 +75,7 @@ func (repr Representation) parseRequestInto(dst *runner.Config) error { if err != nil { return fmt.Errorf(`configparse: invalid url: %q`, *rawURL) } - req.URL = parsedURL + dst.Request.URL = parsedURL } if header := repr.Request.Header; header != nil { @@ -81,19 +83,18 @@ func (repr Representation) parseRequestInto(dst *runner.Config) error { for key, val := range header { httpHeader[key] = val } - req.Header = httpHeader + dst.Request.Header = httpHeader } if body := repr.Request.Body; body != nil { switch body.Type { case "raw": - req.Body = io.NopCloser(bytes.NewReader([]byte(body.Content))) + dst.Request.Body = io.NopCloser(bytes.NewReader([]byte(body.Content))) default: return errors.New(`configparse: request.body.type: only "raw" accepted`) } } - *dst.Request = *req return nil } From 3999dbd2484edfd9941caeb69f1f3f67538ba025 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 19 Oct 2022 03:19:50 +0200 Subject: [PATCH 09/25] refactor(runner): nuke Config, flatten Runner fields --- configparse/json.go | 10 +-- configparse/json_test.go | 22 +++---- configparse/parse.go | 20 +++--- runner/config.go | 77 ----------------------- runner/default.go | 23 ++++--- runner/error.go | 6 +- runner/error_test.go | 4 +- runner/report.go | 4 +- runner/runner.go | 76 ++++++++++++++++++---- runner/{config_test.go => runner_test.go} | 41 ++++++------ 10 files changed, 127 insertions(+), 156 deletions(-) delete mode 100644 runner/config.go rename runner/{config_test.go => runner_test.go} (70%) diff --git a/configparse/json.go b/configparse/json.go index 7297947..897c44f 100644 --- a/configparse/json.go +++ b/configparse/json.go @@ -4,17 +4,17 @@ import ( "github.com/benchttp/engine/runner" ) -// JSON reads input bytes as JSON and unmarshals it into a runner.Config. -func JSON(in []byte) (runner.Config, error) { +// JSON reads input bytes as JSON and unmarshals it into a runner.Runner. +func JSON(in []byte) (runner.Runner, error) { parser := JSONParser{} repr := Representation{} if err := parser.Parse(in, &repr); err != nil { - return runner.Config{}, err + return runner.Runner{}, err } - cfg := runner.DefaultConfig() + cfg := runner.DefaultRunner() if err := repr.ParseInto(&cfg); err != nil { - return runner.Config{}, err + return runner.Runner{}, err } return cfg, nil diff --git a/configparse/json_test.go b/configparse/json_test.go index adc2105..4d76c10 100644 --- a/configparse/json_test.go +++ b/configparse/json_test.go @@ -19,7 +19,7 @@ func TestJSON(t *testing.T) { testcases := []struct { name string input []byte - isValidConfig func(runner.Config) bool + isValidRunner func(runner.Runner) bool expError error }{ { @@ -27,7 +27,7 @@ func TestJSON(t *testing.T) { input: baseInput.assign(object{ "badkey": "marcel-patulacci", }).json(), - isValidConfig: func(cfg runner.Config) bool { return true }, + isValidRunner: func(cfg runner.Runner) bool { return true }, expError: errors.New(`invalid field ("badkey"): does not exist`), }, { @@ -37,7 +37,7 @@ func TestJSON(t *testing.T) { "concurrency": "bad value", // want int }, }).json(), - isValidConfig: func(runner.Config) bool { return true }, + isValidRunner: func(runner.Runner) bool { return true }, expError: errors.New(`wrong type for field runner.concurrency: want int, got string`), }, { @@ -45,12 +45,12 @@ func TestJSON(t *testing.T) { input: baseInput.assign(object{ "runner": object{"concurrency": 3}, }).json(), - isValidConfig: func(cfg runner.Config) bool { - defaultConfig := runner.DefaultConfig() + isValidRunner: func(r runner.Runner) bool { + defaultRunner := runner.DefaultRunner() - isInputValueParsed := cfg.Runner.Concurrency == 3 - isMergedWithDefault := cfg.Request.Method == defaultConfig.Request.Method && - cfg.Runner.GlobalTimeout == defaultConfig.Runner.GlobalTimeout + isInputValueParsed := r.Concurrency == 3 + isMergedWithDefault := r.Request.Method == defaultRunner.Request.Method && + r.GlobalTimeout == defaultRunner.GlobalTimeout return isInputValueParsed && isMergedWithDefault }, @@ -60,9 +60,9 @@ func TestJSON(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - gotConfig, gotError := configparse.JSON(tc.input) - if !tc.isValidConfig(gotConfig) { - t.Errorf("unexpected config:\n%+v", gotConfig) + gotRunner, gotError := configparse.JSON(tc.input) + if !tc.isValidRunner(gotRunner) { + t.Errorf("unexpected config:\n%+v", gotRunner) } if !sameErrors(gotError, tc.expError) { diff --git a/configparse/parse.go b/configparse/parse.go index 101c1b1..a33e8e8 100644 --- a/configparse/parse.go +++ b/configparse/parse.go @@ -48,10 +48,10 @@ type Representation struct { } `yaml:"tests" json:"tests"` } -// ParseInto parses the Representation receiver as a runner.Config +// ParseInto parses the Representation receiver as a runner.Runner // and stores any non-nil field value into the corresponding field // of dst. -func (repr Representation) ParseInto(dst *runner.Config) error { +func (repr Representation) ParseInto(dst *runner.Runner) error { if err := repr.parseRequestInto(dst); err != nil { return err } @@ -61,7 +61,7 @@ func (repr Representation) ParseInto(dst *runner.Config) error { return repr.parseTestsInto(dst) } -func (repr Representation) parseRequestInto(dst *runner.Config) error { +func (repr Representation) parseRequestInto(dst *runner.Runner) error { if dst.Request == nil { dst.Request = &http.Request{} } @@ -98,13 +98,13 @@ func (repr Representation) parseRequestInto(dst *runner.Config) error { return nil } -func (repr Representation) parseRunnerInto(dst *runner.Config) error { +func (repr Representation) parseRunnerInto(dst *runner.Runner) error { if requests := repr.Runner.Requests; requests != nil { - dst.Runner.Requests = *requests + dst.Requests = *requests } if concurrency := repr.Runner.Concurrency; concurrency != nil { - dst.Runner.Concurrency = *concurrency + dst.Concurrency = *concurrency } if interval := repr.Runner.Interval; interval != nil { @@ -112,7 +112,7 @@ func (repr Representation) parseRunnerInto(dst *runner.Config) error { if err != nil { return err } - dst.Runner.Interval = parsedInterval + dst.Interval = parsedInterval } if requestTimeout := repr.Runner.RequestTimeout; requestTimeout != nil { @@ -120,7 +120,7 @@ func (repr Representation) parseRunnerInto(dst *runner.Config) error { if err != nil { return err } - dst.Runner.RequestTimeout = parsedTimeout + dst.RequestTimeout = parsedTimeout } if globalTimeout := repr.Runner.GlobalTimeout; globalTimeout != nil { @@ -128,13 +128,13 @@ func (repr Representation) parseRunnerInto(dst *runner.Config) error { if err != nil { return err } - dst.Runner.GlobalTimeout = parsedGlobalTimeout + dst.GlobalTimeout = parsedGlobalTimeout } return nil } -func (repr Representation) parseTestsInto(dst *runner.Config) error { +func (repr Representation) parseTestsInto(dst *runner.Runner) error { testSuite := repr.Tests if len(testSuite) == 0 { return nil diff --git a/runner/config.go b/runner/config.go deleted file mode 100644 index 30158bb..0000000 --- a/runner/config.go +++ /dev/null @@ -1,77 +0,0 @@ -package runner - -import ( - "encoding/json" - "errors" - "fmt" - "net/http" - "time" - - "github.com/benchttp/engine/runner/internal/tests" -) - -// RunnerConfig contains options relative to the runner. -type RunnerConfig struct { - Requests int - Concurrency int - Interval time.Duration - RequestTimeout time.Duration - GlobalTimeout time.Duration -} - -// Config represents the global configuration of the runner. -// It must be validated using Config.Validate before usage. -type Config struct { - Request *http.Request - Runner RunnerConfig - Tests []tests.Case -} - -// String implements fmt.Stringer. It returns an indented JSON representation -// of Config for debugging purposes. -func (cfg Config) String() string { - b, _ := json.MarshalIndent(cfg, "", " ") - return string(b) -} - -// Validate returns a non-nil InvalidConfigError if any of its fields -// does not meet the requirements. -func (cfg Config) Validate() error { //nolint:gocognit - errs := []error{} - appendError := func(err error) { - errs = append(errs, err) - } - - if cfg.Request == nil { - appendError(errors.New("unexpected nil request")) - } - - if cfg.Runner.Requests < 1 && cfg.Runner.Requests != -1 { - appendError(fmt.Errorf("requests (%d): want >= 0", cfg.Runner.Requests)) - } - - if cfg.Runner.Concurrency < 1 || cfg.Runner.Concurrency > cfg.Runner.Requests { - appendError(fmt.Errorf( - "concurrency (%d): want > 0 and <= requests (%d)", - cfg.Runner.Concurrency, cfg.Runner.Requests, - )) - } - - if cfg.Runner.Interval < 0 { - appendError(fmt.Errorf("interval (%d): want >= 0", cfg.Runner.Interval)) - } - - if cfg.Runner.RequestTimeout < 1 { - appendError(fmt.Errorf("requestTimeout (%d): want > 0", cfg.Runner.RequestTimeout)) - } - - if cfg.Runner.GlobalTimeout < 1 { - appendError(fmt.Errorf("globalTimeout (%d): want > 0", cfg.Runner.GlobalTimeout)) - } - - if len(errs) > 0 { - return &InvalidConfigError{errs} - } - - return nil -} diff --git a/runner/default.go b/runner/default.go index 84b896d..32ca603 100644 --- a/runner/default.go +++ b/runner/default.go @@ -6,20 +6,19 @@ import ( "time" ) -var defaultConfig = Config{ - Request: defaultRequest(), - Runner: RunnerConfig{ - Concurrency: 10, - Requests: 100, - Interval: 0 * time.Second, - RequestTimeout: 5 * time.Second, - GlobalTimeout: 30 * time.Second, - }, +// DefaultRunner returns a default Runner that is safe to use. +func DefaultRunner() Runner { + return defaultRunner } -// DefaultConfig returns a default config that is safe to use. -func DefaultConfig() Config { - return defaultConfig +var defaultRunner = Runner{ + Request: defaultRequest(), + + Concurrency: 10, + Requests: 100, + Interval: 0 * time.Second, + RequestTimeout: 5 * time.Second, + GlobalTimeout: 30 * time.Second, } func defaultRequest() *http.Request { diff --git a/runner/error.go b/runner/error.go index 145f72c..98f82ba 100644 --- a/runner/error.go +++ b/runner/error.go @@ -2,14 +2,14 @@ package runner import "strings" -// InvalidConfigError is the errors returned by Config.Validate +// InvalidRunnerError is the errors returned by Config.Validate // when values are missing or invalid. -type InvalidConfigError struct { +type InvalidRunnerError struct { Errors []error } // Error returns the joined errors of InvalidConfigError as a string. -func (e *InvalidConfigError) Error() string { +func (e *InvalidRunnerError) Error() string { const sep = "\n - " var b strings.Builder diff --git a/runner/error_test.go b/runner/error_test.go index f8c3af2..8b1aad7 100644 --- a/runner/error_test.go +++ b/runner/error_test.go @@ -7,8 +7,8 @@ import ( "github.com/benchttp/engine/runner" ) -func TestInvalidConfigError_Error(t *testing.T) { - e := runner.InvalidConfigError{ +func TestInvalidRunnerError(t *testing.T) { + e := runner.InvalidRunnerError{ Errors: []error{ errors.New("error 0"), errors.New("error 1\nwith new line"), diff --git a/runner/report.go b/runner/report.go index cd75bf9..a0d959d 100644 --- a/runner/report.go +++ b/runner/report.go @@ -16,14 +16,14 @@ type Report struct { // Metadata contains contextual information about a run. type Metadata struct { - Config Config + Config Runner FinishedAt time.Time TotalDuration time.Duration } // newReport returns an initialized *Report. func newReport( - cfg Config, + cfg Runner, d time.Duration, m metrics.Aggregate, t tests.SuiteResult, diff --git a/runner/runner.go b/runner/runner.go index 74db536..e421911 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -2,6 +2,9 @@ package runner import ( "context" + "errors" + "fmt" + "net/http" "time" "github.com/benchttp/engine/runner/internal/metrics" @@ -34,6 +37,16 @@ const ( var ErrCanceled = recorder.ErrCanceled type Runner struct { + Request *http.Request + + Requests int + Concurrency int + Interval time.Duration + RequestTimeout time.Duration + GlobalTimeout time.Duration + + Tests []tests.Case + recorder *recorder.Recorder onRecordingProgress func(RecordingProgress) } @@ -42,14 +55,14 @@ func New(onRecordingProgress func(RecordingProgress)) *Runner { return &Runner{onRecordingProgress: onRecordingProgress} } -func (r *Runner) Run(ctx context.Context, cfg Config) (*Report, error) { +func (r *Runner) Run(ctx context.Context, cfg Runner) (*Report, error) { // Validate input config if err := cfg.Validate(); err != nil { return nil, err } // Create and attach request recorder - r.recorder = recorder.New(recorderConfig(cfg, r.onRecordingProgress)) + r.recorder = recorder.New(r.recorderConfig()) startTime := time.Now() @@ -79,16 +92,55 @@ func (r *Runner) Progress() RecordingProgress { } // recorderConfig returns a runner.RequesterConfig generated from cfg. -func recorderConfig( - cfg Config, - onRecordingProgress func(recorder.Progress), -) recorder.Config { +func (r *Runner) recorderConfig() recorder.Config { return recorder.Config{ - Requests: cfg.Runner.Requests, - Concurrency: cfg.Runner.Concurrency, - Interval: cfg.Runner.Interval, - RequestTimeout: cfg.Runner.RequestTimeout, - GlobalTimeout: cfg.Runner.GlobalTimeout, - OnProgress: onRecordingProgress, + Requests: r.Requests, + Concurrency: r.Concurrency, + Interval: r.Interval, + RequestTimeout: r.RequestTimeout, + GlobalTimeout: r.GlobalTimeout, + OnProgress: r.onRecordingProgress, + } +} + +// Validate returns a non-nil InvalidConfigError if any of its fields +// does not meet the requirements. +func (r Runner) Validate() error { //nolint:gocognit + errs := []error{} + appendError := func(err error) { + errs = append(errs, err) } + + if r.Request == nil { + appendError(errors.New("unexpected nil request")) + } + + if r.Requests < 1 && r.Requests != -1 { + appendError(fmt.Errorf("requests (%d): want >= 0", r.Requests)) + } + + if r.Concurrency < 1 || r.Concurrency > r.Requests { + appendError(fmt.Errorf( + "concurrency (%d): want > 0 and <= requests (%d)", + r.Concurrency, r.Requests, + )) + } + + if r.Interval < 0 { + appendError(fmt.Errorf("interval (%d): want >= 0", r.Interval)) + } + + if r.RequestTimeout < 1 { + appendError(fmt.Errorf("requestTimeout (%d): want > 0", r.RequestTimeout)) + } + + if r.GlobalTimeout < 1 { + appendError(fmt.Errorf("globalTimeout (%d): want > 0", r.GlobalTimeout)) + } + + if len(errs) > 0 { + return &InvalidRunnerError{errs} + } + + return nil } diff --git a/runner/config_test.go b/runner/runner_test.go similarity index 70% rename from runner/config_test.go rename to runner/runner_test.go index c3a2734..ebf1a2a 100644 --- a/runner/config_test.go +++ b/runner/runner_test.go @@ -8,41 +8,38 @@ import ( "github.com/benchttp/engine/runner" ) -func TestConfig_Validate(t *testing.T) { +func TestRunner_Validate(t *testing.T) { t.Run("return nil if config is valid", func(t *testing.T) { - cfg := runner.Config{ - Request: validRequest(), - Runner: runner.RunnerConfig{ - Requests: 5, - Concurrency: 5, - Interval: 5, - RequestTimeout: 5, - GlobalTimeout: 5, - }, + brunner := runner.Runner{ + Request: validRequest(), + Requests: 5, + Concurrency: 5, + Interval: 5, + RequestTimeout: 5, + GlobalTimeout: 5, } - if err := cfg.Validate(); err != nil { + + if err := brunner.Validate(); err != nil { t.Errorf("unexpected error: %v", err) } }) t.Run("return cumulated errors if config is invalid", func(t *testing.T) { - cfg := runner.Config{ - Request: nil, - Runner: runner.RunnerConfig{ - Requests: -5, - Concurrency: -5, - Interval: -5, - RequestTimeout: -5, - GlobalTimeout: -5, - }, + brunner := runner.Runner{ + Request: nil, + Requests: -5, + Concurrency: -5, + Interval: -5, + RequestTimeout: -5, + GlobalTimeout: -5, } - err := cfg.Validate() + err := brunner.Validate() if err == nil { t.Fatal("invalid configuration considered valid") } - var errInvalid *runner.InvalidConfigError + var errInvalid *runner.InvalidRunnerError if !errors.As(err, &errInvalid) { t.Fatalf("unexpected error type: %T", err) } From 5cd1c8041e7fc7a95703bc91bc957211525448d7 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 22 Oct 2022 23:18:36 +0200 Subject: [PATCH 10/25] refactor(runner): use httptest in unit tests --- runner/runner_test.go | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/runner/runner_test.go b/runner/runner_test.go index ebf1a2a..e6fe4d5 100644 --- a/runner/runner_test.go +++ b/runner/runner_test.go @@ -2,7 +2,7 @@ package runner_test import ( "errors" - "net/http" + "net/http/httptest" "testing" "github.com/benchttp/engine/runner" @@ -11,7 +11,7 @@ import ( func TestRunner_Validate(t *testing.T) { t.Run("return nil if config is valid", func(t *testing.T) { brunner := runner.Runner{ - Request: validRequest(), + Request: httptest.NewRequest("GET", "https://a.b/#c?d=e&f=g", nil), Requests: 5, Concurrency: 5, Interval: 5, @@ -45,12 +45,12 @@ func TestRunner_Validate(t *testing.T) { } errs := errInvalid.Errors - findErrorOrFail(t, errs, `unexpected nil request`) - findErrorOrFail(t, errs, `requests (-5): want >= 0`) - findErrorOrFail(t, errs, `concurrency (-5): want > 0 and <= requests (-5)`) - findErrorOrFail(t, errs, `interval (-5): want >= 0`) - findErrorOrFail(t, errs, `requestTimeout (-5): want > 0`) - findErrorOrFail(t, errs, `globalTimeout (-5): want > 0`) + assertError(t, errs, `unexpected nil request`) + assertError(t, errs, `requests (-5): want >= 0`) + assertError(t, errs, `concurrency (-5): want > 0 and <= requests (-5)`) + assertError(t, errs, `interval (-5): want >= 0`) + assertError(t, errs, `requestTimeout (-5): want > 0`) + assertError(t, errs, `globalTimeout (-5): want > 0`) t.Logf("got error:\n%v", errInvalid) }) @@ -58,16 +58,8 @@ func TestRunner_Validate(t *testing.T) { // helpers -func validRequest() *http.Request { - req, err := http.NewRequest("GET", "https://a.b#c?d=e&f=g", nil) - if err != nil { - panic(err) - } - return req -} - -// findErrorOrFail fails t if no error in src matches msg. -func findErrorOrFail(t *testing.T, src []error, msg string) { +// assertError fails t if no error in src matches msg. +func assertError(t *testing.T, src []error, msg string) { t.Helper() for _, err := range src { if err.Error() == msg { From da47ab3e95fbd7996450e99802b55490b80b19b4 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 22 Oct 2022 23:22:51 +0200 Subject: [PATCH 11/25] chore: remove obsolete env file --- .env.development | 1 - 1 file changed, 1 deletion(-) delete mode 100644 .env.development diff --git a/.env.development b/.env.development deleted file mode 100644 index 98da8de..0000000 --- a/.env.development +++ /dev/null @@ -1 +0,0 @@ -SERVER_PORT=8080 From 721986a6898cb893c3e0edbc257cd808418dc9d1 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 22 Oct 2022 23:23:29 +0200 Subject: [PATCH 12/25] chore: clean up .gitignore --- .gitignore | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.gitignore b/.gitignore index 575893a..a960199 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,3 @@ -# Env files -.env - -# Binary files -/bin - # IDE files /.vscode /.idea @@ -12,6 +6,3 @@ /.benchttp.yml /.benchttp.yaml /.benchttp.json - -# Benchttp reports -/benchttp.report.*.json From 96e99e70de0dc31401d294e681d3002771115133 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 22 Oct 2022 23:31:37 +0200 Subject: [PATCH 13/25] chore: remove deprecated linters --- .golangci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 786a2a0..04d396e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -131,7 +131,6 @@ linters: disable-all: true enable: - bodyclose # enforce resp.Body.Close() - - deadcode - dupl # duplicate code - errcheck - exportloopref @@ -145,10 +144,8 @@ linters: - prealloc # enforce capacity allocation when possible - revive # golint enhancement - staticcheck # go vet enhancement - - structcheck # unused struct fields - testpackage # checks on tests (*_test) - thelper # enforce t.Helper() - - varcheck # unused global var and const - wastedassign fast: false From 930c0eadf08cd45b780ff79bf929f534255b7889 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 16:56:41 +0200 Subject: [PATCH 14/25] refactor(runner): remove unnecessary constructor --- runner/report.go | 4 ++-- runner/runner.go | 19 ++++++++----------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/runner/report.go b/runner/report.go index a0d959d..f1cde3e 100644 --- a/runner/report.go +++ b/runner/report.go @@ -23,7 +23,7 @@ type Metadata struct { // newReport returns an initialized *Report. func newReport( - cfg Runner, + r Runner, d time.Duration, m metrics.Aggregate, t tests.SuiteResult, @@ -32,7 +32,7 @@ func newReport( Metrics: m, Tests: t, Metadata: Metadata{ - Config: cfg, + Config: r, FinishedAt: time.Now(), // TODO: change, unreliable TotalDuration: d, }, diff --git a/runner/runner.go b/runner/runner.go index e421911..ff052b9 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -47,17 +47,14 @@ type Runner struct { Tests []tests.Case - recorder *recorder.Recorder - onRecordingProgress func(RecordingProgress) -} + OnProgress func(RecordingProgress) -func New(onRecordingProgress func(RecordingProgress)) *Runner { - return &Runner{onRecordingProgress: onRecordingProgress} + recorder *recorder.Recorder } -func (r *Runner) Run(ctx context.Context, cfg Runner) (*Report, error) { +func (r *Runner) Run(ctx context.Context) (*Report, error) { // Validate input config - if err := cfg.Validate(); err != nil { + if err := r.Validate(); err != nil { return nil, err } @@ -67,7 +64,7 @@ func (r *Runner) Run(ctx context.Context, cfg Runner) (*Report, error) { startTime := time.Now() // Run request recorder - records, err := r.recorder.Record(ctx, cfg.Request) + records, err := r.recorder.Record(ctx, r.Request) if err != nil { return nil, err } @@ -76,9 +73,9 @@ func (r *Runner) Run(ctx context.Context, cfg Runner) (*Report, error) { agg := metrics.NewAggregate(records) - testResults := tests.Run(agg, cfg.Tests) + testResults := tests.Run(agg, r.Tests) - return newReport(cfg, duration, agg, testResults), nil + return newReport(*r, duration, agg, testResults), nil } // Progress returns the current progress of the recording. @@ -99,7 +96,7 @@ func (r *Runner) recorderConfig() recorder.Config { Interval: r.Interval, RequestTimeout: r.RequestTimeout, GlobalTimeout: r.GlobalTimeout, - OnProgress: r.onRecordingProgress, + OnProgress: r.OnProgress, } } From e360b5ebec146bc5af5796839ef7f3d0f0bff3fa Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 16:56:55 +0200 Subject: [PATCH 15/25] docs: update docs --- README.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index feb7365..a07ddb0 100644 --- a/README.md +++ b/README.md @@ -46,12 +46,15 @@ import ( ) func main(t *testing.T) { - // Set runner configuration - config := runner.DefaultConfig() - config.Request = config.Request.WithURL("https://example.com") + // Set the request to send + req, _ := http.NewRequest("GET", "http://localhost:3000", nil) - // Instantiate runner and run benchmark - report, _ := runner.New(nil).Run(context.Background(), config) + // Configure the runner + rnr := runner.DefaultRunner() + rnr.Request = req + + // Run benchmark, get report + report, _ := rnr.Run(context.Background()) fmt.Println(report.Metrics.ResponseTimes.Mean) } @@ -67,7 +70,6 @@ import ( "fmt" "github.com/benchttp/engine/configparse" - "github.com/benchttp/engine/runner" ) func main() { @@ -75,12 +77,12 @@ func main() { jsonConfig := []byte(` { "request": { - "url": "https://example.com" + "url": "http://localhost:9999" } }`) - config, _ := configparse.JSON(jsonConfig) - report, _ := runner.New(nil).Run(context.Background(), config) + runner, _ := configparse.JSON(jsonConfig) + report, _ := runner.Run(context.Background()) fmt.Println(report.Metrics.ResponseTimes.Mean) } From e5cfd1e0362e28f226de0b8ad24363bba37fe9a6 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 18:21:14 +0200 Subject: [PATCH 16/25] refactor(configparse): JSON: use pointer destination param --- configparse/json.go | 15 ++++----------- configparse/json_test.go | 5 +++-- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/configparse/json.go b/configparse/json.go index 897c44f..b48a5ac 100644 --- a/configparse/json.go +++ b/configparse/json.go @@ -5,17 +5,10 @@ import ( ) // JSON reads input bytes as JSON and unmarshals it into a runner.Runner. -func JSON(in []byte) (runner.Runner, error) { - parser := JSONParser{} +func JSON(in []byte, dst *runner.Runner) error { repr := Representation{} - if err := parser.Parse(in, &repr); err != nil { - return runner.Runner{}, err + if err := (JSONParser{}).Parse(in, &repr); err != nil { + return err } - - cfg := runner.DefaultRunner() - if err := repr.ParseInto(&cfg); err != nil { - return runner.Runner{}, err - } - - return cfg, nil + return repr.ParseInto(dst) } diff --git a/configparse/json_test.go b/configparse/json_test.go index 4d76c10..6e14a14 100644 --- a/configparse/json_test.go +++ b/configparse/json_test.go @@ -60,11 +60,12 @@ func TestJSON(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - gotRunner, gotError := configparse.JSON(tc.input) + gotRunner := runner.DefaultRunner() + gotError := configparse.JSON(tc.input, &gotRunner) + if !tc.isValidRunner(gotRunner) { t.Errorf("unexpected config:\n%+v", gotRunner) } - if !sameErrors(gotError, tc.expError) { t.Errorf("unexpected error:\nexp %v,\ngot %v", tc.expError, gotError) } From 749fb6aa59ce261700c98b5c31f29712b7f5a49c Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 18:57:07 +0200 Subject: [PATCH 17/25] refactor: rename module and packages - module github.com/benchttp/engine -> github.com/benchttp/sdk - package runner -> benchttp --- .golangci.yml | 2 +- Makefile | 2 +- README.md | 24 ++++++++--------- {runner => benchttp}/default.go | 4 +-- {runner => benchttp}/error.go | 2 +- {runner => benchttp}/error_test.go | 6 ++--- .../internal/metrics/aggregate.go | 4 +-- .../internal/metrics/aggregate_test.go | 6 ++--- .../internal/metrics/compare.go | 0 .../internal/metrics/field.go | 2 +- .../internal/metrics/field_test.go | 2 +- .../internal/metrics/metrics.go | 2 +- .../internal/metrics/metrics_test.go | 4 +-- .../internal/metrics/timestats/sort.go | 0 .../internal/metrics/timestats/timestats.go | 0 .../metrics/timestats/timestats_test.go | 2 +- .../internal/recorder/error.go | 0 .../internal/recorder/httputil.go | 0 .../internal/recorder/progress.go | 0 .../internal/recorder/recorder.go | 2 +- .../recorder/recorder_internal_test.go | 2 +- .../internal/recorder/tracer.go | 0 .../internal/recorder/tracer_internal_test.go | 0 .../internal/reflectpath/resolver.go | 0 .../internal/reflectpath/type.go | 0 .../internal/reflectpath/value.go | 0 .../internal/tests/predicate.go | 4 +-- .../internal/tests/predicate_test.go | 4 +-- {runner => benchttp}/internal/tests/tests.go | 2 +- .../internal/tests/tests_test.go | 6 ++--- {runner => benchttp}/report.go | 6 ++--- {runner => benchttp}/runner.go | 8 +++--- {runner => benchttp}/runner_test.go | 14 +++++----- configparse/json.go | 8 +++--- configparse/json_test.go | 16 ++++++------ configparse/parse.go | 26 +++++++++---------- configparse/parser_json_test.go | 2 +- configparse/parser_yaml_test.go | 2 +- go.mod | 2 +- internal/dispatcher/dispatcher_test.go | 2 +- 40 files changed, 83 insertions(+), 85 deletions(-) rename {runner => benchttp}/default.go (88%) rename {runner => benchttp}/error.go (96%) rename {runner => benchttp}/error_test.go (80%) rename {runner => benchttp}/internal/metrics/aggregate.go (96%) rename {runner => benchttp}/internal/metrics/aggregate_test.go (94%) rename {runner => benchttp}/internal/metrics/compare.go (100%) rename {runner => benchttp}/internal/metrics/field.go (94%) rename {runner => benchttp}/internal/metrics/field_test.go (96%) rename {runner => benchttp}/internal/metrics/metrics.go (97%) rename {runner => benchttp}/internal/metrics/metrics_test.go (96%) rename {runner => benchttp}/internal/metrics/timestats/sort.go (100%) rename {runner => benchttp}/internal/metrics/timestats/timestats.go (100%) rename {runner => benchttp}/internal/metrics/timestats/timestats_test.go (97%) rename {runner => benchttp}/internal/recorder/error.go (100%) rename {runner => benchttp}/internal/recorder/httputil.go (100%) rename {runner => benchttp}/internal/recorder/progress.go (100%) rename {runner => benchttp}/internal/recorder/recorder.go (98%) rename {runner => benchttp}/internal/recorder/recorder_internal_test.go (99%) rename {runner => benchttp}/internal/recorder/tracer.go (100%) rename {runner => benchttp}/internal/recorder/tracer_internal_test.go (100%) rename {runner => benchttp}/internal/reflectpath/resolver.go (100%) rename {runner => benchttp}/internal/reflectpath/type.go (100%) rename {runner => benchttp}/internal/reflectpath/value.go (100%) rename {runner => benchttp}/internal/tests/predicate.go (91%) rename {runner => benchttp}/internal/tests/predicate_test.go (94%) rename {runner => benchttp}/internal/tests/tests.go (95%) rename {runner => benchttp}/internal/tests/tests_test.go (95%) rename {runner => benchttp}/report.go (84%) rename {runner => benchttp}/runner.go (94%) rename {runner => benchttp}/runner_test.go (86%) diff --git a/.golangci.yml b/.golangci.yml index 04d396e..d9e410a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -111,7 +111,7 @@ linters-settings: extra-rules: true goimports: - local-prefixes: github.com/benchttp/engine + local-prefixes: github.com/benchttp/sdk misspell: locale: US diff --git a/Makefile b/Makefile index 5f4e1e0..0ed0f52 100644 --- a/Makefile +++ b/Makefile @@ -41,5 +41,5 @@ test-cov: .PHONY: docs docs: - @echo "\033[4mhttp://localhost:9995/pkg/github.com/benchttp/engine/\033[0m" + @echo "\033[4mhttp://localhost:9995/pkg/github.com/benchttp/sdk/\033[0m" @godoc -http=localhost:9995 diff --git a/README.md b/README.md index a07ddb0..a11bf1b 100644 --- a/README.md +++ b/README.md @@ -5,12 +5,12 @@ Github Worklow Status Code coverage - - Go Report Card + + Go Report Card
- + Go package Reference - + Latest version

@@ -28,7 +28,7 @@ Go1.17 environment or higher is required. Install. ```txt -go get github.com/benchttp/engine +go get github.com/benchttp/sdk ``` ## Usage @@ -42,19 +42,19 @@ import ( "context" "fmt" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" ) func main(t *testing.T) { // Set the request to send - req, _ := http.NewRequest("GET", "http://localhost:3000", nil) + request, _ := http.NewRequest("GET", "http://localhost:3000", nil) // Configure the runner - rnr := runner.DefaultRunner() - rnr.Request = req + runner := runner.DefaultRunner() + runner.Request = request // Run benchmark, get report - report, _ := rnr.Run(context.Background()) + report, _ := runner.Run(context.Background()) fmt.Println(report.Metrics.ResponseTimes.Mean) } @@ -69,7 +69,7 @@ import ( "context" "fmt" - "github.com/benchttp/engine/configparse" + "github.com/benchttp/sdk/configparse" ) func main() { @@ -88,7 +88,7 @@ func main() { } ``` -📄 Please refer to [our Wiki](https://github.com/benchttp/engine/wiki/IO-Structures) for exhaustive `Config` and `Report` structures (and more!) +📄 Please refer to [our Wiki](https://github.com/benchttp/sdk/wiki/IO-Structures) for exhaustive `Runner` and `Report` structures (and more!) ## Development diff --git a/runner/default.go b/benchttp/default.go similarity index 88% rename from runner/default.go rename to benchttp/default.go index 32ca603..545cf15 100644 --- a/runner/default.go +++ b/benchttp/default.go @@ -1,4 +1,4 @@ -package runner +package benchttp import ( "fmt" @@ -24,7 +24,7 @@ var defaultRunner = Runner{ func defaultRequest() *http.Request { req, err := http.NewRequest("GET", "", nil) if err != nil { - panic(fmt.Sprintf("benchttp/runner: %s", err)) + panic(fmt.Sprintf("benchttp: %s", err)) } return req } diff --git a/runner/error.go b/benchttp/error.go similarity index 96% rename from runner/error.go rename to benchttp/error.go index 98f82ba..19ceeac 100644 --- a/runner/error.go +++ b/benchttp/error.go @@ -1,4 +1,4 @@ -package runner +package benchttp import "strings" diff --git a/runner/error_test.go b/benchttp/error_test.go similarity index 80% rename from runner/error_test.go rename to benchttp/error_test.go index 8b1aad7..cdbb1c9 100644 --- a/runner/error_test.go +++ b/benchttp/error_test.go @@ -1,14 +1,14 @@ -package runner_test +package benchttp_test import ( "errors" "testing" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" ) func TestInvalidRunnerError(t *testing.T) { - e := runner.InvalidRunnerError{ + e := benchttp.InvalidRunnerError{ Errors: []error{ errors.New("error 0"), errors.New("error 1\nwith new line"), diff --git a/runner/internal/metrics/aggregate.go b/benchttp/internal/metrics/aggregate.go similarity index 96% rename from runner/internal/metrics/aggregate.go rename to benchttp/internal/metrics/aggregate.go index 7a40929..b44723e 100644 --- a/runner/internal/metrics/aggregate.go +++ b/benchttp/internal/metrics/aggregate.go @@ -3,8 +3,8 @@ package metrics import ( "time" - "github.com/benchttp/engine/runner/internal/metrics/timestats" - "github.com/benchttp/engine/runner/internal/recorder" + "github.com/benchttp/sdk/benchttp/internal/metrics/timestats" + "github.com/benchttp/sdk/benchttp/internal/recorder" ) type TimeStats = timestats.TimeStats diff --git a/runner/internal/metrics/aggregate_test.go b/benchttp/internal/metrics/aggregate_test.go similarity index 94% rename from runner/internal/metrics/aggregate_test.go rename to benchttp/internal/metrics/aggregate_test.go index 5883a20..a2af956 100644 --- a/runner/internal/metrics/aggregate_test.go +++ b/benchttp/internal/metrics/aggregate_test.go @@ -5,9 +5,9 @@ import ( "testing" "time" - "github.com/benchttp/engine/runner/internal/metrics" - "github.com/benchttp/engine/runner/internal/metrics/timestats" - "github.com/benchttp/engine/runner/internal/recorder" + "github.com/benchttp/sdk/benchttp/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/metrics/timestats" + "github.com/benchttp/sdk/benchttp/internal/recorder" ) func TestNewAggregate(t *testing.T) { diff --git a/runner/internal/metrics/compare.go b/benchttp/internal/metrics/compare.go similarity index 100% rename from runner/internal/metrics/compare.go rename to benchttp/internal/metrics/compare.go diff --git a/runner/internal/metrics/field.go b/benchttp/internal/metrics/field.go similarity index 94% rename from runner/internal/metrics/field.go rename to benchttp/internal/metrics/field.go index e1a8d73..2228ab2 100644 --- a/runner/internal/metrics/field.go +++ b/benchttp/internal/metrics/field.go @@ -3,7 +3,7 @@ package metrics import ( "errors" - "github.com/benchttp/engine/internal/errorutil" + "github.com/benchttp/sdk/internal/errorutil" ) // ErrUnknownField occurs when a Field is used with an invalid path. diff --git a/runner/internal/metrics/field_test.go b/benchttp/internal/metrics/field_test.go similarity index 96% rename from runner/internal/metrics/field_test.go rename to benchttp/internal/metrics/field_test.go index f3a32c0..41cea87 100644 --- a/runner/internal/metrics/field_test.go +++ b/benchttp/internal/metrics/field_test.go @@ -3,7 +3,7 @@ package metrics_test import ( "testing" - "github.com/benchttp/engine/runner/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/metrics" ) func TestField_Type(t *testing.T) { diff --git a/runner/internal/metrics/metrics.go b/benchttp/internal/metrics/metrics.go similarity index 97% rename from runner/internal/metrics/metrics.go rename to benchttp/internal/metrics/metrics.go index 61e946d..5eb51ea 100644 --- a/runner/internal/metrics/metrics.go +++ b/benchttp/internal/metrics/metrics.go @@ -3,7 +3,7 @@ package metrics import ( "strings" - "github.com/benchttp/engine/runner/internal/reflectpath" + "github.com/benchttp/sdk/benchttp/internal/reflectpath" ) // Value is a concrete metric value, e.g. 120 or 3 * time.Second. diff --git a/runner/internal/metrics/metrics_test.go b/benchttp/internal/metrics/metrics_test.go similarity index 96% rename from runner/internal/metrics/metrics_test.go rename to benchttp/internal/metrics/metrics_test.go index b9ff933..b1cab65 100644 --- a/runner/internal/metrics/metrics_test.go +++ b/benchttp/internal/metrics/metrics_test.go @@ -4,8 +4,8 @@ import ( "testing" "time" - "github.com/benchttp/engine/runner/internal/metrics" - "github.com/benchttp/engine/runner/internal/metrics/timestats" + "github.com/benchttp/sdk/benchttp/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/metrics/timestats" ) func TestMetric_Compare(t *testing.T) { diff --git a/runner/internal/metrics/timestats/sort.go b/benchttp/internal/metrics/timestats/sort.go similarity index 100% rename from runner/internal/metrics/timestats/sort.go rename to benchttp/internal/metrics/timestats/sort.go diff --git a/runner/internal/metrics/timestats/timestats.go b/benchttp/internal/metrics/timestats/timestats.go similarity index 100% rename from runner/internal/metrics/timestats/timestats.go rename to benchttp/internal/metrics/timestats/timestats.go diff --git a/runner/internal/metrics/timestats/timestats_test.go b/benchttp/internal/metrics/timestats/timestats_test.go similarity index 97% rename from runner/internal/metrics/timestats/timestats_test.go rename to benchttp/internal/metrics/timestats/timestats_test.go index e3854f4..65c0537 100644 --- a/runner/internal/metrics/timestats/timestats_test.go +++ b/benchttp/internal/metrics/timestats/timestats_test.go @@ -4,7 +4,7 @@ import ( "testing" "time" - "github.com/benchttp/engine/runner/internal/metrics/timestats" + "github.com/benchttp/sdk/benchttp/internal/metrics/timestats" ) func TestCompute(t *testing.T) { diff --git a/runner/internal/recorder/error.go b/benchttp/internal/recorder/error.go similarity index 100% rename from runner/internal/recorder/error.go rename to benchttp/internal/recorder/error.go diff --git a/runner/internal/recorder/httputil.go b/benchttp/internal/recorder/httputil.go similarity index 100% rename from runner/internal/recorder/httputil.go rename to benchttp/internal/recorder/httputil.go diff --git a/runner/internal/recorder/progress.go b/benchttp/internal/recorder/progress.go similarity index 100% rename from runner/internal/recorder/progress.go rename to benchttp/internal/recorder/progress.go diff --git a/runner/internal/recorder/recorder.go b/benchttp/internal/recorder/recorder.go similarity index 98% rename from runner/internal/recorder/recorder.go rename to benchttp/internal/recorder/recorder.go index 1e9be56..54fc0ac 100644 --- a/runner/internal/recorder/recorder.go +++ b/benchttp/internal/recorder/recorder.go @@ -7,7 +7,7 @@ import ( "sync" "time" - "github.com/benchttp/engine/internal/dispatcher" + "github.com/benchttp/sdk/internal/dispatcher" ) const ( diff --git a/runner/internal/recorder/recorder_internal_test.go b/benchttp/internal/recorder/recorder_internal_test.go similarity index 99% rename from runner/internal/recorder/recorder_internal_test.go rename to benchttp/internal/recorder/recorder_internal_test.go index bc79fe8..9ef9bf8 100644 --- a/runner/internal/recorder/recorder_internal_test.go +++ b/benchttp/internal/recorder/recorder_internal_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/benchttp/engine/internal/dispatcher" + "github.com/benchttp/sdk/internal/dispatcher" ) var errTest = errors.New("test-generated error") diff --git a/runner/internal/recorder/tracer.go b/benchttp/internal/recorder/tracer.go similarity index 100% rename from runner/internal/recorder/tracer.go rename to benchttp/internal/recorder/tracer.go diff --git a/runner/internal/recorder/tracer_internal_test.go b/benchttp/internal/recorder/tracer_internal_test.go similarity index 100% rename from runner/internal/recorder/tracer_internal_test.go rename to benchttp/internal/recorder/tracer_internal_test.go diff --git a/runner/internal/reflectpath/resolver.go b/benchttp/internal/reflectpath/resolver.go similarity index 100% rename from runner/internal/reflectpath/resolver.go rename to benchttp/internal/reflectpath/resolver.go diff --git a/runner/internal/reflectpath/type.go b/benchttp/internal/reflectpath/type.go similarity index 100% rename from runner/internal/reflectpath/type.go rename to benchttp/internal/reflectpath/type.go diff --git a/runner/internal/reflectpath/value.go b/benchttp/internal/reflectpath/value.go similarity index 100% rename from runner/internal/reflectpath/value.go rename to benchttp/internal/reflectpath/value.go diff --git a/runner/internal/tests/predicate.go b/benchttp/internal/tests/predicate.go similarity index 91% rename from runner/internal/tests/predicate.go rename to benchttp/internal/tests/predicate.go index d8146d0..17e508f 100644 --- a/runner/internal/tests/predicate.go +++ b/benchttp/internal/tests/predicate.go @@ -3,8 +3,8 @@ package tests import ( "errors" - "github.com/benchttp/engine/internal/errorutil" - "github.com/benchttp/engine/runner/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/metrics" + "github.com/benchttp/sdk/internal/errorutil" ) var ErrUnknownPredicate = errors.New("tests: unknown predicate") diff --git a/runner/internal/tests/predicate_test.go b/benchttp/internal/tests/predicate_test.go similarity index 94% rename from runner/internal/tests/predicate_test.go rename to benchttp/internal/tests/predicate_test.go index f9aee16..b70ce87 100644 --- a/runner/internal/tests/predicate_test.go +++ b/benchttp/internal/tests/predicate_test.go @@ -4,8 +4,8 @@ import ( "testing" "time" - "github.com/benchttp/engine/runner/internal/metrics" - "github.com/benchttp/engine/runner/internal/tests" + "github.com/benchttp/sdk/benchttp/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/tests" ) func TestPredicate(t *testing.T) { diff --git a/runner/internal/tests/tests.go b/benchttp/internal/tests/tests.go similarity index 95% rename from runner/internal/tests/tests.go rename to benchttp/internal/tests/tests.go index a3e3f36..3fb03cf 100644 --- a/runner/internal/tests/tests.go +++ b/benchttp/internal/tests/tests.go @@ -3,7 +3,7 @@ package tests import ( "fmt" - "github.com/benchttp/engine/runner/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/metrics" ) type Case struct { diff --git a/runner/internal/tests/tests_test.go b/benchttp/internal/tests/tests_test.go similarity index 95% rename from runner/internal/tests/tests_test.go rename to benchttp/internal/tests/tests_test.go index a358601..6c58019 100644 --- a/runner/internal/tests/tests_test.go +++ b/benchttp/internal/tests/tests_test.go @@ -5,9 +5,9 @@ import ( "testing" "time" - "github.com/benchttp/engine/runner/internal/metrics" - "github.com/benchttp/engine/runner/internal/metrics/timestats" - "github.com/benchttp/engine/runner/internal/tests" + "github.com/benchttp/sdk/benchttp/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/metrics/timestats" + "github.com/benchttp/sdk/benchttp/internal/tests" ) func TestRun(t *testing.T) { diff --git a/runner/report.go b/benchttp/report.go similarity index 84% rename from runner/report.go rename to benchttp/report.go index f1cde3e..da11c31 100644 --- a/runner/report.go +++ b/benchttp/report.go @@ -1,10 +1,10 @@ -package runner +package benchttp import ( "time" - "github.com/benchttp/engine/runner/internal/metrics" - "github.com/benchttp/engine/runner/internal/tests" + "github.com/benchttp/sdk/benchttp/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/tests" ) // Report represents a run result as exported by the runner. diff --git a/runner/runner.go b/benchttp/runner.go similarity index 94% rename from runner/runner.go rename to benchttp/runner.go index ff052b9..ef28f32 100644 --- a/runner/runner.go +++ b/benchttp/runner.go @@ -1,4 +1,4 @@ -package runner +package benchttp import ( "context" @@ -7,9 +7,9 @@ import ( "net/http" "time" - "github.com/benchttp/engine/runner/internal/metrics" - "github.com/benchttp/engine/runner/internal/recorder" - "github.com/benchttp/engine/runner/internal/tests" + "github.com/benchttp/sdk/benchttp/internal/metrics" + "github.com/benchttp/sdk/benchttp/internal/recorder" + "github.com/benchttp/sdk/benchttp/internal/tests" ) type ( diff --git a/runner/runner_test.go b/benchttp/runner_test.go similarity index 86% rename from runner/runner_test.go rename to benchttp/runner_test.go index e6fe4d5..09bbc1c 100644 --- a/runner/runner_test.go +++ b/benchttp/runner_test.go @@ -1,16 +1,16 @@ -package runner_test +package benchttp_test import ( "errors" "net/http/httptest" "testing" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" ) func TestRunner_Validate(t *testing.T) { t.Run("return nil if config is valid", func(t *testing.T) { - brunner := runner.Runner{ + runner := benchttp.Runner{ Request: httptest.NewRequest("GET", "https://a.b/#c?d=e&f=g", nil), Requests: 5, Concurrency: 5, @@ -19,13 +19,13 @@ func TestRunner_Validate(t *testing.T) { GlobalTimeout: 5, } - if err := brunner.Validate(); err != nil { + if err := runner.Validate(); err != nil { t.Errorf("unexpected error: %v", err) } }) t.Run("return cumulated errors if config is invalid", func(t *testing.T) { - brunner := runner.Runner{ + runner := benchttp.Runner{ Request: nil, Requests: -5, Concurrency: -5, @@ -34,12 +34,12 @@ func TestRunner_Validate(t *testing.T) { GlobalTimeout: -5, } - err := brunner.Validate() + err := runner.Validate() if err == nil { t.Fatal("invalid configuration considered valid") } - var errInvalid *runner.InvalidRunnerError + var errInvalid *benchttp.InvalidRunnerError if !errors.As(err, &errInvalid) { t.Fatalf("unexpected error type: %T", err) } diff --git a/configparse/json.go b/configparse/json.go index b48a5ac..c255f24 100644 --- a/configparse/json.go +++ b/configparse/json.go @@ -1,11 +1,9 @@ package configparse -import ( - "github.com/benchttp/engine/runner" -) +import "github.com/benchttp/sdk/benchttp" -// JSON reads input bytes as JSON and unmarshals it into a runner.Runner. -func JSON(in []byte, dst *runner.Runner) error { +// JSON reads input bytes as JSON and unmarshals it into a benchttp.Runner. +func JSON(in []byte, dst *benchttp.Runner) error { repr := Representation{} if err := (JSONParser{}).Parse(in, &repr); err != nil { return err diff --git a/configparse/json_test.go b/configparse/json_test.go index 6e14a14..0809be9 100644 --- a/configparse/json_test.go +++ b/configparse/json_test.go @@ -5,8 +5,8 @@ import ( "errors" "testing" - "github.com/benchttp/engine/configparse" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" + "github.com/benchttp/sdk/configparse" ) func TestJSON(t *testing.T) { @@ -19,7 +19,7 @@ func TestJSON(t *testing.T) { testcases := []struct { name string input []byte - isValidRunner func(runner.Runner) bool + isValidRunner func(benchttp.Runner) bool expError error }{ { @@ -27,7 +27,7 @@ func TestJSON(t *testing.T) { input: baseInput.assign(object{ "badkey": "marcel-patulacci", }).json(), - isValidRunner: func(cfg runner.Runner) bool { return true }, + isValidRunner: func(cfg benchttp.Runner) bool { return true }, expError: errors.New(`invalid field ("badkey"): does not exist`), }, { @@ -37,7 +37,7 @@ func TestJSON(t *testing.T) { "concurrency": "bad value", // want int }, }).json(), - isValidRunner: func(runner.Runner) bool { return true }, + isValidRunner: func(benchttp.Runner) bool { return true }, expError: errors.New(`wrong type for field runner.concurrency: want int, got string`), }, { @@ -45,8 +45,8 @@ func TestJSON(t *testing.T) { input: baseInput.assign(object{ "runner": object{"concurrency": 3}, }).json(), - isValidRunner: func(r runner.Runner) bool { - defaultRunner := runner.DefaultRunner() + isValidRunner: func(r benchttp.Runner) bool { + defaultRunner := benchttp.DefaultRunner() isInputValueParsed := r.Concurrency == 3 isMergedWithDefault := r.Request.Method == defaultRunner.Request.Method && @@ -60,7 +60,7 @@ func TestJSON(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - gotRunner := runner.DefaultRunner() + gotRunner := benchttp.DefaultRunner() gotError := configparse.JSON(tc.input, &gotRunner) if !tc.isValidRunner(gotRunner) { diff --git a/configparse/parse.go b/configparse/parse.go index a33e8e8..54e3938 100644 --- a/configparse/parse.go +++ b/configparse/parse.go @@ -10,7 +10,7 @@ import ( "strconv" "time" - "github.com/benchttp/engine/runner" + "github.com/benchttp/sdk/benchttp" ) // Representation is a raw data model for formatted runner config (json, yaml). @@ -48,10 +48,10 @@ type Representation struct { } `yaml:"tests" json:"tests"` } -// ParseInto parses the Representation receiver as a runner.Runner +// ParseInto parses the Representation receiver as a benchttp.Runner // and stores any non-nil field value into the corresponding field // of dst. -func (repr Representation) ParseInto(dst *runner.Runner) error { +func (repr Representation) ParseInto(dst *benchttp.Runner) error { if err := repr.parseRequestInto(dst); err != nil { return err } @@ -61,7 +61,7 @@ func (repr Representation) ParseInto(dst *runner.Runner) error { return repr.parseTestsInto(dst) } -func (repr Representation) parseRequestInto(dst *runner.Runner) error { +func (repr Representation) parseRequestInto(dst *benchttp.Runner) error { if dst.Request == nil { dst.Request = &http.Request{} } @@ -98,7 +98,7 @@ func (repr Representation) parseRequestInto(dst *runner.Runner) error { return nil } -func (repr Representation) parseRunnerInto(dst *runner.Runner) error { +func (repr Representation) parseRunnerInto(dst *benchttp.Runner) error { if requests := repr.Runner.Requests; requests != nil { dst.Requests = *requests } @@ -134,13 +134,13 @@ func (repr Representation) parseRunnerInto(dst *runner.Runner) error { return nil } -func (repr Representation) parseTestsInto(dst *runner.Runner) error { +func (repr Representation) parseTestsInto(dst *benchttp.Runner) error { testSuite := repr.Tests if len(testSuite) == 0 { return nil } - cases := make([]runner.TestCase, len(testSuite)) + cases := make([]benchttp.TestCase, len(testSuite)) for i, t := range testSuite { fieldPath := func(caseField string) string { return fmt.Sprintf("tests[%d].%s", i, caseField) @@ -155,12 +155,12 @@ func (repr Representation) parseTestsInto(dst *runner.Runner) error { return err } - field := runner.MetricsField(*t.Field) + field := benchttp.MetricsField(*t.Field) if err := field.Validate(); err != nil { return fmt.Errorf("%s: %s", fieldPath("field"), err) } - predicate := runner.TestPredicate(*t.Predicate) + predicate := benchttp.TestPredicate(*t.Predicate) if err := predicate.Validate(); err != nil { return fmt.Errorf("%s: %s", fieldPath("predicate"), err) } @@ -170,7 +170,7 @@ func (repr Representation) parseTestsInto(dst *runner.Runner) error { return fmt.Errorf("%s: %s", fieldPath("target"), err) } - cases[i] = runner.TestCase{ + cases[i] = benchttp.TestCase{ Name: *t.Name, Field: field, Predicate: predicate, @@ -217,11 +217,11 @@ func parseOptionalDuration(raw string) (time.Duration, error) { } func parseMetricValue( - field runner.MetricsField, + field benchttp.MetricsField, inputValue string, -) (runner.MetricsValue, error) { +) (benchttp.MetricsValue, error) { fieldType := field.Type() - handleError := func(v interface{}, err error) (runner.MetricsValue, error) { + handleError := func(v interface{}, err error) (benchttp.MetricsValue, error) { if err != nil { return nil, fmt.Errorf( "value %q is incompatible with field %s (want %s)", diff --git a/configparse/parser_json_test.go b/configparse/parser_json_test.go index 51a4914..85deaae 100644 --- a/configparse/parser_json_test.go +++ b/configparse/parser_json_test.go @@ -3,7 +3,7 @@ package configparse_test import ( "testing" - "github.com/benchttp/engine/configparse" + "github.com/benchttp/sdk/configparse" ) func TestJSONParser(t *testing.T) { diff --git a/configparse/parser_yaml_test.go b/configparse/parser_yaml_test.go index e80439a..4b6ca8b 100644 --- a/configparse/parser_yaml_test.go +++ b/configparse/parser_yaml_test.go @@ -7,7 +7,7 @@ import ( "gopkg.in/yaml.v3" - "github.com/benchttp/engine/configparse" + "github.com/benchttp/sdk/configparse" ) func TestYAMLParser(t *testing.T) { diff --git a/go.mod b/go.mod index 23faa4d..06d8eb9 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/benchttp/engine +module github.com/benchttp/sdk go 1.17 diff --git a/internal/dispatcher/dispatcher_test.go b/internal/dispatcher/dispatcher_test.go index 6b5d9bd..953dad7 100644 --- a/internal/dispatcher/dispatcher_test.go +++ b/internal/dispatcher/dispatcher_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/benchttp/engine/internal/dispatcher" + "github.com/benchttp/sdk/internal/dispatcher" ) func TestNew(t *testing.T) { From a970ab902616086509e6f5b85bb09bfd95bdfce0 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 21:07:32 +0200 Subject: [PATCH 18/25] refactor: remove Makefile, use shell scripts --- .github/workflows/ci.yml | 4 ++-- Makefile | 45 ---------------------------------------- README.md | 10 ++++----- script/coverage | 3 +++ script/doc | 11 ++++++++++ script/lint | 3 +++ script/test | 8 +++++++ 7 files changed, 32 insertions(+), 52 deletions(-) delete mode 100644 Makefile create mode 100755 script/coverage create mode 100755 script/doc create mode 100755 script/lint create mode 100755 script/test diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b682a6..f41d910 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: go-version: 1.17 - name: Install coverage tool - run: go get github.com/ory/go-acc + run: go install github.com/ory/go-acc@v0.2.8 # Check #1: Lint - name: Lint @@ -28,7 +28,7 @@ jobs: # Check #2: Test & generate coverage report - name: Test & coverage - run: make test-cov + run: ./script/coverage - name: Upload coverage report uses: codecov/codecov-action@v1.0.2 diff --git a/Makefile b/Makefile deleted file mode 100644 index 0ed0f52..0000000 --- a/Makefile +++ /dev/null @@ -1,45 +0,0 @@ -# Default command - -.PHONY: default -default: - @make check - -# Check code - -.PHONY: check -check: - @make lint - @make tests - -.PHONY: lint -lint: - @golangci-lint run - -.PHONY: tests -tests: - @go test -race -timeout 10s ./... - -TEST_FUNC=^.*$$ -ifdef t -TEST_FUNC=$(t) -endif -TEST_PKG=./... -ifdef p -TEST_PKG=./$(p) -endif - -.PHONY: test -test: - @go test -race -timeout 10s -run $(TEST_FUNC) $(TEST_PKG) - - -.PHONY: test-cov -test-cov: - @go-acc ./... - -# Docs - -.PHONY: docs -docs: - @echo "\033[4mhttp://localhost:9995/pkg/github.com/benchttp/sdk/\033[0m" - @godoc -http=localhost:9995 diff --git a/README.md b/README.md index a11bf1b..dac188b 100644 --- a/README.md +++ b/README.md @@ -99,8 +99,8 @@ func main() { ### Main commands -| Command | Description | -| ------------ | ----------------------------------- | -| `make lint` | Runs lint on the codebase | -| `make tests` | Runs tests suites from all packages | -| `make check` | Runs both lint and tests | +| Command | Description | +| --------------- | ------------------------------------------------- | +| `./script/lint` | Runs lint on the codebase | +| `./script/test` | Runs tests suites from all packages | +| `./script/doc` | Serves Go doc for this module at `localhost:9995` | diff --git a/script/coverage b/script/coverage new file mode 100755 index 0000000..677aed3 --- /dev/null +++ b/script/coverage @@ -0,0 +1,3 @@ +#!/bin/bash + +go-acc ./... -- -race -timeout=10s diff --git a/script/doc b/script/doc new file mode 100755 index 0000000..715e407 --- /dev/null +++ b/script/doc @@ -0,0 +1,11 @@ +#!/bin/bash + +domain="localhost" +port="9995" +addr="${domain}:${port}" + +main() { + echo "http://${addr}/pkg/github.com/benchttp/sdk/" + godoc -http="${addr}" +} +main diff --git a/script/lint b/script/lint new file mode 100755 index 0000000..3d13e8d --- /dev/null +++ b/script/lint @@ -0,0 +1,3 @@ +#!/bin/bash + +golangci-lint run diff --git a/script/test b/script/test new file mode 100755 index 0000000..5ae7c1c --- /dev/null +++ b/script/test @@ -0,0 +1,8 @@ +#!/bin/bash + +timeout="10s" + +main() { + go test -race -timeout "${timeout}" "${@:1}" ./... +} +main "${@}" From c4fca542628ab97d0af19cc75d7853cec55213cf Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 22:01:03 +0200 Subject: [PATCH 19/25] chore(benchttp): remove Runner.Progress method Irrelevant method, use Runner.OnProgress instead. --- benchttp/runner.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/benchttp/runner.go b/benchttp/runner.go index ef28f32..0af562b 100644 --- a/benchttp/runner.go +++ b/benchttp/runner.go @@ -78,16 +78,6 @@ func (r *Runner) Run(ctx context.Context) (*Report, error) { return newReport(*r, duration, agg, testResults), nil } -// Progress returns the current progress of the recording. -// r.Run must have been called before, otherwise it returns -// a zero RecorderProgress. -func (r *Runner) Progress() RecordingProgress { - if r.recorder == nil { - return RecordingProgress{} - } - return r.recorder.Progress() -} - // recorderConfig returns a runner.RequesterConfig generated from cfg. func (r *Runner) recorderConfig() recorder.Config { return recorder.Config{ From 6b7b9feb5b179010bdc228d70537ff57e8251607 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 22:34:25 +0200 Subject: [PATCH 20/25] refactor(benchttp): DefaultRunner - simplify with less declarations - move into fie runner.go - more accurate docs --- benchttp/default.go | 30 ------------------------------ benchttp/runner.go | 14 +++++++++++++- benchttp/runner_test.go | 12 ++++++------ configparse/json_test.go | 39 ++++++++++++++------------------------- 4 files changed, 33 insertions(+), 62 deletions(-) delete mode 100644 benchttp/default.go diff --git a/benchttp/default.go b/benchttp/default.go deleted file mode 100644 index 545cf15..0000000 --- a/benchttp/default.go +++ /dev/null @@ -1,30 +0,0 @@ -package benchttp - -import ( - "fmt" - "net/http" - "time" -) - -// DefaultRunner returns a default Runner that is safe to use. -func DefaultRunner() Runner { - return defaultRunner -} - -var defaultRunner = Runner{ - Request: defaultRequest(), - - Concurrency: 10, - Requests: 100, - Interval: 0 * time.Second, - RequestTimeout: 5 * time.Second, - GlobalTimeout: 30 * time.Second, -} - -func defaultRequest() *http.Request { - req, err := http.NewRequest("GET", "", nil) - if err != nil { - panic(fmt.Sprintf("benchttp: %s", err)) - } - return req -} diff --git a/benchttp/runner.go b/benchttp/runner.go index 0af562b..f5f09d1 100644 --- a/benchttp/runner.go +++ b/benchttp/runner.go @@ -52,6 +52,18 @@ type Runner struct { recorder *recorder.Recorder } +// DefaultRunner returns a default Runner that is ready to use, +// except for Runner.Request that still needs to be set. +func DefaultRunner() Runner { + return Runner{ + Concurrency: 10, + Requests: 100, + Interval: 0 * time.Second, + RequestTimeout: 5 * time.Second, + GlobalTimeout: 30 * time.Second, + } +} + func (r *Runner) Run(ctx context.Context) (*Report, error) { // Validate input config if err := r.Validate(); err != nil { @@ -99,7 +111,7 @@ func (r Runner) Validate() error { //nolint:gocognit } if r.Request == nil { - appendError(errors.New("unexpected nil request")) + appendError(errors.New("Runner.Request must not be nil")) } if r.Requests < 1 && r.Requests != -1 { diff --git a/benchttp/runner_test.go b/benchttp/runner_test.go index 09bbc1c..9600b57 100644 --- a/benchttp/runner_test.go +++ b/benchttp/runner_test.go @@ -45,12 +45,12 @@ func TestRunner_Validate(t *testing.T) { } errs := errInvalid.Errors - assertError(t, errs, `unexpected nil request`) - assertError(t, errs, `requests (-5): want >= 0`) - assertError(t, errs, `concurrency (-5): want > 0 and <= requests (-5)`) - assertError(t, errs, `interval (-5): want >= 0`) - assertError(t, errs, `requestTimeout (-5): want > 0`) - assertError(t, errs, `globalTimeout (-5): want > 0`) + assertError(t, errs, "Runner.Request must not be nil") + assertError(t, errs, "requests (-5): want >= 0") + assertError(t, errs, "concurrency (-5): want > 0 and <= requests (-5)") + assertError(t, errs, "interval (-5): want >= 0") + assertError(t, errs, "requestTimeout (-5): want > 0") + assertError(t, errs, "globalTimeout (-5): want > 0") t.Logf("got error:\n%v", errInvalid) }) diff --git a/configparse/json_test.go b/configparse/json_test.go index 0809be9..4707c98 100644 --- a/configparse/json_test.go +++ b/configparse/json_test.go @@ -10,16 +10,17 @@ import ( ) func TestJSON(t *testing.T) { + const testURL = "https://example.com" baseInput := object{ "request": object{ - "url": "https://example.com", + "url": testURL, }, } testcases := []struct { name string input []byte - isValidRunner func(benchttp.Runner) bool + isValidRunner func(base, got benchttp.Runner) bool expError error }{ { @@ -27,7 +28,7 @@ func TestJSON(t *testing.T) { input: baseInput.assign(object{ "badkey": "marcel-patulacci", }).json(), - isValidRunner: func(cfg benchttp.Runner) bool { return true }, + isValidRunner: func(_, _ benchttp.Runner) bool { return true }, expError: errors.New(`invalid field ("badkey"): does not exist`), }, { @@ -37,22 +38,16 @@ func TestJSON(t *testing.T) { "concurrency": "bad value", // want int }, }).json(), - isValidRunner: func(benchttp.Runner) bool { return true }, + isValidRunner: func(_, _ benchttp.Runner) bool { return true }, expError: errors.New(`wrong type for field runner.concurrency: want int, got string`), }, { - name: "unmarshals JSON config and merges it with default", - input: baseInput.assign(object{ - "runner": object{"concurrency": 3}, - }).json(), - isValidRunner: func(r benchttp.Runner) bool { - defaultRunner := benchttp.DefaultRunner() - - isInputValueParsed := r.Concurrency == 3 - isMergedWithDefault := r.Request.Method == defaultRunner.Request.Method && - r.GlobalTimeout == defaultRunner.GlobalTimeout - - return isInputValueParsed && isMergedWithDefault + name: "unmarshals JSON config and merges it with base runner", + input: baseInput.json(), + isValidRunner: func(base, got benchttp.Runner) bool { + isParsed := got.Request.URL.String() == testURL + isMerged := got.GlobalTimeout == base.GlobalTimeout + return isParsed && isMerged }, expError: nil, }, @@ -63,8 +58,8 @@ func TestJSON(t *testing.T) { gotRunner := benchttp.DefaultRunner() gotError := configparse.JSON(tc.input, &gotRunner) - if !tc.isValidRunner(gotRunner) { - t.Errorf("unexpected config:\n%+v", gotRunner) + if !tc.isValidRunner(benchttp.DefaultRunner(), gotRunner) { + t.Errorf("unexpected runner:\n%+v", gotRunner) } if !sameErrors(gotError, tc.expError) { t.Errorf("unexpected error:\nexp %v,\ngot %v", tc.expError, gotError) @@ -95,11 +90,5 @@ func (o object) assign(other object) object { } func sameErrors(a, b error) bool { - if a == nil && b == nil { - return true - } - if a == nil || b == nil { - return false - } - return a.Error() == b.Error() + return (a == nil && b == nil) || !(a == nil || b == nil) || a.Error() == b.Error() } From b427482a1a4e093a72f2ca62054c7e76347cd0fc Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 23:00:44 +0200 Subject: [PATCH 21/25] feat(benchttp): implement request setters for Runner Allows convenient chaining, e.g. when using de default runner: DefaultRunner().WithNewRequest("", "http://a.b", nil).Run(ctx) - Runner.WithRequest - Runner.WithNewRequest --- benchttp/runner.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/benchttp/runner.go b/benchttp/runner.go index f5f09d1..2bb14c2 100644 --- a/benchttp/runner.go +++ b/benchttp/runner.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "net/http" "time" @@ -64,6 +65,23 @@ func DefaultRunner() Runner { } } +// WithRequest attaches the given HTTP request to the Runner. +func (r *Runner) WithRequest(req *http.Request) *Runner { + r.Request = req + return r +} + +// WithNewRequest calls http.NewRequest with the given parameters +// and attaches the result to the Runner. If the call to http.NewRequest +// returns a non-nil error, it panics with the content of that error. +func (r *Runner) WithNewRequest(method, uri string, body io.Reader) *Runner { + req, err := http.NewRequest(method, uri, body) + if err != nil { + panic(err) + } + return r.WithRequest(req) +} + func (r *Runner) Run(ctx context.Context) (*Report, error) { // Validate input config if err := r.Validate(); err != nil { From 918ac767953abb5eef9e00daa2fa74a3c7134a1a Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 23 Oct 2022 23:00:56 +0200 Subject: [PATCH 22/25] docs: update README --- README.md | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index dac188b..f4a969b 100644 --- a/README.md +++ b/README.md @@ -45,16 +45,11 @@ import ( "github.com/benchttp/sdk/benchttp" ) -func main(t *testing.T) { - // Set the request to send - request, _ := http.NewRequest("GET", "http://localhost:3000", nil) - - // Configure the runner - runner := runner.DefaultRunner() - runner.Request = request - - // Run benchmark, get report - report, _ := runner.Run(context.Background()) +func main() { + report, _ := benchttp. + DefaultRunner(). // Default runner with safe configuration + WithNewRequest("GET", "http://localhost:3000", nil). // Attach request + Run(context.Background()) // Run benchmark, retrieve report fmt.Println(report.Metrics.ResponseTimes.Mean) } @@ -69,6 +64,7 @@ import ( "context" "fmt" + "github.com/benchttp/sdk/benchttp" "github.com/benchttp/sdk/configparse" ) @@ -77,11 +73,17 @@ func main() { jsonConfig := []byte(` { "request": { - "url": "http://localhost:9999" + "url": "http://localhost:3000" } }`) - runner, _ := configparse.JSON(jsonConfig) + // Instantiate a base Runner (here the default with a safe configuration) + runner := benchttp.DefaultRunner() + + // Parse the json configuration into the Runner + _ = configparse.JSON(jsonConfig, &runner) + + // Run benchmark, retrieve report report, _ := runner.Run(context.Background()) fmt.Println(report.Metrics.ResponseTimes.Mean) From 673862faad37f74e0bbbff121950273a2797fe29 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Fri, 28 Oct 2022 16:42:14 +0200 Subject: [PATCH 23/25] faet(runner): use value rexeivers --- benchttp/runner.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/benchttp/runner.go b/benchttp/runner.go index 2bb14c2..3141b6f 100644 --- a/benchttp/runner.go +++ b/benchttp/runner.go @@ -66,7 +66,7 @@ func DefaultRunner() Runner { } // WithRequest attaches the given HTTP request to the Runner. -func (r *Runner) WithRequest(req *http.Request) *Runner { +func (r Runner) WithRequest(req *http.Request) Runner { r.Request = req return r } @@ -74,7 +74,7 @@ func (r *Runner) WithRequest(req *http.Request) *Runner { // WithNewRequest calls http.NewRequest with the given parameters // and attaches the result to the Runner. If the call to http.NewRequest // returns a non-nil error, it panics with the content of that error. -func (r *Runner) WithNewRequest(method, uri string, body io.Reader) *Runner { +func (r Runner) WithNewRequest(method, uri string, body io.Reader) Runner { req, err := http.NewRequest(method, uri, body) if err != nil { panic(err) @@ -82,7 +82,7 @@ func (r *Runner) WithNewRequest(method, uri string, body io.Reader) *Runner { return r.WithRequest(req) } -func (r *Runner) Run(ctx context.Context) (*Report, error) { +func (r Runner) Run(ctx context.Context) (*Report, error) { // Validate input config if err := r.Validate(); err != nil { return nil, err @@ -105,11 +105,11 @@ func (r *Runner) Run(ctx context.Context) (*Report, error) { testResults := tests.Run(agg, r.Tests) - return newReport(*r, duration, agg, testResults), nil + return newReport(r, duration, agg, testResults), nil } // recorderConfig returns a runner.RequesterConfig generated from cfg. -func (r *Runner) recorderConfig() recorder.Config { +func (r Runner) recorderConfig() recorder.Config { return recorder.Config{ Requests: r.Requests, Concurrency: r.Concurrency, From c0449adc91a7e33af6046b0e7a93db02dcc8be58 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Fri, 28 Oct 2022 16:42:28 +0200 Subject: [PATCH 24/25] test(runner) write unit tests for setters --- benchttp/runner_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/benchttp/runner_test.go b/benchttp/runner_test.go index 9600b57..db60da1 100644 --- a/benchttp/runner_test.go +++ b/benchttp/runner_test.go @@ -8,6 +8,37 @@ import ( "github.com/benchttp/sdk/benchttp" ) +func TestRunner_WithRequest(t *testing.T) { + request := httptest.NewRequest("POST", "https://example.com", nil) + runner := benchttp.DefaultRunner().WithRequest(request) + + if runner.Request != request { + t.Error("request not set") + } +} + +func TestRunner_WithNewRequest(t *testing.T) { + t.Run("attach valid request", func(t *testing.T) { + const method = "POST" + const urlString = "https://example.com" + + runner := benchttp.DefaultRunner().WithNewRequest(method, urlString, nil) + + if runner.Request.Method != method || runner.Request.URL.String() != urlString { + t.Error("request incorrectly seet") + } + }) + + t.Run("panic for bad request params", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("did not panic") + } + }() + _ = benchttp.DefaultRunner().WithNewRequest("ù", "", nil) + }) +} + func TestRunner_Validate(t *testing.T) { t.Run("return nil if config is valid", func(t *testing.T) { runner := benchttp.Runner{ From 3492ed902fb7afac106c5e3af8a4ced1f32f3efe Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 19 Feb 2023 22:52:17 +0100 Subject: [PATCH 25/25] chore: remove remaining Config mentions --- benchttp/error.go | 4 ++-- benchttp/report.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/benchttp/error.go b/benchttp/error.go index 19ceeac..8fe04b4 100644 --- a/benchttp/error.go +++ b/benchttp/error.go @@ -2,13 +2,13 @@ package benchttp import "strings" -// InvalidRunnerError is the errors returned by Config.Validate +// InvalidRunnerError is the errors returned by Runner.Validate // when values are missing or invalid. type InvalidRunnerError struct { Errors []error } -// Error returns the joined errors of InvalidConfigError as a string. +// Error returns the joined errors of InvalidRunnerError as a string. func (e *InvalidRunnerError) Error() string { const sep = "\n - " diff --git a/benchttp/report.go b/benchttp/report.go index da11c31..0957840 100644 --- a/benchttp/report.go +++ b/benchttp/report.go @@ -16,7 +16,7 @@ type Report struct { // Metadata contains contextual information about a run. type Metadata struct { - Config Runner + Runner Runner FinishedAt time.Time TotalDuration time.Duration } @@ -32,7 +32,7 @@ func newReport( Metrics: m, Tests: t, Metadata: Metadata{ - Config: r, + Runner: r, FinishedAt: time.Now(), // TODO: change, unreliable TotalDuration: d, },