Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
nolanmar511 committed May 9, 2018
1 parent 35882ee commit 82a75d4
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 32 deletions.
19 changes: 9 additions & 10 deletions internal/driver/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package driver

import (
"errors"
"fmt"
"os"
"strings"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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
}
Expand Down Expand Up @@ -271,15 +270,15 @@ 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}
}
}
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}
}
Expand Down
40 changes: 19 additions & 21 deletions internal/driver/fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{},
"",
Expand All @@ -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{},
"",
Expand All @@ -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{
{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/report/report_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down

0 comments on commit 82a75d4

Please sign in to comment.