From 76387d2b6441595bee816cd15e456b67f890f6e9 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 27 Jul 2022 15:34:10 +0200 Subject: [PATCH 01/26] feat: implement test runner - create package runner/internal/tests --- runner/internal/tests/predicate.go | 66 +++++++++++++++++ runner/internal/tests/predicate_test.go | 96 +++++++++++++++++++++++++ runner/internal/tests/tests.go | 47 ++++++++++++ runner/internal/tests/tests_test.go | 48 +++++++++++++ 4 files changed, 257 insertions(+) create mode 100644 runner/internal/tests/predicate.go create mode 100644 runner/internal/tests/predicate_test.go create mode 100644 runner/internal/tests/tests.go create mode 100644 runner/internal/tests/tests_test.go diff --git a/runner/internal/tests/predicate.go b/runner/internal/tests/predicate.go new file mode 100644 index 0000000..0e18da1 --- /dev/null +++ b/runner/internal/tests/predicate.go @@ -0,0 +1,66 @@ +package tests + +import "fmt" + +type Predicate string + +const ( + EQ Predicate = "EQ" + NEQ Predicate = "NEQ" + GT Predicate = "GT" + GTE Predicate = "GTE" + LT Predicate = "LT" + LTE Predicate = "LTE" +) + +func (p Predicate) Apply(left, right Value) SingleResult { + pass := p.passFunc()(left, right) + return SingleResult{ + Pass: pass, + Explain: p.explain(left, right, pass), + } +} + +func (p Predicate) passFunc() func(left, right Value) bool { + return func(left, right Value) bool { + switch p { + case EQ: + return left == right + case NEQ: + return left != right + case GT: + return left > right + case GTE: + return left >= right + case LT: + return left < right + case LTE: + return left <= right + default: + panic(fmt.Sprintf("%s: unknown predicate", p)) + } + } +} + +func (p Predicate) explain(metric, compared Value, pass bool) string { + return fmt.Sprintf("want %s %d, got %d", p.Symbol(), compared, metric) +} + +func (p Predicate) Symbol() string { + switch p { + case EQ: + return "==" + case NEQ: + return "!=" + case GT: + return ">" + case GTE: + return ">=" + case LT: + return "<" + case LTE: + return "<=" + default: + return "[unknown predicate]" + } +} diff --git a/runner/internal/tests/predicate_test.go b/runner/internal/tests/predicate_test.go new file mode 100644 index 0000000..6d804f8 --- /dev/null +++ b/runner/internal/tests/predicate_test.go @@ -0,0 +1,96 @@ +package tests_test + +import ( + "testing" + + "github.com/benchttp/engine/runner/internal/tests" +) + +func TestPredicate(t *testing.T) { + const ( + metric = 100 + metricInc = metric + 1 + metricDec = metric - 1 + ) + + testcases := []struct { + Predicate tests.Predicate + PassValues []tests.Value + FailValues []tests.Value + }{ + { + Predicate: tests.EQ, + PassValues: []tests.Value{metric}, + FailValues: []tests.Value{metricDec, metricInc}, + }, + { + Predicate: tests.NEQ, + PassValues: []tests.Value{metricInc, metricDec}, + FailValues: []tests.Value{metric}, + }, + { + Predicate: tests.LT, + PassValues: []tests.Value{metricDec}, + FailValues: []tests.Value{metric, metricInc}, + }, + { + Predicate: tests.LTE, + PassValues: []tests.Value{metricDec, metric}, + FailValues: []tests.Value{metricInc}, + }, + { + Predicate: tests.GT, + PassValues: []tests.Value{metricInc}, + FailValues: []tests.Value{metric, metricDec}, + }, + { + Predicate: tests.GTE, + PassValues: []tests.Value{metricInc, metric}, + FailValues: []tests.Value{metricDec}, + }, + } + + for _, tc := range testcases { + t.Run(string(tc.Predicate)+":pass", func(t *testing.T) { + for _, passValue := range tc.PassValues { + expectPredicatePass(t, tc.Predicate, metric, passValue) + } + }) + t.Run(string(tc.Predicate+":fail"), func(t *testing.T) { + for _, failValue := range tc.FailValues { + expectPredicateFail(t, tc.Predicate, metric, failValue) + } + }) + } +} + +func expectPredicatePass( + t *testing.T, + p tests.Predicate, + l, r tests.Value, +) { + t.Helper() + expectPredicateResult(t, p, l, r, true) +} + +func expectPredicateFail( + t *testing.T, + p tests.Predicate, + l, r tests.Value, +) { + t.Helper() + expectPredicateResult(t, p, l, r, false) +} + +func expectPredicateResult( + t *testing.T, + p tests.Predicate, + l, r tests.Value, + expPass bool, +) { + t.Helper() + + if pass := p.Apply(r, l).Pass; pass != expPass { + t.Errorf("exp %v %d %d -> %v, got %v", p, l, r, expPass, pass) + } +} diff --git a/runner/internal/tests/tests.go b/runner/internal/tests/tests.go new file mode 100644 index 0000000..a521725 --- /dev/null +++ b/runner/internal/tests/tests.go @@ -0,0 +1,47 @@ +package tests + +import ( + "github.com/benchttp/engine/runner/internal/metrics" +) + +type Value = int + +type Input struct { + Name string + Metric func(metrics.Aggregate) Value + Predicate Predicate + Value Value +} + +type SuiteResult struct { + Pass bool + Results []SingleResult +} + +type SingleResult struct { + Pass bool + Explain string +} + +func Run(agg metrics.Aggregate, inputs []Input) SuiteResult { + allpass := true + results := make([]SingleResult, len(inputs)) + for i, input := range inputs { + currentResult := runSingle(agg, input) + results[i] = currentResult + if !currentResult.Pass { + allpass = false + } + } + return SuiteResult{ + Pass: allpass, + Results: results, + } +} + +func runSingle(agg metrics.Aggregate, input Input) SingleResult { + gotMetric := input.Metric(agg) + comparedValue := input.Value + + return input.Predicate.Apply(gotMetric, comparedValue) +} diff --git a/runner/internal/tests/tests_test.go b/runner/internal/tests/tests_test.go new file mode 100644 index 0000000..db10f0f --- /dev/null +++ b/runner/internal/tests/tests_test.go @@ -0,0 +1,48 @@ +package tests_test + +import ( + "fmt" + "testing" + "time" + + "github.com/benchttp/engine/runner/internal/metrics" + "github.com/benchttp/engine/runner/internal/tests" +) + +func TestRun(t *testing.T) { + t.Run("happy path", func(t *testing.T) { + agg := metricsStub() + queries := []tests.Input{ + { + Name: "metrics.Min", + Metric: func(agg metrics.Aggregate) tests.Value { + return tests.Value(agg.Min) + }, + Predicate: tests.GT, + Value: tests.Value(50 * time.Millisecond), // expect pass + }, + { + Name: "metrics.Max", + Metric: func(agg metrics.Aggregate) tests.Value { + return tests.Value(agg.Max) + }, + Predicate: tests.LT, + Value: tests.Value(110 * time.Millisecond), // expect fail + }, + } + result := tests.Run(agg, queries) + fmt.Printf("%+v\n", result) + }) +} + +func metricsStub() metrics.Aggregate { + return metrics.Aggregate{ + Min: 80 * time.Millisecond, + Max: 120 * time.Millisecond, + Avg: 100 * time.Millisecond, + + TotalCount: 10, + FailureCount: 1, + SuccessCount: 9, + } +} From cfba02ef9a1b8bffcbcea97388c0b867d2c17f2d Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 27 Jul 2022 19:19:16 +0200 Subject: [PATCH 02/26] feat: parse input test configuration --- internal/configparse/parse.go | 25 ++++++++++++++++++- .../configparse/testdata/valid/benchttp.yml | 14 +++++++++++ runner/internal/config/config.go | 12 +++++++++ runner/internal/config/field.go | 2 ++ runner/internal/tests/tests.go | 15 ++++++++++- runner/internal/tests/tests_test.go | 2 +- runner/runner.go | 7 ++++++ 7 files changed, 74 insertions(+), 3 deletions(-) diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index 9bcb084..feeebba 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -41,6 +41,13 @@ type UnmarshaledConfig struct { Silent *bool `yaml:"silent" json:"silent"` Template *string `yaml:"template" json:"template"` } `yaml:"output" json:"output"` + + Tests []struct { + Name *string `yaml:"name" json:"name"` + Metric *string `yaml:"metric" json:"metric"` + Predicate *string `yaml:"predicate" json:"predicate"` + Value *string `yaml:"value" json:"value"` + } `yaml:"tests" json:"tests"` } // Parse parses a benchttp runner config file into a runner.ConfigGlobal @@ -165,7 +172,7 @@ func (pconf *parsedConfig) add(field string) { // newParsedConfig parses an input raw config as a runner.ConfigGlobal and returns // a parsedConfig or the first non-nil error occurring in the process. func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:gocognit // acceptable complexity for a parsing func - const numField = 12 // should match the number of config Fields (not critical) + const numField = 13 // should match the number of config Fields (not critical) pconf := parsedConfig{ fields: make([]string, 0, numField), @@ -250,6 +257,22 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g pconf.add(runner.ConfigFieldTemplate) } + // TODO: handle nil values (assumed non nil for now) + if tests := uconf.Tests; len(tests) > 0 { + adaptedTests := make([]runner.TestConfig, len(tests)) + for i, t := range tests { + d, _ := parseOptionalDuration(*t.Value) + adaptedTests[i] = runner.TestConfig{ + Name: *t.Name, + Metric: runner.TestMetric(*t.Metric), + Predicate: runner.TestPredicate(*t.Predicate), + Value: runner.TestValue(d), + } + } + cfg.Tests = adaptedTests + pconf.add(runner.ConfigFieldTests) + } + return pconf, nil } diff --git a/internal/configparse/testdata/valid/benchttp.yml b/internal/configparse/testdata/valid/benchttp.yml index bef73f7..15a045e 100644 --- a/internal/configparse/testdata/valid/benchttp.yml +++ b/internal/configparse/testdata/valid/benchttp.yml @@ -20,3 +20,17 @@ runner: output: silent: true template: "{{ .Metrics.Avg }}" + +tests: + - name: minimum response time + metric: MIN + predicate: GT + value: 80ms + - name: maximum response time + metric: MAX + predicate: LTE + value: 120ms + # - name: 100% availability + # metric: FAILURE_COUNT + # predicate: EQ + # value: 0 diff --git a/runner/internal/config/config.go b/runner/internal/config/config.go index 1b10a90..a56cfea 100644 --- a/runner/internal/config/config.go +++ b/runner/internal/config/config.go @@ -8,6 +8,8 @@ import ( "net/http" "net/url" "time" + + "github.com/benchttp/engine/runner/internal/tests" ) // RequestBody represents a request body associated with a type. @@ -82,12 +84,20 @@ type Output struct { Template string } +type Test struct { + Name string + Metric tests.Metric + Predicate tests.Predicate + Value tests.Value +} + // Global represents the global configuration of the runner. // It must be validated using Global.Validate before usage. type Global struct { Request Request Runner Runner Output Output + Tests []Test } // String returns an indented JSON representation of Config @@ -125,6 +135,8 @@ func (cfg Global) Override(c Global, fields ...string) Global { cfg.Output.Silent = c.Output.Silent case FieldTemplate: cfg.Output.Template = c.Output.Template + case FieldTests: + cfg.Tests = c.Tests } } return cfg diff --git a/runner/internal/config/field.go b/runner/internal/config/field.go index 4c33f0d..8124264 100644 --- a/runner/internal/config/field.go +++ b/runner/internal/config/field.go @@ -12,6 +12,7 @@ const ( FieldGlobalTimeout = "globalTimeout" FieldSilent = "silent" FieldTemplate = "template" + FieldTests = "tests" ) // FieldsUsage is a record of all available config fields and their usage. @@ -27,6 +28,7 @@ var FieldsUsage = map[string]string{ FieldGlobalTimeout: "Max duration of test", FieldSilent: "Silent mode (no write to stdout)", FieldTemplate: "Output template", + FieldTests: "Test suite", } func IsField(v string) bool { diff --git a/runner/internal/tests/tests.go b/runner/internal/tests/tests.go index a521725..b36b899 100644 --- a/runner/internal/tests/tests.go +++ b/runner/internal/tests/tests.go @@ -1,10 +1,23 @@ package tests import ( + "time" + "github.com/benchttp/engine/runner/internal/metrics" ) -type Value = int +type Value = time.Duration + +type Metric string + +const ( + MetricAvg Metric = "AVG" + MetricMin Metric = "MIN" + MetricMax Metric = "MAX" + MetricFailureCount Metric = "FAILURE_COUNT" + MetricSuccessCount Metric = "SUCCESS_COUNT" + MetricTotalCount Metric = "TOTAL_COUNT" +) type Input struct { Name string diff --git a/runner/internal/tests/tests_test.go b/runner/internal/tests/tests_test.go index db10f0f..6abede1 100644 --- a/runner/internal/tests/tests_test.go +++ b/runner/internal/tests/tests_test.go @@ -10,7 +10,7 @@ import ( ) func TestRun(t *testing.T) { - t.Run("happy path", func(t *testing.T) { + t.Run("happy path", func(_ *testing.T) { agg := metricsStub() queries := []tests.Input{ { diff --git a/runner/runner.go b/runner/runner.go index 31e2cb0..95646d5 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -8,6 +8,7 @@ import ( "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 ( @@ -16,11 +17,16 @@ type ( RequestBody = config.RequestBody RecorderConfig = config.Runner OutputConfig = config.Output + TestConfig = config.Test RecordingProgress = recorder.Progress RecordingStatus = recorder.Status Report = report.Report + + TestMetric = tests.Metric + TestPredicate = tests.Predicate + TestValue = tests.Value ) const ( @@ -40,6 +46,7 @@ const ( ConfigFieldGlobalTimeout = config.FieldGlobalTimeout ConfigFieldSilent = config.FieldSilent ConfigFieldTemplate = config.FieldTemplate + ConfigFieldTests = config.FieldTests ) var ( From 2dbe0ad004da4038f3a8272a2a9bcb22de893890 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 27 Jul 2022 19:41:26 +0200 Subject: [PATCH 03/26] feat: run test suite from config --- runner/runner.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/runner/runner.go b/runner/runner.go index 95646d5..3a67ac4 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -2,6 +2,7 @@ package runner import ( "context" + "fmt" "time" "github.com/benchttp/engine/runner/internal/config" @@ -92,6 +93,10 @@ func (r *Runner) Run(ctx context.Context, cfg config.Global) (*Report, error) { duration := time.Since(startTime) + testResults := tests.Run(agg, testConfig(cfg.Tests)) + + fmt.Println(testResults) + return report.New(agg, cfg, duration), nil } @@ -119,3 +124,35 @@ func recorderConfig( OnProgress: onRecordingProgress, } } + +func testConfig(in []config.Test) []tests.Input { + suite := make([]tests.Input, len(in)) + for i, t := range in { + suite[i] = tests.Input{ + Name: t.Name, + Metric: metricGetter(t.Metric), + Predicate: t.Predicate, + Value: t.Value, + } + } + return suite +} + +func metricGetter(m tests.Metric) func(agg metrics.Aggregate) tests.Value { + switch m { + case tests.MetricMin: + return func(agg metrics.Aggregate) tests.Value { + return tests.Value(agg.Min) + } + case tests.MetricMax: + return func(agg metrics.Aggregate) tests.Value { + return tests.Value(agg.Max) + } + case tests.MetricAvg: + return func(agg metrics.Aggregate) tests.Value { + return tests.Value(agg.Avg) + } + default: + panic("unimplemented") + } +} From 1587db743753c19f9d975900b3f36c547cbb651f Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 27 Jul 2022 19:56:03 +0200 Subject: [PATCH 04/26] feat: report test results - add result to server reponse (auto) - display summary in CLI --- runner/internal/report/report.go | 63 ++++++++++++++++++++++++++- runner/internal/report/report_test.go | 7 +-- runner/internal/tests/tests.go | 2 + runner/runner.go | 5 +-- 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/runner/internal/report/report.go b/runner/internal/report/report.go index b4139ed..213d203 100644 --- a/runner/internal/report/report.go +++ b/runner/internal/report/report.go @@ -9,14 +9,17 @@ import ( "strings" "time" + "github.com/benchttp/engine/internal/cli/ansi" "github.com/benchttp/engine/runner/internal/config" "github.com/benchttp/engine/runner/internal/metrics" + "github.com/benchttp/engine/runner/internal/tests" ) // Report represents a run result as exported by the runner. type Report struct { Metrics metrics.Aggregate Metadata Metadata + Tests tests.SuiteResult errTemplateFailTriggered error } @@ -29,9 +32,15 @@ type Metadata struct { } // New returns an initialized *Report. -func New(m metrics.Aggregate, cfg config.Global, d time.Duration) *Report { +func New( + m metrics.Aggregate, + cfg config.Global, + d time.Duration, + testResults tests.SuiteResult, +) *Report { return &Report{ Metrics: m, + Tests: testResults, Metadata: Metadata{ Config: cfg, FinishedAt: time.Now(), // TODO: change, unreliable @@ -49,6 +58,7 @@ func (rep *Report) String() string { case err == nil: // template is non-empty and correctly executed, // return its result instead of default summary. + rep.writeTestsResult(&b) return s case errors.Is(err, errTemplateSyntax): // template is non-empty but has syntax errors, @@ -60,6 +70,8 @@ func (rep *Report) String() string { } rep.writeDefaultSummary(&b) + rep.writeTestsResult(&b) + return b.String() } @@ -98,6 +110,8 @@ func (rep *Report) writeDefaultSummary(w io.StringWriter) { cfg = rep.Metadata.Config ) + w.WriteString(ansi.Bold("→ Summary")) + w.WriteString("\n") w.WriteString(line("Endpoint", cfg.Request.URL)) w.WriteString(line("Requests", formatRequests(m.TotalCount, cfg.Runner.Requests))) w.WriteString(line("Errors", m.FailureCount)) @@ -106,3 +120,50 @@ func (rep *Report) writeDefaultSummary(w io.StringWriter) { w.WriteString(line("Mean response time", msString(m.Avg))) w.WriteString(line("Total duration", msString(rep.Metadata.TotalDuration))) } + +func (rep *Report) writeTestsResult(w io.StringWriter) { + sr := rep.Tests + if len(sr.Results) == 0 { + return + } + + w.WriteString("\n") + w.WriteString(ansi.Bold("→ Test suite")) + w.WriteString("\n") + + writeResultString(w, sr.Pass) + w.WriteString("\n") + + for _, tr := range sr.Results { + writeIndent(w, 1) + writeResultString(w, tr.Pass) + w.WriteString(": ") + w.WriteString(tr.Name) + + if !tr.Pass { + w.WriteString("\n") + writeIndent(w, 2) + w.WriteString(tr.Explain) + } + + w.WriteString("\n") + } +} + +func writeResultString(w io.StringWriter, pass bool) { + if pass { + w.WriteString(ansi.Green("√")) + w.WriteString(" PASS") + } else { + w.WriteString(ansi.Red("x")) + w.WriteString(" FAIL") + } +} + +func writeIndent(w io.StringWriter, count int) { + if count <= 0 { + return + } + const baseIndent = " " + w.WriteString(strings.Repeat(baseIndent, count)) +} diff --git a/runner/internal/report/report_test.go b/runner/internal/report/report_test.go index dcc8800..8032e3f 100644 --- a/runner/internal/report/report_test.go +++ b/runner/internal/report/report_test.go @@ -10,6 +10,7 @@ import ( "github.com/benchttp/engine/runner/internal/config" "github.com/benchttp/engine/runner/internal/metrics" "github.com/benchttp/engine/runner/internal/report" + "github.com/benchttp/engine/runner/internal/tests" ) func TestReport_String(t *testing.T) { @@ -18,7 +19,7 @@ func TestReport_String(t *testing.T) { t.Run("return default summary if template is empty", func(t *testing.T) { const tpl = "" - rep := report.New(newMetrics(), newConfigWithTemplate(tpl), d) + rep := report.New(newMetrics(), newConfigWithTemplate(tpl), d, tests.SuiteResult{}) checkSummary(t, rep.String()) }) @@ -26,7 +27,7 @@ func TestReport_String(t *testing.T) { const tpl = "{{ .Metrics.TotalCount }}" m := newMetrics() - rep := report.New(m, newConfigWithTemplate(tpl), d) + rep := report.New(m, newConfigWithTemplate(tpl), d, tests.SuiteResult{}) if got, exp := rep.String(), strconv.Itoa(m.TotalCount); got != exp { t.Errorf("\nunexpected output\nexp %s\ngot %s", exp, got) @@ -36,7 +37,7 @@ func TestReport_String(t *testing.T) { t.Run("fallback to default summary if template is invalid", func(t *testing.T) { const tpl = "{{ .Marcel.Patulacci }}" - rep := report.New(newMetrics(), newConfigWithTemplate(tpl), d) + rep := report.New(newMetrics(), newConfigWithTemplate(tpl), d, tests.SuiteResult{}) got := rep.String() split := strings.Split(got, "Falling back to default summary:\n") diff --git a/runner/internal/tests/tests.go b/runner/internal/tests/tests.go index b36b899..431d250 100644 --- a/runner/internal/tests/tests.go +++ b/runner/internal/tests/tests.go @@ -32,6 +32,7 @@ type SuiteResult struct { } type SingleResult struct { + Name string Pass bool Explain string } @@ -42,6 +43,7 @@ func Run(agg metrics.Aggregate, inputs []Input) SuiteResult { for i, input := range inputs { currentResult := runSingle(agg, input) results[i] = currentResult + results[i].Name = input.Name if !currentResult.Pass { allpass = false } diff --git a/runner/runner.go b/runner/runner.go index 3a67ac4..f86caae 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -2,7 +2,6 @@ package runner import ( "context" - "fmt" "time" "github.com/benchttp/engine/runner/internal/config" @@ -95,9 +94,7 @@ func (r *Runner) Run(ctx context.Context, cfg config.Global) (*Report, error) { testResults := tests.Run(agg, testConfig(cfg.Tests)) - fmt.Println(testResults) - - return report.New(agg, cfg, duration), nil + return report.New(agg, cfg, duration, testResults), nil } // Progress returns the current progress of the recording. From 2568b59085de1295a4d148f38fe284c7801d4852 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 27 Jul 2022 20:32:31 +0200 Subject: [PATCH 05/26] test(configparse): update unit tests --- internal/configparse/parse_test.go | 14 ++++++++++++++ .../configparse/testdata/valid/benchttp.json | 16 +++++++++++++++- .../configparse/testdata/valid/benchttp.yaml | 14 ++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/internal/configparse/parse_test.go b/internal/configparse/parse_test.go index 05c9728..68e3604 100644 --- a/internal/configparse/parse_test.go +++ b/internal/configparse/parse_test.go @@ -209,6 +209,20 @@ func newExpConfig() runner.Config { Silent: true, Template: "{{ .Metrics.Avg }}", }, + Tests: []runner.TestConfig{ + { + Name: "minimum response time", + Metric: "MIN", + Predicate: "GT", + Value: 80 * time.Millisecond, + }, + { + Name: "maximum response time", + Metric: "MAX", + Predicate: "LTE", + Value: 120 * time.Millisecond, + }, + }, } } diff --git a/internal/configparse/testdata/valid/benchttp.json b/internal/configparse/testdata/valid/benchttp.json index 8ce24b1..1416690 100644 --- a/internal/configparse/testdata/valid/benchttp.json +++ b/internal/configparse/testdata/valid/benchttp.json @@ -24,5 +24,19 @@ "output": { "silent": true, "template": "{{ .Metrics.Avg }}" - } + }, + "tests": [ + { + "name": "minimum response time", + "metric": "MIN", + "predicate": "GT", + "value": "80ms" + }, + { + "name": "maximum response time", + "metric": "MAX", + "predicate": "LTE", + "value": "120ms" + } + ] } diff --git a/internal/configparse/testdata/valid/benchttp.yaml b/internal/configparse/testdata/valid/benchttp.yaml index 25541c5..0578939 100644 --- a/internal/configparse/testdata/valid/benchttp.yaml +++ b/internal/configparse/testdata/valid/benchttp.yaml @@ -23,3 +23,17 @@ runner: output: silent: true template: "{{ .Metrics.Avg }}" + +tests: + - name: minimum response time + metric: MIN + predicate: GT + value: 80ms + - name: maximum response time + metric: MAX + predicate: LTE + value: 120ms + # - name: 100% availability + # metric: FAILURE_COUNT + # predicate: EQ + # value: 0 From aa639e67a20e6ccbf8c6eae80488f74a19a4e976 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Wed, 27 Jul 2022 20:35:54 +0200 Subject: [PATCH 06/26] test(report): update unit test for Report.String --- runner/internal/report/report_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/runner/internal/report/report_test.go b/runner/internal/report/report_test.go index 8032e3f..a02800c 100644 --- a/runner/internal/report/report_test.go +++ b/runner/internal/report/report_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/benchttp/engine/internal/cli/ansi" "github.com/benchttp/engine/runner/internal/config" "github.com/benchttp/engine/runner/internal/metrics" "github.com/benchttp/engine/runner/internal/report" @@ -79,7 +80,7 @@ func newConfigWithTemplate(tpl string) config.Global { func checkSummary(t *testing.T, summary string) { t.Helper() - expSummary := ` + expSummary := ansi.Bold("→ Summary") + ` Endpoint https://a.b.com Requests 3/∞ Errors 1 @@ -87,7 +88,7 @@ Min response time 4000ms Max response time 6000ms Mean response time 5000ms Total duration 15000ms -`[1:] +` if summary != expSummary { t.Errorf("\nexp summary:\n%q\ngot summary:\n%q", expSummary, summary) From d7ffbbc9285b6899ee1e3d4ed3d01a26c8439b57 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sat, 30 Jul 2022 19:30:30 +0200 Subject: [PATCH 07/26] refactor(metrics,tests): simplify testing workflow - delegate metrics comparison logics to package metrics - remove usage of metric getter - use tests.Case as config.Global.Tests input - adapt TestPredicate to new tests API - several renamings: - metrics.Metric -> metrics.Source - tests.Input -> tests.Case - tests.SingleResult -> tests.CaseResult --- internal/configparse/parse.go | 16 +++--- internal/configparse/parse_test.go | 10 ++-- runner/internal/config/config.go | 9 +--- runner/internal/metrics/aggregate.go | 43 +++++++++++++++ runner/internal/metrics/compare.go | 60 +++++++++++++++++++++ runner/internal/metrics/metrics.go | 69 ++++++++++++------------ runner/internal/report/report.go | 4 +- runner/internal/tests/predicate.go | 71 +++++++++++-------------- runner/internal/tests/predicate_test.go | 58 ++++++++++++-------- runner/internal/tests/tests.go | 56 +++++++++---------- runner/internal/tests/tests_test.go | 18 +++---- runner/runner.go | 41 ++------------ 12 files changed, 258 insertions(+), 197 deletions(-) create mode 100644 runner/internal/metrics/aggregate.go create mode 100644 runner/internal/metrics/compare.go diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index feeebba..ae84956 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -44,9 +44,9 @@ type UnmarshaledConfig struct { Tests []struct { Name *string `yaml:"name" json:"name"` - Metric *string `yaml:"metric" json:"metric"` + Source *string `yaml:"source" json:"source"` Predicate *string `yaml:"predicate" json:"predicate"` - Value *string `yaml:"value" json:"value"` + Target *string `yaml:"target" json:"target"` } `yaml:"tests" json:"tests"` } @@ -259,17 +259,17 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g // TODO: handle nil values (assumed non nil for now) if tests := uconf.Tests; len(tests) > 0 { - adaptedTests := make([]runner.TestConfig, len(tests)) + cases := make([]runner.TestCase, len(tests)) for i, t := range tests { - d, _ := parseOptionalDuration(*t.Value) - adaptedTests[i] = runner.TestConfig{ + d, _ := parseOptionalDuration(*t.Target) + cases[i] = runner.TestCase{ Name: *t.Name, - Metric: runner.TestMetric(*t.Metric), + Source: runner.MetricsSource(*t.Source), Predicate: runner.TestPredicate(*t.Predicate), - Value: runner.TestValue(d), + Target: runner.MetricsValue(d), } } - cfg.Tests = adaptedTests + cfg.Tests = cases pconf.add(runner.ConfigFieldTests) } diff --git a/internal/configparse/parse_test.go b/internal/configparse/parse_test.go index 68e3604..ffc3c79 100644 --- a/internal/configparse/parse_test.go +++ b/internal/configparse/parse_test.go @@ -209,18 +209,18 @@ func newExpConfig() runner.Config { Silent: true, Template: "{{ .Metrics.Avg }}", }, - Tests: []runner.TestConfig{ + Tests: []runner.TestCase{ { Name: "minimum response time", - Metric: "MIN", + Source: "MIN", Predicate: "GT", - Value: 80 * time.Millisecond, + Target: 80 * time.Millisecond, }, { Name: "maximum response time", - Metric: "MAX", + Source: "MAX", Predicate: "LTE", - Value: 120 * time.Millisecond, + Target: 120 * time.Millisecond, }, }, } diff --git a/runner/internal/config/config.go b/runner/internal/config/config.go index a56cfea..3cfde53 100644 --- a/runner/internal/config/config.go +++ b/runner/internal/config/config.go @@ -84,20 +84,13 @@ type Output struct { Template string } -type Test struct { - Name string - Metric tests.Metric - Predicate tests.Predicate - Value tests.Value -} - // Global represents the global configuration of the runner. // It must be validated using Global.Validate before usage. type Global struct { Request Request Runner Runner Output Output - Tests []Test + Tests []tests.Case } // String returns an indented JSON representation of Config diff --git a/runner/internal/metrics/aggregate.go b/runner/internal/metrics/aggregate.go new file mode 100644 index 0000000..97b6594 --- /dev/null +++ b/runner/internal/metrics/aggregate.go @@ -0,0 +1,43 @@ +package metrics + +import ( + "time" + + "github.com/benchttp/engine/runner/internal/recorder" +) + +// Aggregate is an aggregate of metrics resulting from +// from recorded requests. +type Aggregate struct { + Min, Max, Avg time.Duration + SuccessCount, FailureCount, TotalCount int + // Median, StdDev time.Duration + // Deciles map[int]float64 + // StatusCodeDistribution map[string]int + // RequestEventsDistribution map[recorder.Event]int +} + +// Compute computes and aggregates metrics from the given +// requests records. +func Compute(records []recorder.Record) (agg Aggregate) { + if len(records) == 0 { + return + } + + agg.TotalCount = len(records) + agg.Min, agg.Max = records[0].Time, records[0].Time + for _, rec := range records { + if rec.Error != "" { + agg.FailureCount++ + } + if rec.Time < agg.Min { + agg.Min = rec.Time + } + if rec.Time > agg.Max { + agg.Max = rec.Time + } + agg.Avg += rec.Time / time.Duration(len(records)) + } + agg.SuccessCount = agg.TotalCount - agg.FailureCount + return +} diff --git a/runner/internal/metrics/compare.go b/runner/internal/metrics/compare.go new file mode 100644 index 0000000..61b0fb1 --- /dev/null +++ b/runner/internal/metrics/compare.go @@ -0,0 +1,60 @@ +package metrics + +import ( + "fmt" + "time" +) + +type ComparisonResult int + +const ( + INF ComparisonResult = -1 + EQ ComparisonResult = 0 + SUP ComparisonResult = 1 +) + +func compareMetrics(m, n Metric) ComparisonResult { + a, b := m.Value, n.Value + if a, b, isDuration := assertDurations(a, b); isDuration { + return compareDurations(a, b) + } + if a, b, isInt := assertInts(a, b); isInt { + return compareInts(a, b) + } + panic(fmt.Sprintf( + "metrics: unhandled comparison: %v (%T) and %v (%T)", + a, a, b, b, + )) +} + +func compareInts(a, b int) ComparisonResult { + if b < a { + return INF + } + if b > a { + return SUP + } + return EQ +} + +func compareDurations(a, b time.Duration) ComparisonResult { + return compareInts(int(a), int(b)) +} + +func assertInts(a, b Value) (x, y int, ok bool) { + x, ok = a.(int) + if !ok { + return + } + y, ok = b.(int) + return +} + +func assertDurations(a, b Value) (x, y time.Duration, ok bool) { + x, ok = a.(time.Duration) + if !ok { + return + } + y, ok = b.(time.Duration) + return +} diff --git a/runner/internal/metrics/metrics.go b/runner/internal/metrics/metrics.go index 97b6594..975b15e 100644 --- a/runner/internal/metrics/metrics.go +++ b/runner/internal/metrics/metrics.go @@ -1,43 +1,44 @@ package metrics -import ( - "time" +type Value interface{} - "github.com/benchttp/engine/runner/internal/recorder" +type Source string + +const ( + ResponseTimeAvg Source = "AVG" + ResponseTimeMin Source = "MIN" + ResponseTimeMax Source = "MAX" + RequestFailCount Source = "FAILURE_COUNT" + RequestSuccessCount Source = "SUCCESS_COUNT" + RequestCount Source = "TOTAL_COUNT" ) -// Aggregate is an aggregate of metrics resulting from -// from recorded requests. -type Aggregate struct { - Min, Max, Avg time.Duration - SuccessCount, FailureCount, TotalCount int - // Median, StdDev time.Duration - // Deciles map[int]float64 - // StatusCodeDistribution map[string]int - // RequestEventsDistribution map[recorder.Event]int +func (agg Aggregate) MetricOf(src Source) Metric { + var v interface{} + switch src { + case ResponseTimeAvg: + v = agg.Avg + case ResponseTimeMin: + v = agg.Min + case ResponseTimeMax: + v = agg.Max + case RequestFailCount: + v = agg.FailureCount + case RequestSuccessCount: + v = agg.SuccessCount + case RequestCount: + v = agg.TotalCount + default: + panic("metrics.Aggregate.MetricOf: unknown Source: " + src) + } + return Metric{Source: src, Value: v} } -// Compute computes and aggregates metrics from the given -// requests records. -func Compute(records []recorder.Record) (agg Aggregate) { - if len(records) == 0 { - return - } +type Metric struct { + Source Source + Value Value +} - agg.TotalCount = len(records) - agg.Min, agg.Max = records[0].Time, records[0].Time - for _, rec := range records { - if rec.Error != "" { - agg.FailureCount++ - } - if rec.Time < agg.Min { - agg.Min = rec.Time - } - if rec.Time > agg.Max { - agg.Max = rec.Time - } - agg.Avg += rec.Time / time.Duration(len(records)) - } - agg.SuccessCount = agg.TotalCount - agg.FailureCount - return +func (m Metric) Compare(to Metric) ComparisonResult { + return compareMetrics(m, to) } diff --git a/runner/internal/report/report.go b/runner/internal/report/report.go index 213d203..aecd079 100644 --- a/runner/internal/report/report.go +++ b/runner/internal/report/report.go @@ -138,12 +138,12 @@ func (rep *Report) writeTestsResult(w io.StringWriter) { writeIndent(w, 1) writeResultString(w, tr.Pass) w.WriteString(": ") - w.WriteString(tr.Name) + w.WriteString(tr.Input.Name) if !tr.Pass { w.WriteString("\n") writeIndent(w, 2) - w.WriteString(tr.Explain) + w.WriteString(tr.Summary) } w.WriteString("\n") diff --git a/runner/internal/tests/predicate.go b/runner/internal/tests/predicate.go index 0e18da1..b662b8f 100644 --- a/runner/internal/tests/predicate.go +++ b/runner/internal/tests/predicate.go @@ -1,6 +1,8 @@ package tests -import "fmt" +import ( + "github.com/benchttp/engine/runner/internal/metrics" +) type Predicate string @@ -13,54 +15,41 @@ const ( LTE Predicate = "LTE" ) -func (p Predicate) Apply(left, right Value) SingleResult { - pass := p.passFunc()(left, right) - return SingleResult{ - Pass: pass, - Explain: p.explain(left, right, pass), - } -} +func (p Predicate) match(comparisonResult metrics.ComparisonResult) bool { + sup := comparisonResult == metrics.SUP + inf := comparisonResult == metrics.INF -func (p Predicate) passFunc() func(left, right Value) bool { - return func(left, right Value) bool { - switch p { - case EQ: - return left == right - case NEQ: - return left != right - case GT: - return left > right - case GTE: - return left >= right - case LT: - return left < right - case LTE: - return left <= right - default: - panic(fmt.Sprintf("%s: unknown predicate", p)) - } - } -} - -func (p Predicate) explain(metric, compared Value, pass bool) string { - return fmt.Sprintf("want %s %d, got %d", p.Symbol(), compared, metric) -} - -func (p Predicate) Symbol() string { switch p { case EQ: - return "==" + return !sup && !inf case NEQ: - return "!=" + return sup || inf case GT: - return ">" + return sup case GTE: - return ">=" + return !inf case LT: - return "<" + return inf case LTE: - return "<=" + return !sup default: - return "[unknown predicate]" + panic("tests: unknown predicate: " + string(p)) + } +} + +var predicateSymbols = map[Predicate]string{ + EQ: "==", + NEQ: "!=", + GT: ">", + GTE: ">=", + LT: "<", + LTE: "<=", +} + +func (p Predicate) symbol() string { + s, ok := predicateSymbols[p] + if !ok { + return "unknown predicate" } + return s } diff --git a/runner/internal/tests/predicate_test.go b/runner/internal/tests/predicate_test.go index 6d804f8..261b2aa 100644 --- a/runner/internal/tests/predicate_test.go +++ b/runner/internal/tests/predicate_test.go @@ -3,6 +3,7 @@ package tests_test import ( "testing" + "github.com/benchttp/engine/runner/internal/metrics" "github.com/benchttp/engine/runner/internal/tests" ) @@ -15,38 +16,38 @@ func TestPredicate(t *testing.T) { testcases := []struct { Predicate tests.Predicate - PassValues []tests.Value - FailValues []tests.Value + PassValues []int + FailValues []int }{ { Predicate: tests.EQ, - PassValues: []tests.Value{metric}, - FailValues: []tests.Value{metricDec, metricInc}, + PassValues: []int{metric}, + FailValues: []int{metricDec, metricInc}, }, { Predicate: tests.NEQ, - PassValues: []tests.Value{metricInc, metricDec}, - FailValues: []tests.Value{metric}, + PassValues: []int{metricInc, metricDec}, + FailValues: []int{metric}, }, { Predicate: tests.LT, - PassValues: []tests.Value{metricDec}, - FailValues: []tests.Value{metric, metricInc}, + PassValues: []int{metricDec}, + FailValues: []int{metric, metricInc}, }, { Predicate: tests.LTE, - PassValues: []tests.Value{metricDec, metric}, - FailValues: []tests.Value{metricInc}, + PassValues: []int{metricDec, metric}, + FailValues: []int{metricInc}, }, { Predicate: tests.GT, - PassValues: []tests.Value{metricInc}, - FailValues: []tests.Value{metric, metricDec}, + PassValues: []int{metricInc}, + FailValues: []int{metric, metricDec}, }, { Predicate: tests.GTE, - PassValues: []tests.Value{metricInc, metric}, - FailValues: []tests.Value{metricDec}, + PassValues: []int{metricInc, metric}, + FailValues: []int{metricDec}, }, } @@ -67,30 +68,45 @@ func TestPredicate(t *testing.T) { func expectPredicatePass( t *testing.T, p tests.Predicate, - l, r tests.Value, + src, tar int, ) { t.Helper() - expectPredicateResult(t, p, l, r, true) + expectPredicateResult(t, p, src, tar, true) } func expectPredicateFail( t *testing.T, p tests.Predicate, - l, r tests.Value, + src, tar int, ) { t.Helper() - expectPredicateResult(t, p, l, r, false) + expectPredicateResult(t, p, src, tar, false) } func expectPredicateResult( t *testing.T, p tests.Predicate, - l, r tests.Value, + src, tar int, expPass bool, ) { t.Helper() - if pass := p.Apply(r, l).Pass; pass != expPass { - t.Errorf("exp %v %d %d -> %v, got %v", p, l, r, expPass, pass) + agg := metrics.Aggregate{ + TotalCount: src, + } + + cases := []tests.Case{{ + Predicate: p, + Source: metrics.RequestCount, + Target: metrics.Value(tar), + }} + + result := tests.Run(agg, cases) + + if pass := result.Pass; pass != expPass { + t.Errorf( + "exp %v %d %d -> %v, got %v", + p, src, tar, expPass, pass, + ) } } diff --git a/runner/internal/tests/tests.go b/runner/internal/tests/tests.go index 431d250..b5b731b 100644 --- a/runner/internal/tests/tests.go +++ b/runner/internal/tests/tests.go @@ -1,49 +1,35 @@ package tests import ( - "time" + "fmt" "github.com/benchttp/engine/runner/internal/metrics" ) -type Value = time.Duration - -type Metric string - -const ( - MetricAvg Metric = "AVG" - MetricMin Metric = "MIN" - MetricMax Metric = "MAX" - MetricFailureCount Metric = "FAILURE_COUNT" - MetricSuccessCount Metric = "SUCCESS_COUNT" - MetricTotalCount Metric = "TOTAL_COUNT" -) - -type Input struct { +type Case struct { Name string - Metric func(metrics.Aggregate) Value + Source metrics.Source Predicate Predicate - Value Value + Target metrics.Value } type SuiteResult struct { Pass bool - Results []SingleResult + Results []CaseResult } -type SingleResult struct { - Name string +type CaseResult struct { + Input Case Pass bool - Explain string + Summary string } -func Run(agg metrics.Aggregate, inputs []Input) SuiteResult { +func Run(agg metrics.Aggregate, cases []Case) SuiteResult { allpass := true - results := make([]SingleResult, len(inputs)) - for i, input := range inputs { - currentResult := runSingle(agg, input) + results := make([]CaseResult, len(cases)) + for i, input := range cases { + currentResult := runTestCase(agg, input) results[i] = currentResult - results[i].Name = input.Name if !currentResult.Pass { allpass = false } @@ -54,9 +40,17 @@ func Run(agg metrics.Aggregate, inputs []Input) SuiteResult { } } -func runSingle(agg metrics.Aggregate, input Input) SingleResult { - gotMetric := input.Metric(agg) - comparedValue := input.Value - - return input.Predicate.Apply(gotMetric, comparedValue) +func runTestCase(agg metrics.Aggregate, c Case) CaseResult { + gotMetric := agg.MetricOf(c.Source) + tarMetric := metrics.Metric{Source: c.Source, Value: c.Target} + comparisonResult := gotMetric.Compare(tarMetric) + + return CaseResult{ + Input: c, + Pass: c.Predicate.match(comparisonResult), + Summary: fmt.Sprintf( + "want %s %s %v, got %d", + c.Source, c.Predicate.symbol(), c.Target, gotMetric.Value, + ), + } } diff --git a/runner/internal/tests/tests_test.go b/runner/internal/tests/tests_test.go index 6abede1..1ab5806 100644 --- a/runner/internal/tests/tests_test.go +++ b/runner/internal/tests/tests_test.go @@ -12,22 +12,18 @@ import ( func TestRun(t *testing.T) { t.Run("happy path", func(_ *testing.T) { agg := metricsStub() - queries := []tests.Input{ + queries := []tests.Case{ { - Name: "metrics.Min", - Metric: func(agg metrics.Aggregate) tests.Value { - return tests.Value(agg.Min) - }, + Name: "minimum response time is 50ms", // succeeding case + Source: metrics.ResponseTimeMax, Predicate: tests.GT, - Value: tests.Value(50 * time.Millisecond), // expect pass + Target: metrics.Value(50 * time.Millisecond), }, { - Name: "metrics.Max", - Metric: func(agg metrics.Aggregate) tests.Value { - return tests.Value(agg.Max) - }, + Name: "maximum response time is 110ms", // failing case + Source: metrics.ResponseTimeMax, Predicate: tests.LT, - Value: tests.Value(110 * time.Millisecond), // expect fail + Target: metrics.Value(110 * time.Millisecond), }, } result := tests.Run(agg, queries) diff --git a/runner/runner.go b/runner/runner.go index f86caae..f1fb2e8 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -17,16 +17,17 @@ type ( RequestBody = config.RequestBody RecorderConfig = config.Runner OutputConfig = config.Output - TestConfig = config.Test RecordingProgress = recorder.Progress RecordingStatus = recorder.Status Report = report.Report - TestMetric = tests.Metric + MetricsSource = metrics.Source + MetricsValue = metrics.Value + + TestCase = tests.Case TestPredicate = tests.Predicate - TestValue = tests.Value ) const ( @@ -92,7 +93,7 @@ func (r *Runner) Run(ctx context.Context, cfg config.Global) (*Report, error) { duration := time.Since(startTime) - testResults := tests.Run(agg, testConfig(cfg.Tests)) + testResults := tests.Run(agg, cfg.Tests) return report.New(agg, cfg, duration, testResults), nil } @@ -121,35 +122,3 @@ func recorderConfig( OnProgress: onRecordingProgress, } } - -func testConfig(in []config.Test) []tests.Input { - suite := make([]tests.Input, len(in)) - for i, t := range in { - suite[i] = tests.Input{ - Name: t.Name, - Metric: metricGetter(t.Metric), - Predicate: t.Predicate, - Value: t.Value, - } - } - return suite -} - -func metricGetter(m tests.Metric) func(agg metrics.Aggregate) tests.Value { - switch m { - case tests.MetricMin: - return func(agg metrics.Aggregate) tests.Value { - return tests.Value(agg.Min) - } - case tests.MetricMax: - return func(agg metrics.Aggregate) tests.Value { - return tests.Value(agg.Max) - } - case tests.MetricAvg: - return func(agg metrics.Aggregate) tests.Value { - return tests.Value(agg.Avg) - } - default: - panic("unimplemented") - } -} From cdae0dc58684baecd737363e269b0b7181108f53 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 31 Jul 2022 16:19:17 +0200 Subject: [PATCH 08/26] feat(metrics,configparse): allow any metric types for tests --- internal/configparse/parse.go | 21 ++++++++++++++-- internal/configparse/parse_test.go | 6 +++++ .../configparse/testdata/valid/benchttp.json | 14 ++++++++--- .../configparse/testdata/valid/benchttp.yaml | 16 ++++++------ .../configparse/testdata/valid/benchttp.yml | 16 ++++++------ runner/internal/metrics/metrics.go | 25 ++++++++++++++++++- runner/runner.go | 4 +++ 7 files changed, 79 insertions(+), 23 deletions(-) diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index ae84956..0ba956a 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -2,10 +2,12 @@ package configparse import ( "errors" + "fmt" "net/http" "net/url" "os" "path/filepath" + "strconv" "time" "github.com/benchttp/engine/runner" @@ -261,12 +263,16 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g if tests := uconf.Tests; len(tests) > 0 { cases := make([]runner.TestCase, len(tests)) for i, t := range tests { - d, _ := parseOptionalDuration(*t.Target) + source := runner.MetricsSource(*t.Source) + target, err := parseMetricValue(source.Type(), *t.Target) + if err != nil { + return parsedConfig{}, err + } cases[i] = runner.TestCase{ Name: *t.Name, Source: runner.MetricsSource(*t.Source), Predicate: runner.TestPredicate(*t.Predicate), - Target: runner.MetricsValue(d), + Target: target, } } cfg.Tests = cases @@ -309,3 +315,14 @@ func parseOptionalDuration(raw string) (time.Duration, error) { } return time.ParseDuration(raw) } + +func parseMetricValue(typ runner.MetricsType, in string) (runner.MetricsValue, error) { + switch typ { + case runner.MetricsTypeInt: + return strconv.Atoi(in) + case runner.MetricsTypeDuration: + return time.ParseDuration(in) + default: + return nil, fmt.Errorf("cannot parse metrics type: %v", typ) + } +} diff --git a/internal/configparse/parse_test.go b/internal/configparse/parse_test.go index ffc3c79..30ebad3 100644 --- a/internal/configparse/parse_test.go +++ b/internal/configparse/parse_test.go @@ -222,6 +222,12 @@ func newExpConfig() runner.Config { Predicate: "LTE", Target: 120 * time.Millisecond, }, + { + Name: "100% availability", + Source: "FAILURE_COUNT", + Predicate: "EQ", + Target: 0, + }, }, } } diff --git a/internal/configparse/testdata/valid/benchttp.json b/internal/configparse/testdata/valid/benchttp.json index 1416690..7506566 100644 --- a/internal/configparse/testdata/valid/benchttp.json +++ b/internal/configparse/testdata/valid/benchttp.json @@ -28,15 +28,21 @@ "tests": [ { "name": "minimum response time", - "metric": "MIN", + "source": "MIN", "predicate": "GT", - "value": "80ms" + "target": "80ms" }, { "name": "maximum response time", - "metric": "MAX", + "source": "MAX", "predicate": "LTE", - "value": "120ms" + "target": "120ms" + }, + { + "name": "100% availability", + "source": "FAILURE_COUNT", + "predicate": "EQ", + "target": "0" } ] } diff --git a/internal/configparse/testdata/valid/benchttp.yaml b/internal/configparse/testdata/valid/benchttp.yaml index 0578939..9baa624 100644 --- a/internal/configparse/testdata/valid/benchttp.yaml +++ b/internal/configparse/testdata/valid/benchttp.yaml @@ -26,14 +26,14 @@ output: tests: - name: minimum response time - metric: MIN + source: MIN predicate: GT - value: 80ms + target: 80ms - name: maximum response time - metric: MAX + source: MAX predicate: LTE - value: 120ms - # - name: 100% availability - # metric: FAILURE_COUNT - # predicate: EQ - # value: 0 + target: 120ms + - name: 100% availability + source: FAILURE_COUNT + predicate: EQ + target: 0 diff --git a/internal/configparse/testdata/valid/benchttp.yml b/internal/configparse/testdata/valid/benchttp.yml index 15a045e..1f5af6f 100644 --- a/internal/configparse/testdata/valid/benchttp.yml +++ b/internal/configparse/testdata/valid/benchttp.yml @@ -23,14 +23,14 @@ output: tests: - name: minimum response time - metric: MIN + source: MIN predicate: GT - value: 80ms + target: 80ms - name: maximum response time - metric: MAX + source: MAX predicate: LTE - value: 120ms - # - name: 100% availability - # metric: FAILURE_COUNT - # predicate: EQ - # value: 0 + target: 120ms + - name: 100% availability + source: FAILURE_COUNT + predicate: EQ + target: 0 diff --git a/runner/internal/metrics/metrics.go b/runner/internal/metrics/metrics.go index 975b15e..92fd551 100644 --- a/runner/internal/metrics/metrics.go +++ b/runner/internal/metrics/metrics.go @@ -1,5 +1,7 @@ package metrics +import "reflect" + type Value interface{} type Source string @@ -13,6 +15,23 @@ const ( RequestCount Source = "TOTAL_COUNT" ) +type Type uint8 + +const ( + TypeInt Type = Type(reflect.Int) + TypeDuration Type = 12 +) + +func (src Source) Type() Type { + switch src { + case ResponseTimeAvg, ResponseTimeMin, ResponseTimeMax: + return TypeDuration + case RequestFailCount, RequestSuccessCount, RequestCount: + return TypeInt + } + panic(badSource(src)) +} + func (agg Aggregate) MetricOf(src Source) Metric { var v interface{} switch src { @@ -29,7 +48,7 @@ func (agg Aggregate) MetricOf(src Source) Metric { case RequestCount: v = agg.TotalCount default: - panic("metrics.Aggregate.MetricOf: unknown Source: " + src) + panic(badSource(src)) } return Metric{Source: src, Value: v} } @@ -42,3 +61,7 @@ type Metric struct { func (m Metric) Compare(to Metric) ComparisonResult { return compareMetrics(m, to) } + +func badSource(src Source) string { + return "metrics: unknown Source: " + string(src) +} diff --git a/runner/runner.go b/runner/runner.go index f1fb2e8..164f0c4 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -25,6 +25,7 @@ type ( MetricsSource = metrics.Source MetricsValue = metrics.Value + MetricsType = metrics.Type TestCase = tests.Case TestPredicate = tests.Predicate @@ -55,6 +56,9 @@ var ( ConfigFieldsUsage = config.FieldsUsage NewRequestBody = config.NewRequestBody IsConfigField = config.IsField + + MetricsTypeInt = metrics.TypeInt + MetricsTypeDuration = metrics.TypeDuration ) type Runner struct { From 3391b4e3fc3fae1a1f17275bbc9353d03f79fec4 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 31 Jul 2022 16:53:15 +0200 Subject: [PATCH 09/26] fix(tests): use accurate format for value display - e.g. durations: "85675992" -> "85.675992ms" --- runner/internal/tests/tests.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runner/internal/tests/tests.go b/runner/internal/tests/tests.go index b5b731b..6c80cac 100644 --- a/runner/internal/tests/tests.go +++ b/runner/internal/tests/tests.go @@ -49,7 +49,7 @@ func runTestCase(agg metrics.Aggregate, c Case) CaseResult { Input: c, Pass: c.Predicate.match(comparisonResult), Summary: fmt.Sprintf( - "want %s %s %v, got %d", + "want %s %s %v, got %v", c.Source, c.Predicate.symbol(), c.Target, gotMetric.Value, ), } From 8538c1a9c0e0941b8b5fc40233b45f8f265ad48f Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 31 Jul 2022 20:06:05 +0200 Subject: [PATCH 10/26] fix(metrics): set TypeDuration to last reflect.Kind + 1 --- runner/internal/metrics/metrics.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/runner/internal/metrics/metrics.go b/runner/internal/metrics/metrics.go index 92fd551..1db8321 100644 --- a/runner/internal/metrics/metrics.go +++ b/runner/internal/metrics/metrics.go @@ -18,8 +18,10 @@ const ( type Type uint8 const ( - TypeInt Type = Type(reflect.Int) - TypeDuration Type = 12 + lastGoReflectKind = reflect.UnsafePointer + + TypeInt = Type(reflect.Int) + TypeDuration = Type(lastGoReflectKind + iota) ) func (src Source) Type() Type { From 6ce2447e7fc06726d6e653aa8f576e273d9b7c99 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 31 Jul 2022 22:35:29 +0200 Subject: [PATCH 11/26] test(tests): implement TestRun --- runner/internal/tests/tests_test.go | 120 ++++++++++++++++++++++------ 1 file changed, 95 insertions(+), 25 deletions(-) diff --git a/runner/internal/tests/tests_test.go b/runner/internal/tests/tests_test.go index 1ab5806..91e06cb 100644 --- a/runner/internal/tests/tests_test.go +++ b/runner/internal/tests/tests_test.go @@ -10,35 +10,105 @@ import ( ) func TestRun(t *testing.T) { - t.Run("happy path", func(_ *testing.T) { - agg := metricsStub() - queries := []tests.Case{ - { - Name: "minimum response time is 50ms", // succeeding case - Source: metrics.ResponseTimeMax, - Predicate: tests.GT, - Target: metrics.Value(50 * time.Millisecond), + testcases := []struct { + label string + inputAgg metrics.Aggregate + inputCases []tests.Case + expGlobalPass bool + expCaseResults []tests.CaseResult + }{ + { + label: "pass if all cases pass", + inputAgg: metrics.Aggregate{Avg: 100 * time.Millisecond}, + inputCases: []tests.Case{ + { + Name: "average response time below 120ms (pass)", + Predicate: tests.LT, + Source: metrics.ResponseTimeAvg, + Target: 120 * time.Millisecond, + }, + { + Name: "average response time is above 80ms (pass)", + Predicate: tests.GT, + Source: metrics.ResponseTimeAvg, + Target: 80 * time.Millisecond, + }, }, - { - Name: "maximum response time is 110ms", // failing case - Source: metrics.ResponseTimeMax, - Predicate: tests.LT, - Target: metrics.Value(110 * time.Millisecond), + expGlobalPass: true, + expCaseResults: []tests.CaseResult{ + {Pass: true, Summary: "want AVG < 120ms, got 100ms"}, + {Pass: true, Summary: "want AVG > 80ms, got 100ms"}, }, - } - result := tests.Run(agg, queries) - fmt.Printf("%+v\n", result) - }) + }, + { + label: "fail if at least one case fails", + inputAgg: metrics.Aggregate{Avg: 200 * time.Millisecond}, + inputCases: []tests.Case{ + { + Name: "average response time below 120ms (fail)", + Predicate: tests.LT, + Source: metrics.ResponseTimeAvg, + Target: 120 * time.Millisecond, + }, + { + Name: "average response time is above 80ms (pass)", + Predicate: tests.GT, + Source: metrics.ResponseTimeAvg, + Target: 80 * time.Millisecond, + }, + }, + expGlobalPass: false, + expCaseResults: []tests.CaseResult{ + {Pass: false, Summary: "want AVG < 120ms, got 200ms"}, + {Pass: true, Summary: "want AVG > 80ms, got 200ms"}, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.label, func(t *testing.T) { + suiteResult := tests.Run(tc.inputAgg, tc.inputCases) + + if gotGlobalPass := suiteResult.Pass; gotGlobalPass != tc.expGlobalPass { + t.Errorf( + "exp global pass == %v, got %v", + gotGlobalPass, tc.expGlobalPass, + ) + } + + assertEqualCaseResults(t, tc.expCaseResults, suiteResult.Results) + }) + } } -func metricsStub() metrics.Aggregate { - return metrics.Aggregate{ - Min: 80 * time.Millisecond, - Max: 120 * time.Millisecond, - Avg: 100 * time.Millisecond, +func assertEqualCaseResults(t *testing.T, exp, got []tests.CaseResult) { + t.Helper() + + if gotLen, expLen := len(got), len(exp); gotLen != expLen { + t.Errorf("exp %d case results, got %d", expLen, gotLen) + } + + for i, expResult := range exp { + gotResult := got[i] + caseDesc := fmt.Sprintf("case #%d (%q)", i, gotResult.Input.Name) + + t.Run(fmt.Sprintf("cases[%d].Pass", i), func(t *testing.T) { + if gotResult.Pass != expResult.Pass { + t.Errorf( + "\n%s:\nexp %v, got %v", + caseDesc, expResult.Pass, gotResult.Pass, + ) + } + }) + + t.Run(fmt.Sprintf("cases[%d].Summary", i), func(t *testing.T) { + if gotResult.Summary != expResult.Summary { + t.Errorf( + "\n%s:\nexp %q\ngot %q", + caseDesc, expResult.Summary, gotResult.Summary, + ) + } + }) - TotalCount: 10, - FailureCount: 1, - SuccessCount: 9, } } From 4fe0e3772c83e724a8e38086c64eac5e09b84282 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 31 Jul 2022 22:37:41 +0200 Subject: [PATCH 12/26] test(tests): fix and refactor TestPredicate - PassValues and FailValues were inverted for comparison cases --- runner/internal/tests/predicate_test.go | 43 ++++++++++++------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/runner/internal/tests/predicate_test.go b/runner/internal/tests/predicate_test.go index 261b2aa..65b43e9 100644 --- a/runner/internal/tests/predicate_test.go +++ b/runner/internal/tests/predicate_test.go @@ -9,9 +9,10 @@ import ( func TestPredicate(t *testing.T) { const ( - metric = 100 - metricInc = metric + 1 - metricDec = metric - 1 + source = 100 + same = source + more = source + 1 + less = source - 1 ) testcases := []struct { @@ -21,45 +22,45 @@ func TestPredicate(t *testing.T) { }{ { Predicate: tests.EQ, - PassValues: []int{metric}, - FailValues: []int{metricDec, metricInc}, + PassValues: []int{same}, + FailValues: []int{more, less}, }, { Predicate: tests.NEQ, - PassValues: []int{metricInc, metricDec}, - FailValues: []int{metric}, + PassValues: []int{less, more}, + FailValues: []int{same}, }, { Predicate: tests.LT, - PassValues: []int{metricDec}, - FailValues: []int{metric, metricInc}, + PassValues: []int{more}, + FailValues: []int{same, less}, }, { Predicate: tests.LTE, - PassValues: []int{metricDec, metric}, - FailValues: []int{metricInc}, + PassValues: []int{more, same}, + FailValues: []int{less}, }, { Predicate: tests.GT, - PassValues: []int{metricInc}, - FailValues: []int{metric, metricDec}, + PassValues: []int{less}, + FailValues: []int{same, more}, }, { Predicate: tests.GTE, - PassValues: []int{metricInc, metric}, - FailValues: []int{metricDec}, + PassValues: []int{less, same}, + FailValues: []int{more}, }, } for _, tc := range testcases { t.Run(string(tc.Predicate)+":pass", func(t *testing.T) { for _, passValue := range tc.PassValues { - expectPredicatePass(t, tc.Predicate, metric, passValue) + expectPredicatePass(t, tc.Predicate, source, passValue) } }) t.Run(string(tc.Predicate+":fail"), func(t *testing.T) { for _, failValue := range tc.FailValues { - expectPredicateFail(t, tc.Predicate, metric, failValue) + expectPredicateFail(t, tc.Predicate, source, failValue) } }) } @@ -95,13 +96,11 @@ func expectPredicateResult( TotalCount: src, } - cases := []tests.Case{{ + result := tests.Run(agg, []tests.Case{{ Predicate: p, Source: metrics.RequestCount, - Target: metrics.Value(tar), - }} - - result := tests.Run(agg, cases) + Target: tar, + }}) if pass := result.Pass; pass != expPass { t.Errorf( From 9c209a42fa86a1fc7df3a389aa2fa6719fa771c9 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 31 Jul 2022 22:48:25 +0200 Subject: [PATCH 13/26] fix(metrics): reverse comparison order for Metric.Compare Before: `a.Compare(b) == compare(a, b)` `0.Compare(1) == compare(0, 1) == SUP` Now: `a.Compare(b) == compare(a, b)` `0.Compare(1) == compare(1, 0) == INF` The new order makes more sense because `a` remains the main interest of the operation: when calling `a.Compare(b)` we want to get information about `a` (when compared to `b`) --- runner/internal/metrics/metrics.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runner/internal/metrics/metrics.go b/runner/internal/metrics/metrics.go index 1db8321..3ab3bf9 100644 --- a/runner/internal/metrics/metrics.go +++ b/runner/internal/metrics/metrics.go @@ -61,7 +61,7 @@ type Metric struct { } func (m Metric) Compare(to Metric) ComparisonResult { - return compareMetrics(m, to) + return compareMetrics(to, m) } func badSource(src Source) string { From 6df8217a29182bd2e3957e19555b95c8304bb19f Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 31 Jul 2022 23:31:25 +0200 Subject: [PATCH 14/26] docs(metrics): add doc comments --- runner/internal/metrics/compare.go | 19 +++++++++++++++- runner/internal/metrics/metrics.go | 35 +++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/runner/internal/metrics/compare.go b/runner/internal/metrics/compare.go index 61b0fb1..91e9417 100644 --- a/runner/internal/metrics/compare.go +++ b/runner/internal/metrics/compare.go @@ -5,14 +5,23 @@ import ( "time" ) +// ComparisonResult is the result of a comparison. type ComparisonResult int const ( + // INF is the result of an inferiority check. INF ComparisonResult = -1 - EQ ComparisonResult = 0 + // EQ is the result of an equality check. + EQ ComparisonResult = 0 + // INF is the result of superiority check. SUP ComparisonResult = 1 ) +// comapreMetrics compares the values of m and n, +// and returns the result from the point of view of n. +// +// It panics if m and n are not of the same type, +// or if their type is not handled. func compareMetrics(m, n Metric) ComparisonResult { a, b := m.Value, n.Value if a, b, isDuration := assertDurations(a, b); isDuration { @@ -27,6 +36,8 @@ func compareMetrics(m, n Metric) ComparisonResult { )) } +// compareInts compares a and b and returns a ComparisonResult +// from the point of view of b. func compareInts(a, b int) ComparisonResult { if b < a { return INF @@ -37,10 +48,14 @@ func compareInts(a, b int) ComparisonResult { return EQ } +// compareInts compares a and b and returns a ComparisonResult +// from the point of view of b. func compareDurations(a, b time.Duration) ComparisonResult { return compareInts(int(a), int(b)) } +// assertInts returns a, b as ints and true if a and b +// are both ints, else it returns 0, 0, false. func assertInts(a, b Value) (x, y int, ok bool) { x, ok = a.(int) if !ok { @@ -50,6 +65,8 @@ func assertInts(a, b Value) (x, y int, ok bool) { return } +// assertInts returns a, b as time.Durations and true if a and b +// are both time.Duration, else it returns 0, 0, false. func assertDurations(a, b Value) (x, y time.Duration, ok bool) { x, ok = a.(time.Duration) if !ok { diff --git a/runner/internal/metrics/metrics.go b/runner/internal/metrics/metrics.go index 3ab3bf9..d0fcbab 100644 --- a/runner/internal/metrics/metrics.go +++ b/runner/internal/metrics/metrics.go @@ -2,8 +2,13 @@ package metrics import "reflect" +// Value is a concrete metric value, e.g. 120 or 3 * time.Second. type Value interface{} +// Source represents the origin of a Metric. +// It exposes a method Type that returns the type of the metric. +// It can be used to reference a Metric in an Aggregate +// via Aggregate.MetricOf. type Source string const ( @@ -15,15 +20,19 @@ const ( RequestCount Source = "TOTAL_COUNT" ) +// Type represents the underlying type of a Value. type Type uint8 const ( lastGoReflectKind = reflect.UnsafePointer - TypeInt = Type(reflect.Int) + // TypeInt corresponds to type int. + TypeInt = Type(reflect.Int) + // TypeDuration corresponds to type time.Duration. TypeDuration = Type(lastGoReflectKind + iota) ) +// Type returns the underlying type of the metric src refers to. func (src Source) Type() Type { switch src { case ResponseTimeAvg, ResponseTimeMin, ResponseTimeMax: @@ -34,6 +43,7 @@ func (src Source) Type() Type { panic(badSource(src)) } +// MetricOf returns the Metric for the given Source in Aggregate. func (agg Aggregate) MetricOf(src Source) Metric { var v interface{} switch src { @@ -55,11 +65,34 @@ func (agg Aggregate) MetricOf(src Source) Metric { return Metric{Source: src, Value: v} } +// Metric represents an Aggregate metric. It links together a Value +// and its Source from the Aggregate. +// It exposes a method Compare that compares its Value to another. type Metric struct { Source Source Value Value } +// Compare compares a Metric's value to another. +// It returns a ComparisonResult that indicates the relationship +// between the two values from the receiver's point of view. +// +// It panics if m and n are not of the same type, +// or if their type is not handled. +// +// Examples: +// +// receiver := Metric{Value: 120} +// comparer := Metric{Value: 100} +// receiver.Compare(comparer) == SUP +// +// receiver := Metric{Value: 120 * time.Millisecond} +// comparer := Metric{Value: 100} +// receiver.Compare(comparer) // panics! +// +// receiver := Metric{Value: http.Header{}} +// comparer := Metric{Value: http.Header{}} +// receiver.Compare(comparer) // panics! func (m Metric) Compare(to Metric) ComparisonResult { return compareMetrics(to, m) } From cd471c632e38a066e3d6279812e4adc2d2bf9450 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Sun, 31 Jul 2022 23:55:47 +0200 Subject: [PATCH 15/26] test(metrics): implement TestMetric_Compare --- runner/internal/metrics/metrics_test.go | 87 +++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 runner/internal/metrics/metrics_test.go diff --git a/runner/internal/metrics/metrics_test.go b/runner/internal/metrics/metrics_test.go new file mode 100644 index 0000000..885d176 --- /dev/null +++ b/runner/internal/metrics/metrics_test.go @@ -0,0 +1,87 @@ +package metrics_test + +import ( + "testing" + "time" + + "github.com/benchttp/engine/runner/internal/metrics" +) + +func TestMetric_Compare(t *testing.T) { + const ( + base = 100 + more = base + 1 + less = base - 1 + ) + + testcases := []struct { + label string + baseMetric metrics.Metric + targetMetric metrics.Metric + expResult metrics.ComparisonResult + expPanic bool + }{ + { + label: "base equals target", + baseMetric: metricWithValue(base), + targetMetric: metricWithValue(base), + expResult: metrics.EQ, + expPanic: false, + }, + { + label: "base superior to target", + baseMetric: metricWithValue(base), + targetMetric: metricWithValue(less), + expResult: metrics.SUP, + expPanic: false, + }, + { + label: "base inferior to target", + baseMetric: metricWithValue(base), + targetMetric: metricWithValue(more), + expResult: metrics.INF, + expPanic: false, + }, + { + label: "panics with different type", + baseMetric: metricWithValue(base), + targetMetric: metricWithValue(base * time.Millisecond), + expResult: 0, // irrelevant, should panic + expPanic: true, + }, + { + label: "panics with different type", + baseMetric: metricWithValue(1.23), + targetMetric: metricWithValue(1.23), + expResult: 0, // irrelevant, should panic + expPanic: true, + }, + } + + for _, tc := range testcases { + t.Run(tc.label, func(t *testing.T) { + if tc.expPanic { + defer assertPanic(t) + } + + result := tc.baseMetric.Compare(tc.targetMetric) + + if !tc.expPanic && result != tc.expResult { + t.Errorf( + "\nexp %v.Compare(%v) == %v, got %v", + tc.baseMetric, tc.targetMetric, tc.expResult, result) + } + }) + } +} + +func metricWithValue(v metrics.Value) metrics.Metric { + return metrics.Metric{Value: v} +} + +func assertPanic(t *testing.T) { + t.Helper() + if r := recover(); r == nil { + t.Error("did not panic") + } +} From e6196c016592c848522f89097834b227cc8b51ad Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 01:15:40 +0200 Subject: [PATCH 16/26] refactor(metrics): rename Source -> Field --- internal/configparse/parse.go | 8 ++-- internal/configparse/parse_test.go | 6 +-- .../configparse/testdata/valid/benchttp.json | 6 +-- .../configparse/testdata/valid/benchttp.yaml | 6 +-- .../configparse/testdata/valid/benchttp.yml | 6 +-- runner/internal/metrics/metrics.go | 46 +++++++++---------- runner/internal/tests/predicate_test.go | 25 +++++----- runner/internal/tests/tests.go | 8 ++-- runner/internal/tests/tests_test.go | 8 ++-- runner/runner.go | 6 +-- 10 files changed, 62 insertions(+), 63 deletions(-) diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index 0ba956a..227690e 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -46,7 +46,7 @@ type UnmarshaledConfig struct { Tests []struct { Name *string `yaml:"name" json:"name"` - Source *string `yaml:"source" json:"source"` + Field *string `yaml:"field" json:"field"` Predicate *string `yaml:"predicate" json:"predicate"` Target *string `yaml:"target" json:"target"` } `yaml:"tests" json:"tests"` @@ -263,14 +263,14 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g if tests := uconf.Tests; len(tests) > 0 { cases := make([]runner.TestCase, len(tests)) for i, t := range tests { - source := runner.MetricsSource(*t.Source) - target, err := parseMetricValue(source.Type(), *t.Target) + field := runner.MetricsField(*t.Field) + target, err := parseMetricValue(field.Type(), *t.Target) if err != nil { return parsedConfig{}, err } cases[i] = runner.TestCase{ Name: *t.Name, - Source: runner.MetricsSource(*t.Source), + Field: runner.MetricsField(*t.Field), Predicate: runner.TestPredicate(*t.Predicate), Target: target, } diff --git a/internal/configparse/parse_test.go b/internal/configparse/parse_test.go index 30ebad3..f6cc9b3 100644 --- a/internal/configparse/parse_test.go +++ b/internal/configparse/parse_test.go @@ -212,19 +212,19 @@ func newExpConfig() runner.Config { Tests: []runner.TestCase{ { Name: "minimum response time", - Source: "MIN", + Field: "MIN", Predicate: "GT", Target: 80 * time.Millisecond, }, { Name: "maximum response time", - Source: "MAX", + Field: "MAX", Predicate: "LTE", Target: 120 * time.Millisecond, }, { Name: "100% availability", - Source: "FAILURE_COUNT", + Field: "FAILURE_COUNT", Predicate: "EQ", Target: 0, }, diff --git a/internal/configparse/testdata/valid/benchttp.json b/internal/configparse/testdata/valid/benchttp.json index 7506566..f68438e 100644 --- a/internal/configparse/testdata/valid/benchttp.json +++ b/internal/configparse/testdata/valid/benchttp.json @@ -28,19 +28,19 @@ "tests": [ { "name": "minimum response time", - "source": "MIN", + "field": "MIN", "predicate": "GT", "target": "80ms" }, { "name": "maximum response time", - "source": "MAX", + "field": "MAX", "predicate": "LTE", "target": "120ms" }, { "name": "100% availability", - "source": "FAILURE_COUNT", + "field": "FAILURE_COUNT", "predicate": "EQ", "target": "0" } diff --git a/internal/configparse/testdata/valid/benchttp.yaml b/internal/configparse/testdata/valid/benchttp.yaml index 9baa624..2871ec0 100644 --- a/internal/configparse/testdata/valid/benchttp.yaml +++ b/internal/configparse/testdata/valid/benchttp.yaml @@ -26,14 +26,14 @@ output: tests: - name: minimum response time - source: MIN + field: MIN predicate: GT target: 80ms - name: maximum response time - source: MAX + field: MAX predicate: LTE target: 120ms - name: 100% availability - source: FAILURE_COUNT + field: FAILURE_COUNT predicate: EQ target: 0 diff --git a/internal/configparse/testdata/valid/benchttp.yml b/internal/configparse/testdata/valid/benchttp.yml index 1f5af6f..4968772 100644 --- a/internal/configparse/testdata/valid/benchttp.yml +++ b/internal/configparse/testdata/valid/benchttp.yml @@ -23,14 +23,14 @@ output: tests: - name: minimum response time - source: MIN + field: MIN predicate: GT target: 80ms - name: maximum response time - source: MAX + field: MAX predicate: LTE target: 120ms - name: 100% availability - source: FAILURE_COUNT + field: FAILURE_COUNT predicate: EQ target: 0 diff --git a/runner/internal/metrics/metrics.go b/runner/internal/metrics/metrics.go index d0fcbab..d8ee97d 100644 --- a/runner/internal/metrics/metrics.go +++ b/runner/internal/metrics/metrics.go @@ -5,19 +5,19 @@ import "reflect" // Value is a concrete metric value, e.g. 120 or 3 * time.Second. type Value interface{} -// Source represents the origin of a Metric. +// Field represents the origin of a Metric. // It exposes a method Type that returns the type of the metric. // It can be used to reference a Metric in an Aggregate // via Aggregate.MetricOf. -type Source string +type Field string const ( - ResponseTimeAvg Source = "AVG" - ResponseTimeMin Source = "MIN" - ResponseTimeMax Source = "MAX" - RequestFailCount Source = "FAILURE_COUNT" - RequestSuccessCount Source = "SUCCESS_COUNT" - RequestCount Source = "TOTAL_COUNT" + ResponseTimeAvg Field = "AVG" + ResponseTimeMin Field = "MIN" + ResponseTimeMax Field = "MAX" + RequestFailCount Field = "FAILURE_COUNT" + RequestSuccessCount Field = "SUCCESS_COUNT" + RequestCount Field = "TOTAL_COUNT" ) // Type represents the underlying type of a Value. @@ -32,21 +32,21 @@ const ( TypeDuration = Type(lastGoReflectKind + iota) ) -// Type returns the underlying type of the metric src refers to. -func (src Source) Type() Type { - switch src { +// Type returns the underlying type of the metric field refers to. +func (field Field) Type() Type { + switch field { case ResponseTimeAvg, ResponseTimeMin, ResponseTimeMax: return TypeDuration case RequestFailCount, RequestSuccessCount, RequestCount: return TypeInt } - panic(badSource(src)) + panic(badField(field)) } -// MetricOf returns the Metric for the given Source in Aggregate. -func (agg Aggregate) MetricOf(src Source) Metric { +// MetricOf returns the Metric for the given Field in Aggregate. +func (agg Aggregate) MetricOf(field Field) Metric { var v interface{} - switch src { + switch field { case ResponseTimeAvg: v = agg.Avg case ResponseTimeMin: @@ -60,17 +60,17 @@ func (agg Aggregate) MetricOf(src Source) Metric { case RequestCount: v = agg.TotalCount default: - panic(badSource(src)) + panic(badField(field)) } - return Metric{Source: src, Value: v} + return Metric{Field: field, Value: v} } -// Metric represents an Aggregate metric. It links together a Value -// and its Source from the Aggregate. +// Metric represents an Aggregate metric. It links together a Field +// and its Value from the Aggregate. // It exposes a method Compare that compares its Value to another. type Metric struct { - Source Source - Value Value + Field Field + Value Value } // Compare compares a Metric's value to another. @@ -97,6 +97,6 @@ func (m Metric) Compare(to Metric) ComparisonResult { return compareMetrics(to, m) } -func badSource(src Source) string { - return "metrics: unknown Source: " + string(src) +func badField(field Field) string { + return "metrics: unknown Field: " + string(field) } diff --git a/runner/internal/tests/predicate_test.go b/runner/internal/tests/predicate_test.go index 65b43e9..cf3dae8 100644 --- a/runner/internal/tests/predicate_test.go +++ b/runner/internal/tests/predicate_test.go @@ -9,10 +9,9 @@ import ( func TestPredicate(t *testing.T) { const ( - source = 100 - same = source - more = source + 1 - less = source - 1 + base = 100 + more = base + 1 + less = base - 1 ) testcases := []struct { @@ -22,32 +21,32 @@ func TestPredicate(t *testing.T) { }{ { Predicate: tests.EQ, - PassValues: []int{same}, + PassValues: []int{base}, FailValues: []int{more, less}, }, { Predicate: tests.NEQ, PassValues: []int{less, more}, - FailValues: []int{same}, + FailValues: []int{base}, }, { Predicate: tests.LT, PassValues: []int{more}, - FailValues: []int{same, less}, + FailValues: []int{base, less}, }, { Predicate: tests.LTE, - PassValues: []int{more, same}, + PassValues: []int{more, base}, FailValues: []int{less}, }, { Predicate: tests.GT, PassValues: []int{less}, - FailValues: []int{same, more}, + FailValues: []int{base, more}, }, { Predicate: tests.GTE, - PassValues: []int{less, same}, + PassValues: []int{less, base}, FailValues: []int{more}, }, } @@ -55,12 +54,12 @@ func TestPredicate(t *testing.T) { for _, tc := range testcases { t.Run(string(tc.Predicate)+":pass", func(t *testing.T) { for _, passValue := range tc.PassValues { - expectPredicatePass(t, tc.Predicate, source, passValue) + expectPredicatePass(t, tc.Predicate, base, passValue) } }) t.Run(string(tc.Predicate+":fail"), func(t *testing.T) { for _, failValue := range tc.FailValues { - expectPredicateFail(t, tc.Predicate, source, failValue) + expectPredicateFail(t, tc.Predicate, base, failValue) } }) } @@ -98,7 +97,7 @@ func expectPredicateResult( result := tests.Run(agg, []tests.Case{{ Predicate: p, - Source: metrics.RequestCount, + Field: metrics.RequestCount, Target: tar, }}) diff --git a/runner/internal/tests/tests.go b/runner/internal/tests/tests.go index 6c80cac..431a42e 100644 --- a/runner/internal/tests/tests.go +++ b/runner/internal/tests/tests.go @@ -8,7 +8,7 @@ import ( type Case struct { Name string - Source metrics.Source + Field metrics.Field Predicate Predicate Target metrics.Value } @@ -41,8 +41,8 @@ func Run(agg metrics.Aggregate, cases []Case) SuiteResult { } func runTestCase(agg metrics.Aggregate, c Case) CaseResult { - gotMetric := agg.MetricOf(c.Source) - tarMetric := metrics.Metric{Source: c.Source, Value: c.Target} + gotMetric := agg.MetricOf(c.Field) + tarMetric := metrics.Metric{Field: c.Field, Value: c.Target} comparisonResult := gotMetric.Compare(tarMetric) return CaseResult{ @@ -50,7 +50,7 @@ func runTestCase(agg metrics.Aggregate, c Case) CaseResult { Pass: c.Predicate.match(comparisonResult), Summary: fmt.Sprintf( "want %s %s %v, got %v", - c.Source, c.Predicate.symbol(), c.Target, gotMetric.Value, + c.Field, c.Predicate.symbol(), c.Target, gotMetric.Value, ), } } diff --git a/runner/internal/tests/tests_test.go b/runner/internal/tests/tests_test.go index 91e06cb..f49a7dd 100644 --- a/runner/internal/tests/tests_test.go +++ b/runner/internal/tests/tests_test.go @@ -24,13 +24,13 @@ func TestRun(t *testing.T) { { Name: "average response time below 120ms (pass)", Predicate: tests.LT, - Source: metrics.ResponseTimeAvg, + Field: metrics.ResponseTimeAvg, Target: 120 * time.Millisecond, }, { Name: "average response time is above 80ms (pass)", Predicate: tests.GT, - Source: metrics.ResponseTimeAvg, + Field: metrics.ResponseTimeAvg, Target: 80 * time.Millisecond, }, }, @@ -47,13 +47,13 @@ func TestRun(t *testing.T) { { Name: "average response time below 120ms (fail)", Predicate: tests.LT, - Source: metrics.ResponseTimeAvg, + Field: metrics.ResponseTimeAvg, Target: 120 * time.Millisecond, }, { Name: "average response time is above 80ms (pass)", Predicate: tests.GT, - Source: metrics.ResponseTimeAvg, + Field: metrics.ResponseTimeAvg, Target: 80 * time.Millisecond, }, }, diff --git a/runner/runner.go b/runner/runner.go index 164f0c4..096e8a4 100644 --- a/runner/runner.go +++ b/runner/runner.go @@ -23,9 +23,9 @@ type ( Report = report.Report - MetricsSource = metrics.Source - MetricsValue = metrics.Value - MetricsType = metrics.Type + MetricsField = metrics.Field + MetricsValue = metrics.Value + MetricsType = metrics.Type TestCase = tests.Case TestPredicate = tests.Predicate From 05a2514ed5a25fdb2bfb1ba7cfcd0090fafe5ee1 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 01:32:52 +0200 Subject: [PATCH 17/26] refactor(metrics): use fieldRecord as single source of truth --- runner/internal/metrics/metrics.go | 90 +++++++++++++++++++----------- 1 file changed, 58 insertions(+), 32 deletions(-) diff --git a/runner/internal/metrics/metrics.go b/runner/internal/metrics/metrics.go index d8ee97d..000aba6 100644 --- a/runner/internal/metrics/metrics.go +++ b/runner/internal/metrics/metrics.go @@ -5,10 +5,10 @@ import "reflect" // Value is a concrete metric value, e.g. 120 or 3 * time.Second. type Value interface{} -// Field represents the origin of a Metric. -// It exposes a method Type that returns the type of the metric. -// It can be used to reference a Metric in an Aggregate -// via Aggregate.MetricOf. +// Field references an Aggregate field. +// It exposes a method Type that returns its intrisic type. +// It can be used to retrieve a Metric from an Aggregate +// via Aggregate.MetricOf(field). type Field string const ( @@ -20,6 +20,44 @@ const ( RequestCount Field = "TOTAL_COUNT" ) +type fieldInfo struct { + typ Type + metricOf func(Aggregate) Metric +} + +var fieldRecord = map[Field]fieldInfo{ + ResponseTimeAvg: {TypeDuration, metricGetter(ResponseTimeAvg)}, + ResponseTimeMin: {TypeDuration, metricGetter(ResponseTimeMin)}, + ResponseTimeMax: {TypeDuration, metricGetter(ResponseTimeMax)}, + RequestFailCount: {TypeInt, metricGetter(RequestFailCount)}, + RequestSuccessCount: {TypeInt, metricGetter(RequestSuccessCount)}, + RequestCount: {TypeInt, metricGetter(RequestCount)}, +} + +func metricGetter(field Field) func(Aggregate) Metric { + getter := func(getValue func(Aggregate) Value) func(Aggregate) Metric { + return func(agg Aggregate) Metric { + return Metric{Field: field, Value: getValue(agg)} + } + } + switch field { + case ResponseTimeAvg: + return getter(func(agg Aggregate) Value { return agg.Avg }) + case ResponseTimeMin: + return getter(func(agg Aggregate) Value { return agg.Min }) + case ResponseTimeMax: + return getter(func(agg Aggregate) Value { return agg.Max }) + case RequestFailCount: + return getter(func(agg Aggregate) Value { return agg.FailureCount }) + case RequestSuccessCount: + return getter(func(agg Aggregate) Value { return agg.SuccessCount }) + case RequestCount: + return getter(func(agg Aggregate) Value { return agg.TotalCount }) + default: + panic(badField(field)) + } +} + // Type represents the underlying type of a Value. type Type uint8 @@ -32,37 +70,14 @@ const ( TypeDuration = Type(lastGoReflectKind + iota) ) -// Type returns the underlying type of the metric field refers to. +// Type returns the field's intrisic type. func (field Field) Type() Type { - switch field { - case ResponseTimeAvg, ResponseTimeMin, ResponseTimeMax: - return TypeDuration - case RequestFailCount, RequestSuccessCount, RequestCount: - return TypeInt - } - panic(badField(field)) + return retrieveFieldOrPanic(field).typ } -// MetricOf returns the Metric for the given Field in Aggregate. +// MetricOf returns the Metric for the given Source in Aggregate. func (agg Aggregate) MetricOf(field Field) Metric { - var v interface{} - switch field { - case ResponseTimeAvg: - v = agg.Avg - case ResponseTimeMin: - v = agg.Min - case ResponseTimeMax: - v = agg.Max - case RequestFailCount: - v = agg.FailureCount - case RequestSuccessCount: - v = agg.SuccessCount - case RequestCount: - v = agg.TotalCount - default: - panic(badField(field)) - } - return Metric{Field: field, Value: v} + return retrieveFieldOrPanic(field).metricOf(agg) } // Metric represents an Aggregate metric. It links together a Field @@ -97,6 +112,17 @@ func (m Metric) Compare(to Metric) ComparisonResult { return compareMetrics(to, m) } +// retrieveFieldInfoOrPanic retrieves the fieldInfo for the given field. +// +// It panics if the field is not defined in fieldRecord. +func retrieveFieldOrPanic(field Field) fieldInfo { + fieldData, ok := fieldRecord[field] + if !ok { + panic(badField(field)) + } + return fieldData +} + func badField(field Field) string { - return "metrics: unknown Field: " + string(field) + return "metrics: unknown field: " + string(field) } From fb5147fed052670156561547b04b290ebe4c5428 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 01:37:56 +0200 Subject: [PATCH 18/26] refactor(metrics): reorganize files --- runner/internal/metrics/aggregate.go | 5 ++ runner/internal/metrics/field.go | 88 ++++++++++++++++++++++++++ runner/internal/metrics/metrics.go | 92 ---------------------------- 3 files changed, 93 insertions(+), 92 deletions(-) create mode 100644 runner/internal/metrics/field.go diff --git a/runner/internal/metrics/aggregate.go b/runner/internal/metrics/aggregate.go index 97b6594..c75b89b 100644 --- a/runner/internal/metrics/aggregate.go +++ b/runner/internal/metrics/aggregate.go @@ -17,6 +17,11 @@ type Aggregate struct { // RequestEventsDistribution map[recorder.Event]int } +// MetricOf returns the Metric for the given Source in Aggregate. +func (agg Aggregate) MetricOf(field Field) Metric { + return retrieveFieldOrPanic(field).metricOf(agg) +} + // Compute computes and aggregates metrics from the given // requests records. func Compute(records []recorder.Record) (agg Aggregate) { diff --git a/runner/internal/metrics/field.go b/runner/internal/metrics/field.go new file mode 100644 index 0000000..f5f13d6 --- /dev/null +++ b/runner/internal/metrics/field.go @@ -0,0 +1,88 @@ +package metrics + +import "reflect" + +// Field references an Aggregate field. +// It exposes a method Type that returns its intrisic type. +// It can be used to retrieve a Metric from an Aggregate +// via Aggregate.MetricOf(field). +type Field string + +const ( + ResponseTimeAvg Field = "AVG" + ResponseTimeMin Field = "MIN" + ResponseTimeMax Field = "MAX" + RequestFailCount Field = "FAILURE_COUNT" + RequestSuccessCount Field = "SUCCESS_COUNT" + RequestCount Field = "TOTAL_COUNT" +) + +type fieldInfo struct { + typ Type + metricOf func(Aggregate) Metric +} + +var fieldRecord = map[Field]fieldInfo{ + ResponseTimeAvg: {TypeDuration, metricGetter(ResponseTimeAvg)}, + ResponseTimeMin: {TypeDuration, metricGetter(ResponseTimeMin)}, + ResponseTimeMax: {TypeDuration, metricGetter(ResponseTimeMax)}, + RequestFailCount: {TypeInt, metricGetter(RequestFailCount)}, + RequestSuccessCount: {TypeInt, metricGetter(RequestSuccessCount)}, + RequestCount: {TypeInt, metricGetter(RequestCount)}, +} + +func metricGetter(field Field) func(Aggregate) Metric { + getter := func(getValue func(Aggregate) Value) func(Aggregate) Metric { + return func(agg Aggregate) Metric { + return Metric{Field: field, Value: getValue(agg)} + } + } + switch field { + case ResponseTimeAvg: + return getter(func(agg Aggregate) Value { return agg.Avg }) + case ResponseTimeMin: + return getter(func(agg Aggregate) Value { return agg.Min }) + case ResponseTimeMax: + return getter(func(agg Aggregate) Value { return agg.Max }) + case RequestFailCount: + return getter(func(agg Aggregate) Value { return agg.FailureCount }) + case RequestSuccessCount: + return getter(func(agg Aggregate) Value { return agg.SuccessCount }) + case RequestCount: + return getter(func(agg Aggregate) Value { return agg.TotalCount }) + default: + panic(badField(field)) + } +} + +// Type represents the underlying type of a Value. +type Type uint8 + +const ( + lastGoReflectKind = reflect.UnsafePointer + + // TypeInt corresponds to type int. + TypeInt = Type(reflect.Int) + // TypeDuration corresponds to type time.Duration. + TypeDuration = Type(lastGoReflectKind + iota) +) + +// Type returns the field's intrisic type. +func (field Field) Type() Type { + return retrieveFieldOrPanic(field).typ +} + +// retrieveFieldInfoOrPanic retrieves the fieldInfo for the given field. +// +// It panics if the field is not defined in fieldRecord. +func retrieveFieldOrPanic(field Field) fieldInfo { + fieldData, ok := fieldRecord[field] + if !ok { + panic(badField(field)) + } + return fieldData +} + +func badField(field Field) string { + return "metrics: unknown field: " + string(field) +} diff --git a/runner/internal/metrics/metrics.go b/runner/internal/metrics/metrics.go index 000aba6..7b9fca8 100644 --- a/runner/internal/metrics/metrics.go +++ b/runner/internal/metrics/metrics.go @@ -1,85 +1,8 @@ package metrics -import "reflect" - // Value is a concrete metric value, e.g. 120 or 3 * time.Second. type Value interface{} -// Field references an Aggregate field. -// It exposes a method Type that returns its intrisic type. -// It can be used to retrieve a Metric from an Aggregate -// via Aggregate.MetricOf(field). -type Field string - -const ( - ResponseTimeAvg Field = "AVG" - ResponseTimeMin Field = "MIN" - ResponseTimeMax Field = "MAX" - RequestFailCount Field = "FAILURE_COUNT" - RequestSuccessCount Field = "SUCCESS_COUNT" - RequestCount Field = "TOTAL_COUNT" -) - -type fieldInfo struct { - typ Type - metricOf func(Aggregate) Metric -} - -var fieldRecord = map[Field]fieldInfo{ - ResponseTimeAvg: {TypeDuration, metricGetter(ResponseTimeAvg)}, - ResponseTimeMin: {TypeDuration, metricGetter(ResponseTimeMin)}, - ResponseTimeMax: {TypeDuration, metricGetter(ResponseTimeMax)}, - RequestFailCount: {TypeInt, metricGetter(RequestFailCount)}, - RequestSuccessCount: {TypeInt, metricGetter(RequestSuccessCount)}, - RequestCount: {TypeInt, metricGetter(RequestCount)}, -} - -func metricGetter(field Field) func(Aggregate) Metric { - getter := func(getValue func(Aggregate) Value) func(Aggregate) Metric { - return func(agg Aggregate) Metric { - return Metric{Field: field, Value: getValue(agg)} - } - } - switch field { - case ResponseTimeAvg: - return getter(func(agg Aggregate) Value { return agg.Avg }) - case ResponseTimeMin: - return getter(func(agg Aggregate) Value { return agg.Min }) - case ResponseTimeMax: - return getter(func(agg Aggregate) Value { return agg.Max }) - case RequestFailCount: - return getter(func(agg Aggregate) Value { return agg.FailureCount }) - case RequestSuccessCount: - return getter(func(agg Aggregate) Value { return agg.SuccessCount }) - case RequestCount: - return getter(func(agg Aggregate) Value { return agg.TotalCount }) - default: - panic(badField(field)) - } -} - -// Type represents the underlying type of a Value. -type Type uint8 - -const ( - lastGoReflectKind = reflect.UnsafePointer - - // TypeInt corresponds to type int. - TypeInt = Type(reflect.Int) - // TypeDuration corresponds to type time.Duration. - TypeDuration = Type(lastGoReflectKind + iota) -) - -// Type returns the field's intrisic type. -func (field Field) Type() Type { - return retrieveFieldOrPanic(field).typ -} - -// MetricOf returns the Metric for the given Source in Aggregate. -func (agg Aggregate) MetricOf(field Field) Metric { - return retrieveFieldOrPanic(field).metricOf(agg) -} - // Metric represents an Aggregate metric. It links together a Field // and its Value from the Aggregate. // It exposes a method Compare that compares its Value to another. @@ -111,18 +34,3 @@ type Metric struct { func (m Metric) Compare(to Metric) ComparisonResult { return compareMetrics(to, m) } - -// retrieveFieldInfoOrPanic retrieves the fieldInfo for the given field. -// -// It panics if the field is not defined in fieldRecord. -func retrieveFieldOrPanic(field Field) fieldInfo { - fieldData, ok := fieldRecord[field] - if !ok { - panic(badField(field)) - } - return fieldData -} - -func badField(field Field) string { - return "metrics: unknown field: " + string(field) -} From 3f5a104d50c8d9965a1bd84bdb5b98dc4244281e Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 02:08:53 +0200 Subject: [PATCH 19/26] refactor(metrics): improve field understandability - remove helper metricGetter - some renamings - add documenting comments --- runner/internal/metrics/aggregate.go | 6 ++- runner/internal/metrics/field.go | 73 ++++++++++++---------------- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/runner/internal/metrics/aggregate.go b/runner/internal/metrics/aggregate.go index c75b89b..de972c1 100644 --- a/runner/internal/metrics/aggregate.go +++ b/runner/internal/metrics/aggregate.go @@ -17,9 +17,11 @@ type Aggregate struct { // RequestEventsDistribution map[recorder.Event]int } -// MetricOf returns the Metric for the given Source in Aggregate. +// MetricOf returns the Metric for the given field in Aggregate. +// +// It panics if field is not a known field. func (agg Aggregate) MetricOf(field Field) Metric { - return retrieveFieldOrPanic(field).metricOf(agg) + return Metric{Field: field, Value: field.valueIn(agg)} } // Compute computes and aggregates metrics from the given diff --git a/runner/internal/metrics/field.go b/runner/internal/metrics/field.go index f5f13d6..b25d106 100644 --- a/runner/internal/metrics/field.go +++ b/runner/internal/metrics/field.go @@ -2,7 +2,7 @@ package metrics import "reflect" -// Field references an Aggregate field. +// Field is the name of an Aggregate field. // It exposes a method Type that returns its intrisic type. // It can be used to retrieve a Metric from an Aggregate // via Aggregate.MetricOf(field). @@ -17,42 +17,27 @@ const ( RequestCount Field = "TOTAL_COUNT" ) -type fieldInfo struct { - typ Type - metricOf func(Aggregate) Metric +// fieldDefinition holds the necessary values to identify +// and manipulate a field. +// It consists of an intrinsic type and an accessor that binds +// the field to an Aggregate metric value. +type fieldDefinition struct { + // typ is the intrisic type of the field. + typ Type + // boundValue is an accessor that binds a field + // to the value it represents in an Aggregate. + boundValue func(Aggregate) Value } -var fieldRecord = map[Field]fieldInfo{ - ResponseTimeAvg: {TypeDuration, metricGetter(ResponseTimeAvg)}, - ResponseTimeMin: {TypeDuration, metricGetter(ResponseTimeMin)}, - ResponseTimeMax: {TypeDuration, metricGetter(ResponseTimeMax)}, - RequestFailCount: {TypeInt, metricGetter(RequestFailCount)}, - RequestSuccessCount: {TypeInt, metricGetter(RequestSuccessCount)}, - RequestCount: {TypeInt, metricGetter(RequestCount)}, -} - -func metricGetter(field Field) func(Aggregate) Metric { - getter := func(getValue func(Aggregate) Value) func(Aggregate) Metric { - return func(agg Aggregate) Metric { - return Metric{Field: field, Value: getValue(agg)} - } - } - switch field { - case ResponseTimeAvg: - return getter(func(agg Aggregate) Value { return agg.Avg }) - case ResponseTimeMin: - return getter(func(agg Aggregate) Value { return agg.Min }) - case ResponseTimeMax: - return getter(func(agg Aggregate) Value { return agg.Max }) - case RequestFailCount: - return getter(func(agg Aggregate) Value { return agg.FailureCount }) - case RequestSuccessCount: - return getter(func(agg Aggregate) Value { return agg.SuccessCount }) - case RequestCount: - return getter(func(agg Aggregate) Value { return agg.TotalCount }) - default: - panic(badField(field)) - } +// fieldDefinitions is a table of truth for fields. +// It maps all Field references to their intrinsic fieldDefinition. +var fieldDefinitions = map[Field]fieldDefinition{ + ResponseTimeAvg: {TypeDuration, func(a Aggregate) Value { return a.Avg }}, + ResponseTimeMin: {TypeDuration, func(a Aggregate) Value { return a.Min }}, + ResponseTimeMax: {TypeDuration, func(a Aggregate) Value { return a.Max }}, + RequestFailCount: {TypeInt, func(a Aggregate) Value { return a.FailureCount }}, + RequestSuccessCount: {TypeInt, func(a Aggregate) Value { return a.SuccessCount }}, + RequestCount: {TypeInt, func(a Aggregate) Value { return a.TotalCount }}, } // Type represents the underlying type of a Value. @@ -68,15 +53,21 @@ const ( ) // Type returns the field's intrisic type. +// It panics if field is not defined in fieldDefinitions. func (field Field) Type() Type { - return retrieveFieldOrPanic(field).typ + return field.mustRetrieve().typ +} + +// valueIn returns the field's bound value in the given Aggregate. +// It panics if field is not defined in fieldDefinitions. +func (field Field) valueIn(agg Aggregate) Value { + return field.mustRetrieve().boundValue(agg) } -// retrieveFieldInfoOrPanic retrieves the fieldInfo for the given field. -// -// It panics if the field is not defined in fieldRecord. -func retrieveFieldOrPanic(field Field) fieldInfo { - fieldData, ok := fieldRecord[field] +// mustRetrieve retrieves the fieldDefinition for the given field +// from fieldDefinitions, or panics if not found. +func (field Field) mustRetrieve() fieldDefinition { + fieldData, ok := fieldDefinitions[field] if !ok { panic(badField(field)) } From b0402d0d78fb0d2ce438145867d2c1ccdb6c0e0c Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 03:29:27 +0200 Subject: [PATCH 20/26] refactor(configparse): Ultimate Once And For All Solution --- internal/configparse/parse.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index 227690e..bb8abe0 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -174,11 +174,8 @@ func (pconf *parsedConfig) add(field string) { // newParsedConfig parses an input raw config as a runner.ConfigGlobal and returns // a parsedConfig or the first non-nil error occurring in the process. func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:gocognit // acceptable complexity for a parsing func - const numField = 13 // should match the number of config Fields (not critical) - - pconf := parsedConfig{ - fields: make([]string, 0, numField), - } + maxFields := len(runner.ConfigFieldsUsage) + pconf := parsedConfig{fields: make([]string, 0, maxFields)} cfg := &pconf.config if method := uconf.Request.Method; method != nil { From 360fa20f54a5723bbbbc60bcdb6f9eedc1c7f62e Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 13:01:16 +0200 Subject: [PATCH 21/26] feat(configparse): error handling --- internal/configparse/parse.go | 106 ++++++++++++++++++++++------- runner/internal/metrics/error.go | 12 ++++ runner/internal/metrics/field.go | 44 +++++++++++- runner/internal/tests/error.go | 12 ++++ runner/internal/tests/predicate.go | 9 +++ 5 files changed, 154 insertions(+), 29 deletions(-) create mode 100644 runner/internal/metrics/error.go create mode 100644 runner/internal/tests/error.go diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index bb8abe0..fb792ef 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -178,6 +178,10 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g pconf := parsedConfig{fields: make([]string, 0, maxFields)} cfg := &pconf.config + abort := func(err error) (parsedConfig, error) { + return parsedConfig{}, err + } + if method := uconf.Request.Method; method != nil { cfg.Request.Method = *method pconf.add(runner.ConfigFieldMethod) @@ -186,7 +190,7 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g if rawURL := uconf.Request.URL; rawURL != nil { parsedURL, err := parseAndBuildURL(*uconf.Request.URL, uconf.Request.QueryParams) if err != nil { - return parsedConfig{}, err + return abort(err) } cfg.Request.URL = parsedURL pconf.add(runner.ConfigFieldURL) @@ -222,7 +226,7 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g if interval := uconf.Runner.Interval; interval != nil { parsedInterval, err := parseOptionalDuration(*interval) if err != nil { - return parsedConfig{}, err + return abort(err) } cfg.Runner.Interval = parsedInterval pconf.add(runner.ConfigFieldInterval) @@ -231,7 +235,7 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g if requestTimeout := uconf.Runner.RequestTimeout; requestTimeout != nil { parsedTimeout, err := parseOptionalDuration(*requestTimeout) if err != nil { - return parsedConfig{}, err + return abort(err) } cfg.Runner.RequestTimeout = parsedTimeout pconf.add(runner.ConfigFieldRequestTimeout) @@ -240,7 +244,7 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g if globalTimeout := uconf.Runner.GlobalTimeout; globalTimeout != nil { parsedGlobalTimeout, err := parseOptionalDuration(*globalTimeout) if err != nil { - return parsedConfig{}, err + return abort(err) } cfg.Runner.GlobalTimeout = parsedGlobalTimeout pconf.add(runner.ConfigFieldGlobalTimeout) @@ -256,25 +260,50 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g pconf.add(runner.ConfigFieldTemplate) } - // TODO: handle nil values (assumed non nil for now) - if tests := uconf.Tests; len(tests) > 0 { - cases := make([]runner.TestCase, len(tests)) - for i, t := range tests { - field := runner.MetricsField(*t.Field) - target, err := parseMetricValue(field.Type(), *t.Target) - if err != nil { - return parsedConfig{}, err - } - cases[i] = runner.TestCase{ - Name: *t.Name, - Field: runner.MetricsField(*t.Field), - Predicate: runner.TestPredicate(*t.Predicate), - Target: target, - } + testSuite := uconf.Tests + if len(testSuite) == 0 { + return pconf, nil + } + + cases := make([]runner.TestCase, len(testSuite)) + for i, t := range testSuite { + fieldPath := func(caseField string) string { + return fmt.Sprintf("tests[%d].%s", i, caseField) + } + + if err := requireConfigFields(map[string]*string{ + fieldPath("name"): t.Name, + fieldPath("field"): t.Field, + fieldPath("predicate"): t.Predicate, + fieldPath("target"): t.Target, + }); err != nil { + return abort(err) + } + + field := runner.MetricsField(*t.Field) + if err := field.Validate(); err != nil { + return abort(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)) + } + + target, err := parseMetricValue(field, *t.Target) + if err != nil { + return abort(fmt.Errorf("%s: %s", fieldPath("target"), err)) + } + + cases[i] = runner.TestCase{ + Name: *t.Name, + Field: field, + Predicate: predicate, + Target: target, } - cfg.Tests = cases - pconf.add(runner.ConfigFieldTests) } + cfg.Tests = cases + pconf.add(runner.ConfigFieldTests) return pconf, nil } @@ -313,13 +342,38 @@ func parseOptionalDuration(raw string) (time.Duration, error) { return time.ParseDuration(raw) } -func parseMetricValue(typ runner.MetricsType, in string) (runner.MetricsValue, error) { - switch typ { +func parseMetricValue(field runner.MetricsField, in string) (runner.MetricsValue, error) { + handleError := func(v interface{}, err error) (runner.MetricsValue, error) { + if err != nil { + return nil, fmt.Errorf( + "value %q is incompatible with field %s (want %s)", + in, field, field.Type(), + ) + } + return v, nil + } + switch typ := field.Type(); typ { case runner.MetricsTypeInt: - return strconv.Atoi(in) + return handleError(strconv.Atoi(in)) case runner.MetricsTypeDuration: - return time.ParseDuration(in) + return handleError(time.ParseDuration(in)) default: - return nil, fmt.Errorf("cannot parse metrics type: %v", typ) + return nil, fmt.Errorf("unknown field: %v", field) } } + +func requireConfigFields(fields map[string]*string) error { + for name, value := range fields { + if err := requireConfigField(name, value); err != nil { + return err + } + } + return nil +} + +func requireConfigField(field string, value *string) error { + if value == nil { + return fmt.Errorf("%s: missing field", field) + } + return nil +} diff --git a/runner/internal/metrics/error.go b/runner/internal/metrics/error.go new file mode 100644 index 0000000..6c54928 --- /dev/null +++ b/runner/internal/metrics/error.go @@ -0,0 +1,12 @@ +package metrics + +import ( + "errors" + "fmt" +) + +var ErrUnknownField = errors.New("metrics: unknown field") + +func errWithDetails(err error, details interface{}) error { + return fmt.Errorf("%w: %v", err, details) +} diff --git a/runner/internal/metrics/field.go b/runner/internal/metrics/field.go index b25d106..da8a819 100644 --- a/runner/internal/metrics/field.go +++ b/runner/internal/metrics/field.go @@ -1,6 +1,8 @@ package metrics -import "reflect" +import ( + "reflect" +) // Field is the name of an Aggregate field. // It exposes a method Type that returns its intrisic type. @@ -52,23 +54,59 @@ const ( TypeDuration = Type(lastGoReflectKind + iota) ) +// String returns a human-readable representation of the field. +// +// Example: +// TypeDuration.String() == "time.Duration" +// Type(123).String() == "[unknown type]" +func (typ Type) String() string { + switch typ { + case TypeInt: + return "int" + case TypeDuration: + return "time.Duration" + default: + return "[unknown type]" + } +} + // Type returns the field's intrisic type. // It panics if field is not defined in fieldDefinitions. func (field Field) Type() Type { return field.mustRetrieve().typ } +// Validate returns ErrUnknownField if field is not a know Field, else nil. +func (field Field) Validate() error { + if !field.exists() { + return errWithDetails(ErrUnknownField, field) + } + return nil +} + +// func (field Field) IsCompatibleWith() + // valueIn returns the field's bound value in the given Aggregate. // It panics if field is not defined in fieldDefinitions. func (field Field) valueIn(agg Aggregate) Value { return field.mustRetrieve().boundValue(agg) } +func (field Field) retrieve() (fieldDefinition, bool) { + fieldData, exists := fieldDefinitions[field] + return fieldData, exists +} + +func (field Field) exists() bool { + _, exists := fieldDefinitions[field] + return exists +} + // mustRetrieve retrieves the fieldDefinition for the given field // from fieldDefinitions, or panics if not found. func (field Field) mustRetrieve() fieldDefinition { - fieldData, ok := fieldDefinitions[field] - if !ok { + fieldData, exists := field.retrieve() + if !exists { panic(badField(field)) } return fieldData diff --git a/runner/internal/tests/error.go b/runner/internal/tests/error.go new file mode 100644 index 0000000..6057344 --- /dev/null +++ b/runner/internal/tests/error.go @@ -0,0 +1,12 @@ +package tests + +import ( + "errors" + "fmt" +) + +var ErrUnknownPredicate = errors.New("tests: unknown predicate") + +func errWithDetails(err error, details interface{}) error { + return fmt.Errorf("%w: %s", err, details) +} diff --git a/runner/internal/tests/predicate.go b/runner/internal/tests/predicate.go index b662b8f..7b26483 100644 --- a/runner/internal/tests/predicate.go +++ b/runner/internal/tests/predicate.go @@ -4,6 +4,7 @@ import ( "github.com/benchttp/engine/runner/internal/metrics" ) +// Predicate represents a comparison operator. type Predicate string const ( @@ -15,6 +16,14 @@ const ( LTE Predicate = "LTE" ) +// Validate returns ErrUnknownPredicate if p is not a know Predicate, else nil. +func (p Predicate) Validate() error { + if _, ok := predicateSymbols[p]; !ok { + return errWithDetails(ErrUnknownPredicate, p) + } + return nil +} + func (p Predicate) match(comparisonResult metrics.ComparisonResult) bool { sup := comparisonResult == metrics.SUP inf := comparisonResult == metrics.INF From 24b2b4c6043c9ee74b85c72ec64ee51bcedcaf0f Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 13:18:27 +0200 Subject: [PATCH 22/26] refactor: create package errorutil - implement errorutil.WithDetails - replace local implementations --- internal/configparse/error.go | 12 ------------ internal/configparse/parse.go | 11 ++++++----- internal/errorutil/errorutil.go | 23 +++++++++++++++++++++++ runner/internal/metrics/error.go | 12 ------------ runner/internal/metrics/field.go | 7 ++++++- runner/internal/tests/error.go | 12 ------------ runner/internal/tests/predicate.go | 7 ++++++- 7 files changed, 41 insertions(+), 43 deletions(-) create mode 100644 internal/errorutil/errorutil.go delete mode 100644 runner/internal/metrics/error.go delete mode 100644 runner/internal/tests/error.go diff --git a/internal/configparse/error.go b/internal/configparse/error.go index a9e0ab4..899d3ad 100644 --- a/internal/configparse/error.go +++ b/internal/configparse/error.go @@ -2,8 +2,6 @@ package configparse import ( "errors" - "fmt" - "strings" ) var ( @@ -26,13 +24,3 @@ var ( // ErrCircularExtends signals a circular reference in the config file. ErrCircularExtends = errors.New("circular reference detected") ) - -// errWithDetails returns an error wrapping err, appended with a string -// representation of details separated by ": ". -func errWithDetails(err error, details ...interface{}) error { - detailsStr := make([]string, len(details)) - for i := range details { - detailsStr[i] = fmt.Sprint(details[i]) - } - return fmt.Errorf("%w: %s", err, strings.Join(detailsStr, ": ")) -} diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index fb792ef..f5289dc 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -10,6 +10,7 @@ import ( "strconv" "time" + "github.com/benchttp/engine/internal/errorutil" "github.com/benchttp/engine/runner" ) @@ -114,19 +115,19 @@ func parseFile(filename string) (uconf UnmarshaledConfig, err error) { switch { case err == nil: case errors.Is(err, os.ErrNotExist): - return uconf, errWithDetails(ErrFileNotFound, filename) + return uconf, errorutil.WithDetails(ErrFileNotFound, filename) default: - return uconf, errWithDetails(ErrFileRead, filename, err) + return uconf, errorutil.WithDetails(ErrFileRead, filename, err) } ext := extension(filepath.Ext(filename)) parser, err := newParser(ext) if err != nil { - return uconf, errWithDetails(ErrFileExt, ext, err) + return uconf, errorutil.WithDetails(ErrFileExt, ext, err) } if err = parser.parse(b, &uconf); err != nil { - return uconf, errWithDetails(ErrParse, filename, err) + return uconf, errorutil.WithDetails(ErrParse, filename, err) } return uconf, nil @@ -151,7 +152,7 @@ func parseAndMergeConfigs(uconfs []UnmarshaledConfig) (cfg runner.Config, err er uconf := uconfs[i] pconf, err := newParsedConfig(uconf) if err != nil { - return cfg, errWithDetails(ErrParse, err) + return cfg, errorutil.WithDetails(ErrParse, err) } cfg = cfg.Override(pconf.config, pconf.fields...) } diff --git a/internal/errorutil/errorutil.go b/internal/errorutil/errorutil.go new file mode 100644 index 0000000..c52fee9 --- /dev/null +++ b/internal/errorutil/errorutil.go @@ -0,0 +1,23 @@ +package errorutil + +import ( + "fmt" + "strings" +) + +// WithDetails returns an error wrapping err, appended with a string +// representation of details separated by ": ". +// +// Example +// var ErrNotFound = errors.New("not found") +// err := WithDetails(ErrNotFound, "abc.jpg", "deleted yesterday") +// +// errors.Is(err, ErrNotFound) == true +// err.Error() == "not found: abc.jpg: deleted yesterday" +func WithDetails(base error, details ...interface{}) error { + detailsStr := make([]string, len(details)) + for i := range details { + detailsStr[i] = fmt.Sprint(details[i]) + } + return fmt.Errorf("%w: %s", base, strings.Join(detailsStr, ": ")) +} diff --git a/runner/internal/metrics/error.go b/runner/internal/metrics/error.go deleted file mode 100644 index 6c54928..0000000 --- a/runner/internal/metrics/error.go +++ /dev/null @@ -1,12 +0,0 @@ -package metrics - -import ( - "errors" - "fmt" -) - -var ErrUnknownField = errors.New("metrics: unknown field") - -func errWithDetails(err error, details interface{}) error { - return fmt.Errorf("%w: %v", err, details) -} diff --git a/runner/internal/metrics/field.go b/runner/internal/metrics/field.go index da8a819..1db2b9d 100644 --- a/runner/internal/metrics/field.go +++ b/runner/internal/metrics/field.go @@ -1,9 +1,14 @@ package metrics import ( + "errors" "reflect" + + "github.com/benchttp/engine/internal/errorutil" ) +var ErrUnknownField = errors.New("metrics: unknown field") + // Field is the name of an Aggregate field. // It exposes a method Type that returns its intrisic type. // It can be used to retrieve a Metric from an Aggregate @@ -79,7 +84,7 @@ func (field Field) Type() Type { // Validate returns ErrUnknownField if field is not a know Field, else nil. func (field Field) Validate() error { if !field.exists() { - return errWithDetails(ErrUnknownField, field) + return errorutil.WithDetails(ErrUnknownField, field) } return nil } diff --git a/runner/internal/tests/error.go b/runner/internal/tests/error.go deleted file mode 100644 index 6057344..0000000 --- a/runner/internal/tests/error.go +++ /dev/null @@ -1,12 +0,0 @@ -package tests - -import ( - "errors" - "fmt" -) - -var ErrUnknownPredicate = errors.New("tests: unknown predicate") - -func errWithDetails(err error, details interface{}) error { - return fmt.Errorf("%w: %s", err, details) -} diff --git a/runner/internal/tests/predicate.go b/runner/internal/tests/predicate.go index 7b26483..d8146d0 100644 --- a/runner/internal/tests/predicate.go +++ b/runner/internal/tests/predicate.go @@ -1,9 +1,14 @@ package tests import ( + "errors" + + "github.com/benchttp/engine/internal/errorutil" "github.com/benchttp/engine/runner/internal/metrics" ) +var ErrUnknownPredicate = errors.New("tests: unknown predicate") + // Predicate represents a comparison operator. type Predicate string @@ -19,7 +24,7 @@ const ( // Validate returns ErrUnknownPredicate if p is not a know Predicate, else nil. func (p Predicate) Validate() error { if _, ok := predicateSymbols[p]; !ok { - return errWithDetails(ErrUnknownPredicate, p) + return errorutil.WithDetails(ErrUnknownPredicate, p) } return nil } From 76c397dc540c1fa4b7d8acfda475a7fdc9e29228 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 13:41:34 +0200 Subject: [PATCH 23/26] feat(cli): exit 1 if the test suite fails --- cmd/benchttp/run.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/benchttp/run.go b/cmd/benchttp/run.go index b4a8929..6806bbd 100644 --- a/cmd/benchttp/run.go +++ b/cmd/benchttp/run.go @@ -67,6 +67,10 @@ func (cmd *cmdRun) execute(args []string) error { return err } + if !out.Tests.Pass { + return errors.New("test suite failed") + } + return nil } From 7aa80dfdf0a78b147cd1919df93d889fc2f8fa8a Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 13:42:03 +0200 Subject: [PATCH 24/26] ui(cli): improve test suite results display --- runner/internal/report/report.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/runner/internal/report/report.go b/runner/internal/report/report.go index aecd079..cc069cd 100644 --- a/runner/internal/report/report.go +++ b/runner/internal/report/report.go @@ -137,12 +137,13 @@ func (rep *Report) writeTestsResult(w io.StringWriter) { for _, tr := range sr.Results { writeIndent(w, 1) writeResultString(w, tr.Pass) - w.WriteString(": ") + w.WriteString(" ") w.WriteString(tr.Input.Name) if !tr.Pass { - w.WriteString("\n") - writeIndent(w, 2) + w.WriteString("\n ") + writeIndent(w, 3) + w.WriteString(ansi.Bold("→ ")) w.WriteString(tr.Summary) } @@ -152,11 +153,9 @@ func (rep *Report) writeTestsResult(w io.StringWriter) { func writeResultString(w io.StringWriter, pass bool) { if pass { - w.WriteString(ansi.Green("√")) - w.WriteString(" PASS") + w.WriteString(ansi.Green("PASS")) } else { - w.WriteString(ansi.Red("x")) - w.WriteString(" FAIL") + w.WriteString(ansi.Red("FAIL")) } } From 54791bee24b9ddc0417bc0eb4a913332cd2fe3fa Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Mon, 1 Aug 2022 19:05:38 +0200 Subject: [PATCH 25/26] fix(configparse): accept any value when unmarshaling tests.target - for field tests[n].target, JSON did not accept int values (but YAML did as they were read as string) --- internal/configparse/parse.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index f5289dc..5fbe18a 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -46,10 +46,10 @@ type UnmarshaledConfig struct { } `yaml:"output" json:"output"` Tests []struct { - Name *string `yaml:"name" json:"name"` - Field *string `yaml:"field" json:"field"` - Predicate *string `yaml:"predicate" json:"predicate"` - Target *string `yaml:"target" json:"target"` + Name *string `yaml:"name" json:"name"` + Field *string `yaml:"field" json:"field"` + Predicate *string `yaml:"predicate" json:"predicate"` + Target interface{} `yaml:"target" json:"target"` } `yaml:"tests" json:"tests"` } @@ -272,7 +272,7 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g return fmt.Sprintf("tests[%d].%s", i, caseField) } - if err := requireConfigFields(map[string]*string{ + if err := requireConfigFields(map[string]interface{}{ fieldPath("name"): t.Name, fieldPath("field"): t.Field, fieldPath("predicate"): t.Predicate, @@ -291,7 +291,7 @@ func newParsedConfig(uconf UnmarshaledConfig) (parsedConfig, error) { //nolint:g return abort(fmt.Errorf("%s: %s", fieldPath("predicate"), err)) } - target, err := parseMetricValue(field, *t.Target) + target, err := parseMetricValue(field, fmt.Sprint(t.Target)) if err != nil { return abort(fmt.Errorf("%s: %s", fieldPath("target"), err)) } @@ -363,7 +363,7 @@ func parseMetricValue(field runner.MetricsField, in string) (runner.MetricsValue } } -func requireConfigFields(fields map[string]*string) error { +func requireConfigFields(fields map[string]interface{}) error { for name, value := range fields { if err := requireConfigField(name, value); err != nil { return err @@ -372,7 +372,7 @@ func requireConfigFields(fields map[string]*string) error { return nil } -func requireConfigField(field string, value *string) error { +func requireConfigField(field string, value interface{}) error { if value == nil { return fmt.Errorf("%s: missing field", field) } From 7c9c4ce1e48ed3b6748408f825b2b5e4f0c191c3 Mon Sep 17 00:00:00 2001 From: Gregory Albouy Date: Tue, 2 Aug 2022 21:28:11 +0200 Subject: [PATCH 26/26] refactor(configparse): remove unnecessary helper --- internal/configparse/parse.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/internal/configparse/parse.go b/internal/configparse/parse.go index 5fbe18a..84e324d 100644 --- a/internal/configparse/parse.go +++ b/internal/configparse/parse.go @@ -365,16 +365,9 @@ func parseMetricValue(field runner.MetricsField, in string) (runner.MetricsValue func requireConfigFields(fields map[string]interface{}) error { for name, value := range fields { - if err := requireConfigField(name, value); err != nil { - return err + if value == nil { + return fmt.Errorf("%s: missing field", name) } } return nil } - -func requireConfigField(field string, value interface{}) error { - if value == nil { - return fmt.Errorf("%s: missing field", field) - } - return nil -}