From 79b3df8cd7708d379dcd7012f5ecd8383706be97 Mon Sep 17 00:00:00 2001 From: Maggie Nolan Date: Wed, 9 May 2018 09:15:14 -0700 Subject: [PATCH] address comments --- internal/driver/cli.go | 19 ++++++++-------- internal/driver/fetch_test.go | 40 ++++++++++++++++------------------ internal/report/report_test.go | 2 +- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/internal/driver/cli.go b/internal/driver/cli.go index 4292b64b..53231786 100644 --- a/internal/driver/cli.go +++ b/internal/driver/cli.go @@ -15,6 +15,7 @@ package driver import ( + "errors" "fmt" "os" "strings" @@ -87,7 +88,7 @@ func parseFlags(o *plugin.Options) (*source, []string, error) { usageMsgVars) }) if len(args) == 0 { - return nil, nil, fmt.Errorf("no profile source specified") + return nil, nil, errors.New("no profile source specified") } var execName string @@ -114,7 +115,7 @@ func parseFlags(o *plugin.Options) (*source, []string, error) { return nil, nil, err } if cmd != nil && *flagHTTP != "" { - return nil, nil, fmt.Errorf("-http is not compatible with an output format on the command line") + return nil, nil, errors.New("-http is not compatible with an output format on the command line") } si := pprofVariables["sample_index"].value @@ -148,7 +149,7 @@ func parseFlags(o *plugin.Options) (*source, []string, error) { normalize := pprofVariables["normalize"].boolValue() if normalize && len(source.Base) == 0 { - return nil, nil, fmt.Errorf("Must have base profile to normalize by") + return nil, nil, errors.New("Must have base profile to normalize by") } source.Normalize = normalize @@ -162,17 +163,15 @@ func parseFlags(o *plugin.Options) (*source, []string, error) { // the source. This function will return an error if both base and diff base // profiles are specified. func (source *source) addBaseProfiles(flagBase, flagDiffBase []*string) error { - base := dropEmpty(flagBase) - diffBase := dropEmpty(flagDiffBase) + base, diffBase := dropEmpty(flagBase), dropEmpty(flagDiffBase) if len(base) > 0 && len(diffBase) > 0 { - return fmt.Errorf("-base and -diff_base flags cannot both be specified") + return errors.New("-base and -diff_base flags cannot both be specified") } source.Base = base if len(diffBase) > 0 { - source.Base = diffBase - source.DiffBase = true + source.Base, source.DiffBase = diffBase, true } return nil } @@ -271,7 +270,7 @@ func outputFormat(bcmd map[string]*bool, acmd map[string]*string) (cmd []string, for n, b := range bcmd { if *b { if cmd != nil { - return nil, fmt.Errorf("must set at most one output format") + return nil, errors.New("must set at most one output format") } cmd = []string{n} } @@ -279,7 +278,7 @@ func outputFormat(bcmd map[string]*bool, acmd map[string]*string) (cmd []string, for n, s := range acmd { if *s != "" { if cmd != nil { - return nil, fmt.Errorf("must set at most one output format") + return nil, errors.New("must set at most one output format") } cmd = []string{n, *s} } diff --git a/internal/driver/fetch_test.go b/internal/driver/fetch_test.go index b0fc51b0..e9e68e2a 100644 --- a/internal/driver/fetch_test.go +++ b/internal/driver/fetch_test.go @@ -231,25 +231,25 @@ func TestFetchWithBase(t *testing.T) { "not normalized base is same as source", []string{path + "cppbench.contention"}, []string{path + "cppbench.contention"}, - []string{}, + nil, false, - []WantSample{}, + nil, "", }, { "not normalized base is same as source", []string{path + "cppbench.contention"}, []string{path + "cppbench.contention"}, - []string{}, + nil, false, - []WantSample{}, + nil, "", }, { "not normalized single source, multiple base (all profiles same)", []string{path + "cppbench.contention"}, []string{path + "cppbench.contention", path + "cppbench.contention"}, - []string{}, + nil, false, []WantSample{ { @@ -283,7 +283,7 @@ func TestFetchWithBase(t *testing.T) { "not normalized, different base and source", []string{path + "cppbench.contention"}, []string{path + "cppbench.small.contention"}, - []string{}, + nil, false, []WantSample{ { @@ -317,7 +317,7 @@ func TestFetchWithBase(t *testing.T) { "normalized base is same as source", []string{path + "cppbench.contention"}, []string{path + "cppbench.contention"}, - []string{}, + nil, true, []WantSample{}, "", @@ -326,7 +326,7 @@ func TestFetchWithBase(t *testing.T) { "normalized single source, multiple base (all profiles same)", []string{path + "cppbench.contention"}, []string{path + "cppbench.contention", path + "cppbench.contention"}, - []string{}, + nil, true, []WantSample{}, "", @@ -335,7 +335,7 @@ func TestFetchWithBase(t *testing.T) { "normalized different base and source", []string{path + "cppbench.contention"}, []string{path + "cppbench.small.contention"}, - []string{}, + nil, true, []WantSample{ { @@ -368,7 +368,7 @@ func TestFetchWithBase(t *testing.T) { { "not normalized diff base is same as source", []string{path + "cppbench.contention"}, - []string{}, + nil, []string{path + "cppbench.contention"}, false, []WantSample{ @@ -466,12 +466,12 @@ func TestFetchWithBase(t *testing.T) { if tc.wantErrorMsg != "" { if err == nil { - t.Errorf("%s: want error %q, got nil", tc.desc, tc.wantErrorMsg) + t.Errorf("%s: got nil, want error %q", tc.desc, tc.wantErrorMsg) continue } gotErrMsg := err.Error() if gotErrMsg != tc.wantErrorMsg { - t.Errorf("%s: want error %q, got error %q", tc.desc, tc.wantErrorMsg, gotErrMsg) + t.Errorf("%s: got error %q, want error %q", tc.desc, gotErrMsg, tc.wantErrorMsg) } continue } @@ -489,18 +489,16 @@ func TestFetchWithBase(t *testing.T) { } if want, got := len(tc.wantSamples), len(p.Sample); want != got { - t.Errorf("%s: want %d samples got %d", tc.desc, want, got) + t.Errorf("%s: got %d samples want %d", tc.desc, got, want) continue } - if len(p.Sample) > 0 { - for i, sample := range p.Sample { - if !reflect.DeepEqual(tc.wantSamples[i].values, sample.Value) { - t.Errorf("%s: for sample %d want values %v, got %v", tc.desc, i, tc.wantSamples[i], sample.Value) - } - if !reflect.DeepEqual(tc.wantSamples[i].labels, sample.Label) { - t.Errorf("%s: for sample %d want labels %v, got %v", tc.desc, i, tc.wantSamples[i].labels, sample.Label) - } + for i, sample := range p.Sample { + if !reflect.DeepEqual(tc.wantSamples[i].values, sample.Value) { + t.Errorf("%s: for sample %d got values %v, want %v", tc.desc, i, sample.Value, tc.wantSamples[i]) + } + if !reflect.DeepEqual(tc.wantSamples[i].labels, sample.Label) { + t.Errorf("%s: for sample %d got labels %v, want %v", tc.desc, i, sample.Label, tc.wantSamples[i].labels) } } } diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 57e0bd26..9a641200 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -401,7 +401,7 @@ func TestComputeTotal(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { gotTotal := computeTotal(tc.prof, tc.value, tc.meanDiv) if gotTotal != tc.wantTotal { - t.Errorf("want total %d, got %v", tc.wantTotal, gotTotal) + t.Errorf("got total %d, want %v", gotTotal, tc.wantTotal) } }) }