From ecf475a27a81442a9ecdc4bdb7776e4fdf01ef47 Mon Sep 17 00:00:00 2001 From: Shai Nagar Date: Fri, 25 Jun 2021 08:51:30 +0300 Subject: [PATCH] clearer txt report error indicators (#139) * more meaningful api names * clearer txt report error indicators --- api/trace.go | 6 +++--- internal/report/csv_rawdata_report_writer.go | 6 +++--- internal/report/md_rawdata_report_writer.go | 6 +++--- internal/report/raw_report_test_utils.go | 6 +++--- internal/report/text_report_writer.go | 18 +++++++++++------- internal/report/text_report_writer_test.go | 2 +- pkg/command_exec.go | 8 ++++---- pkg/fake_summary.go | 4 ++-- pkg/summary.go | 6 +++--- pkg/trace.go | 16 ++++++++-------- pkg/trace_test.go | 12 ++++++------ 11 files changed, 47 insertions(+), 43 deletions(-) diff --git a/api/trace.go b/api/trace.go index 9b87295..a628184 100644 --- a/api/trace.go +++ b/api/trace.go @@ -25,9 +25,9 @@ type Tracer interface { // Trace a single time trace type Trace interface { ID() string - Elapsed() time.Duration - System() time.Duration - User() time.Duration + PerceivedTime() time.Duration + SystemCPUTime() time.Duration + UserCPUTime() time.Duration Error() error } diff --git a/internal/report/csv_rawdata_report_writer.go b/internal/report/csv_rawdata_report_writer.go index 4e631ee..a7c9b45 100644 --- a/internal/report/csv_rawdata_report_writer.go +++ b/internal/report/csv_rawdata_report_writer.go @@ -50,9 +50,9 @@ func (rw CsvStreamReportWriter) Handle(trace api.Trace) (err error) { timeStr, trace.ID(), strings.Join(rw.ctx.Labels, ","), - fmt.Sprintf("%d", trace.Elapsed()), - fmt.Sprintf("%d", trace.User()), - fmt.Sprintf("%d", trace.System()), + fmt.Sprintf("%d", trace.PerceivedTime()), + fmt.Sprintf("%d", trace.UserCPUTime()), + fmt.Sprintf("%d", trace.SystemCPUTime()), fmt.Sprintf("%v", trace.Error() != nil), }) diff --git a/internal/report/md_rawdata_report_writer.go b/internal/report/md_rawdata_report_writer.go index 7866d61..30082ed 100644 --- a/internal/report/md_rawdata_report_writer.go +++ b/internal/report/md_rawdata_report_writer.go @@ -38,9 +38,9 @@ func (rw MarkdownStreamReportWriter) Handle(trace api.Trace) (err error) { timeStr, trace.ID(), strings.Join(rw.ctx.Labels, ","), - fmt.Sprintf("%d", trace.Elapsed()), - fmt.Sprintf("%d", trace.User()), - fmt.Sprintf("%d", trace.System()), + fmt.Sprintf("%d", trace.PerceivedTime()), + fmt.Sprintf("%d", trace.UserCPUTime()), + fmt.Sprintf("%d", trace.SystemCPUTime()), fmt.Sprintf("%v", trace.Error() != nil), }) diff --git a/internal/report/raw_report_test_utils.go b/internal/report/raw_report_test_utils.go index bd9a342..7f2a07b 100644 --- a/internal/report/raw_report_test_utils.go +++ b/internal/report/raw_report_test_utils.go @@ -28,9 +28,9 @@ func assertRawTraceRecord(t *testing.T, trace api.Trace, actualRecord []string) assert.NoError(t, err) assert.Equal(t, trace.ID(), actualRecord[1]) assert.Equal(t, expectedLabels, actualRecord[2]) - assert.Equal(t, fmt.Sprint(trace.Elapsed().Nanoseconds()), actualRecord[3]) - assert.Equal(t, fmt.Sprint(trace.User().Nanoseconds()), actualRecord[4]) - assert.Equal(t, fmt.Sprint(trace.System().Nanoseconds()), actualRecord[5]) + assert.Equal(t, fmt.Sprint(trace.PerceivedTime().Nanoseconds()), actualRecord[3]) + assert.Equal(t, fmt.Sprint(trace.UserCPUTime().Nanoseconds()), actualRecord[4]) + assert.Equal(t, fmt.Sprint(trace.SystemCPUTime().Nanoseconds()), actualRecord[5]) assert.Equal(t, fmt.Sprint(trace.Error() != nil), actualRecord[6]) } diff --git a/internal/report/text_report_writer.go b/internal/report/text_report_writer.go index 4569c65..daa6ff0 100644 --- a/internal/report/text_report_writer.go +++ b/internal/report/text_report_writer.go @@ -12,6 +12,8 @@ import ( log "github.com/sirupsen/logrus" ) +const attentionIndicatorRune = '•' + // textReportWriter a simple human readable test report writer type textReportWriter struct { writer *bufio.Writer @@ -102,6 +104,7 @@ func (trw textReportWriter) Write(summary api.Summary, config api.BenchmarkSpec, trw.writeDurationProperty("user", trw.hiblue, userStats.Mean) trw.writeDurationProperty("system", trw.hiblue, sysStats.Mean) + trw.writeErrorRateStat("errors", stats.ErrorRate) trw.writeNewLine() @@ -144,14 +147,15 @@ func (trw textReportWriter) writeDurationProperty(name string, c *color.Color, f trw.writeProperty(name, FormatReportDuration(f), c) } -func (trw textReportWriter) writeErrorRateStat(name string, f func() float64) { - value := f() - errorRate := int(value * 100) - if errorRate > 0 { - trw.writeString(trw.yellow.Sprintf("%11s: %d%%", name, errorRate)) - } else { - trw.writeString(fmt.Sprintf("%11s: %d%%", name, errorRate)) +func (trw textReportWriter) writeErrorRateStat(name string, errorRate func() float64) { + errorRatePercent := int(errorRate() * 100) + var attentionIndicator = "" + + if errorRatePercent > 10 { + attentionIndicator = trw.red.Sprintf("%c", attentionIndicatorRune) } + + trw.writeString(fmt.Sprintf("%11s: %d%% %s", name, errorRatePercent, attentionIndicator)) } func (trw textReportWriter) writeInt64StatLine(name string, f func() (int64, error)) { diff --git a/internal/report/text_report_writer_test.go b/internal/report/text_report_writer_test.go index 6d0f82d..e4abdd5 100644 --- a/internal/report/text_report_writer_test.go +++ b/internal/report/text_report_writer_test.go @@ -67,7 +67,7 @@ func testTxtSanity(t *testing.T, colorsOn bool) { assert.Equal(t, 1, strings.Count(text, "mean: 2.0s")) assert.Equal(t, 1, strings.Count(text, "errors: 0%")) - assert.Equal(t, 1, strings.Count(text, "errors: 100%")) + assert.Equal(t, 1, strings.Count(text, "errors: 100% •")) assert.Equal(t, 1, strings.Count(text, "system: 1")) assert.Equal(t, 1, strings.Count(text, "system: 2")) diff --git a/pkg/command_exec.go b/pkg/command_exec.go index e35cb47..0474c55 100644 --- a/pkg/command_exec.go +++ b/pkg/command_exec.go @@ -34,16 +34,16 @@ func (ce *commandExecutor) ExecuteFn(cmdSpec *api.CommandSpec, defaultWorkingDir return func() (execInfo *api.ExecutionInfo, err error) { defer cancel() - + startTime := time.Now() err = execCmd.Run() perceivedTime := time.Since(startTime) state := execCmd.ProcessState if state != nil && state.Exited() { execInfo = &api.ExecutionInfo{ - ExitCode: state.ExitCode(), - UserTime: state.UserTime(), - SystemTime: state.SystemTime(), + ExitCode: state.ExitCode(), + UserTime: state.UserTime(), + SystemTime: state.SystemTime(), PerceivedTime: perceivedTime, } diff --git a/pkg/fake_summary.go b/pkg/fake_summary.go index 086891e..126ac3e 100644 --- a/pkg/fake_summary.go +++ b/pkg/fake_summary.go @@ -11,8 +11,8 @@ func NewFakeTrace(id string, elapsed, userTime, sysTime time.Duration, err error return &trace{ id: id, perceivedTime: elapsed, - usrTime: userTime, - sysTime: sysTime, + usrCPUTime: userTime, + sysCPUTime: sysTime, error: err, } } diff --git a/pkg/summary.go b/pkg/summary.go index eb5ef56..af02ac2 100644 --- a/pkg/summary.go +++ b/pkg/summary.go @@ -23,9 +23,9 @@ func NewSummary(tracesByID map[api.ID][]api.Trace) api.Summary { errorCount := 0 for ti := range traces { - perceivedSamples[ti] = float64(traces[ti].Elapsed().Nanoseconds()) - systemSamples[ti] = float64(traces[ti].System().Nanoseconds()) - userSamples[ti] = float64(traces[ti].User().Nanoseconds()) + perceivedSamples[ti] = float64(traces[ti].PerceivedTime().Nanoseconds()) + systemSamples[ti] = float64(traces[ti].SystemCPUTime().Nanoseconds()) + userSamples[ti] = float64(traces[ti].UserCPUTime().Nanoseconds()) if traces[ti].Error() != nil { errorCount++ } diff --git a/pkg/trace.go b/pkg/trace.go index d68e828..4efadd7 100644 --- a/pkg/trace.go +++ b/pkg/trace.go @@ -13,8 +13,8 @@ type tracer struct { type trace struct { id string perceivedTime time.Duration - usrTime time.Duration - sysTime time.Duration + usrCPUTime time.Duration + sysCPUTime time.Duration error error } @@ -22,16 +22,16 @@ func (t trace) ID() string { return t.id } -func (t trace) Elapsed() time.Duration { +func (t trace) PerceivedTime() time.Duration { return t.perceivedTime } -func (t trace) System() time.Duration { - return t.sysTime +func (t trace) SystemCPUTime() time.Duration { + return t.sysCPUTime } -func (t trace) User() time.Duration { - return t.usrTime +func (t trace) UserCPUTime() time.Duration { + return t.usrCPUTime } func (t trace) Error() error { @@ -60,7 +60,7 @@ func (tr *tracer) Start(i api.Identifiable) api.End { func (tr *tracer) endFn(t trace) api.End { return func(execInfo *api.ExecutionInfo, exitError error) { if execInfo != nil { - t.perceivedTime, t.usrTime, t.sysTime = execInfo.PerceivedTime, execInfo.UserTime, execInfo.SystemTime + t.perceivedTime, t.usrCPUTime, t.sysCPUTime = execInfo.PerceivedTime, execInfo.UserTime, execInfo.SystemTime } t.error = exitError diff --git a/pkg/trace_test.go b/pkg/trace_test.go index fa35ded..9593e10 100644 --- a/pkg/trace_test.go +++ b/pkg/trace_test.go @@ -32,9 +32,9 @@ func Test_tracer_endFn(t *testing.T) { ) received := <-tracer.Stream() - assert.Equal(t, expectedPerceivedTime, received.Elapsed()) - assert.Equal(t, expectedUserTime, received.User()) - assert.Equal(t, expectedSysTime, received.System()) + assert.Equal(t, expectedPerceivedTime, received.PerceivedTime()) + assert.Equal(t, expectedUserTime, received.UserCPUTime()) + assert.Equal(t, expectedSysTime, received.SystemCPUTime()) assert.Equal(t, expectedError, received.Error()) assert.Equal(t, expectedID, received.ID()) } @@ -53,9 +53,9 @@ func Test_tracer_endFn_withError(t *testing.T) { ) received := <-tracer.Stream() - assert.Equal(t, expectedDuration, received.Elapsed()) - assert.Equal(t, expectedDuration, received.User()) - assert.Equal(t, expectedDuration, received.System()) + assert.Equal(t, expectedDuration, received.PerceivedTime()) + assert.Equal(t, expectedDuration, received.UserCPUTime()) + assert.Equal(t, expectedDuration, received.SystemCPUTime()) assert.Equal(t, expectedError, received.Error()) assert.Equal(t, expectedID, received.ID()) }