Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@nolanmar511
nolanmar511 committed Jul 17, 2018
1 parent d64e368 commit 68e5354
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 74 deletions.
6 changes: 3 additions & 3 deletions internal/report/report.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ type Options struct {
// Generate generates a report as directed by the Report.
func Generate(w io.Writer, rpt *Report, obj plugin.ObjTool, ui plugin.UI) error {
if rpt.unexpNegSamples {
ui.PrintErr("Negative sample values appeared in the profile.\nIf using the -base flag to compare profiles, consider using the -diff_base flag instead.")
ui.PrintErr("Profile has negative sample values.\nIf using the -base flag to compare profiles, consider using the -diff_base flag instead.")
}

o := rpt.options
Expand Down Expand Up @@ -1207,7 +1207,7 @@ func New(prof *profile.Profile, o *Options) *Report {
return measurement.ScaledLabel(v, o.SampleUnit, o.OutputUnit)
}
total, unexpNegSamples := computeTotal(prof, o.SampleValue, o.SampleMeanDivisor)
return &Report{prof, total, o, format, unexpNegSamples}
return &Report{prof, total, unexpNegSamples, o, format}
}

// NewDefault builds a new report indexing the last sample value
Expand Down Expand Up @@ -1268,9 +1268,9 @@ func computeTotal(prof *profile.Profile, value, meanDiv func(v []int64) int64) (
type Report struct {
prof *profile.Profile
total int64
unexpNegSamples bool
options *Options
formatValue func(int64) string
unexpNegSamples bool
}

// Total returns the total number of samples in a report.
Expand Down
76 changes: 22 additions & 54 deletions internal/report/report_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,58 +39,61 @@ func TestSource(t *testing.T) {
sampleValue1 := func(v []int64) int64 {
return v[1]
}

negProfile := testProfile.Copy()
negProfile.Sample = negProfile.Sample[:2]
for _, sample := range negProfile.Sample {
for i, v := range sample.Value {
sample.Value[i] = -v
}
}
for _, tc := range []testcase{
{
rpt: New(
testProfile1.Copy(),
testProfile.Copy(),
&Options{
OutputFormat: List,
Symbol: regexp.MustCompile(`.`),
TrimPath: "/some/path",

SampleValue: sampleValue1,
SampleUnit: testProfile1.SampleType[1].Unit,
SampleUnit: testProfile.SampleType[1].Unit,
},
),
want: path + "source.rpt",
},
{
rpt: New(
testProfile1.Copy(),
testProfile.Copy(),
&Options{
OutputFormat: Dot,
CallTree: true,
Symbol: regexp.MustCompile(`.`),
TrimPath: "/some/path",

SampleValue: sampleValue1,
SampleUnit: testProfile1.SampleType[1].Unit,
SampleUnit: testProfile.SampleType[1].Unit,
},
),
want: path + "source.dot",
},
{
rpt: New(
testProfile2.Copy(),
negProfile.Copy(),
&Options{
OutputFormat: Dot,
CallTree: true,
OutputFormat: List,
Symbol: regexp.MustCompile(`.`),
TrimPath: "/some/path",

SampleValue: sampleValue1,
SampleUnit: testProfile2.SampleType[1].Unit,
SampleValue: sampleValue1,
SampleUnit: negProfile.SampleType[1].Unit,
},
),
wantNegSampleErr: true,
want: path + "source_neg_samples.dot",
want: path + "source_neg_samples.rpt",
},
} {
var b bytes.Buffer
ui := &proftest.TestUI{T: t}
if tc.wantNegSampleErr {
ui.AllowRx = "Negative sample values appeared in the profile.\nIf using the -base flag to compare profiles, consider using the -diff_base flag instead."
ui.AllowRx = "Profile has negative sample values.\nIf using the -base flag to compare profiles, consider using the -diff_base flag instead."
}
if err := Generate(&b, tc.rpt, &binutils.Binutils{}, ui); err != nil {
t.Fatalf("%s: %v", tc.want, err)
Expand Down Expand Up @@ -203,7 +206,7 @@ var testL = []*profile.Location{
},
}

var testProfile1 = &profile.Profile{
var testProfile = &profile.Profile{
PeriodType: &profile.ValueType{Type: "cpu", Unit: "millisecond"},
Period: 10,
DurationNanos: 10e9,
Expand Down Expand Up @@ -238,41 +241,6 @@ var testProfile1 = &profile.Profile{
Mapping: testM,
}

var testProfile2 = &profile.Profile{
PeriodType: &profile.ValueType{Type: "cpu", Unit: "millisecond"},
Period: 10,
DurationNanos: 10e9,
SampleType: []*profile.ValueType{
{Type: "samples", Unit: "count"},
{Type: "cpu", Unit: "cycles"},
},
Sample: []*profile.Sample{
{
Location: []*profile.Location{testL[0]},
Value: []int64{1, 1},
},
{
Location: []*profile.Location{testL[2], testL[1], testL[0]},
Value: []int64{1, 10},
},
{
Location: []*profile.Location{testL[4], testL[2], testL[0]},
Value: []int64{1, -100},
},
{
Location: []*profile.Location{testL[3], testL[0]},
Value: []int64{1, 1000},
},
{
Location: []*profile.Location{testL[4], testL[3], testL[0]},
Value: []int64{1, 10000},
},
},
Location: testL,
Function: testF,
Mapping: testM,
}

func TestDisambiguation(t *testing.T) {
parent1 := &graph.Node{Info: graph.NodeInfo{Name: "parent1"}}
parent2 := &graph.Node{Info: graph.NodeInfo{Name: "parent2"}}
Expand Down Expand Up @@ -356,7 +324,7 @@ func TestLegendActiveFilters(t *testing.T) {
}

func TestComputeTotal(t *testing.T) {
p1 := testProfile1.Copy()
p1 := testProfile.Copy()
p1.Sample = []*profile.Sample{
{
Location: []*profile.Location{testL[0]},
Expand All @@ -372,7 +340,7 @@ func TestComputeTotal(t *testing.T) {
},
}

p2 := testProfile1.Copy()
p2 := testProfile.Copy()
p2.Sample = []*profile.Sample{
{
Location: []*profile.Location{testL[0]},
Expand All @@ -388,7 +356,7 @@ func TestComputeTotal(t *testing.T) {
},
}

p3 := testProfile1.Copy()
p3 := testProfile.Copy()
p3.Sample = []*profile.Sample{
{
Location: []*profile.Location{testL[0]},
Expand Down Expand Up @@ -473,7 +441,7 @@ func TestComputeTotal(t *testing.T) {
t.Errorf("got total %d, want %v", gotTotal, tc.wantTotal)
}
if gotUnexpNegSamples != tc.wantUnexpNegSamples {
t.Errorf("got unexpected negative samples %v, want %v", gotUnexpNegSamples, tc.wantUnexpNegSamples)
t.Errorf("got unexpected negative samples %v, want %v", gotUnexpNegSamples, tc.wantUnexpNegSamples)
}
})
}
Expand Down
17 changes: 0 additions & 17 deletions internal/report/testdata/source_neg_samples.dot
View file

This file was deleted.

34 changes: 34 additions & 0 deletions internal/report/testdata/source_neg_samples.rpt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
Total: 11
ROUTINE ======================== bar in testdata/source1
-10 -10 (flat, cum) 90.91% of Total
. . 5:source1 line 5;
. . 6:source1 line 6;
. . 7:source1 line 7;
. . 8:source1 line 8;
. . 9:source1 line 9;
-10 -10 10:source1 line 10;
. . 11:source1 line 11;
. . 12:source1 line 12;
. . 13:source1 line 13;
. . 14:source1 line 14;
. . 15:source1 line 15;
ROUTINE ======================== foo in testdata/source1
0 -10 (flat, cum) 90.91% of Total
. . 1:source1 line 1;
. . 2:source1 line 2;
. . 3:source1 line 3;
. -10 4:source1 line 4;
. . 5:source1 line 5;
. . 6:source1 line 6;
. . 7:source1 line 7;
. . 8:source1 line 8;
. . 9:source1 line 9;
ROUTINE ======================== main in testdata/source1
-1 -11 (flat, cum) 100% of Total
. . 1:source1 line 1;
-1 -11 2:source1 line 2;
. . 3:source1 line 3;
. . 4:source1 line 4;
. . 5:source1 line 5;
. . 6:source1 line 6;
. . 7:source1 line 7;

0 comments on commit 68e5354

Please sign in to comment.