Merge pull request #42 from rauls5382/symbols

Ensure symbolz is called after local symbolization

internal/symbolizer/symbolizer.go

@@ -38,6 +38,10 @@ type Symbolizer struct {
 	UI  plugin.UI
 }
 
+// test taps for dependency injection
+var symbolzSymbolize = symbolz.Symbolize
+var localSymbolize = doLocalSymbolize
+
 // Symbolize attempts to symbolize profile p. First uses binutils on
 // local binaries; if the source is a URL it attempts to get any
 // missed entries using symbolz.
@@ -70,12 +74,12 @@ func (s *Symbolizer) Symbolize(mode string, sources plugin.MappingSources, p *pr
 	var err error
 	if local {
 		// Symbolize locally using binutils.
-		if err = localSymbolize(mode, p, s.Obj, s.UI); err == nil {
-			remote = false // Already symbolized, no need to apply remote symbolization.
+		if err = localSymbolize(mode, p, s.Obj, s.UI); err != nil {
+			s.UI.PrintErr("local symbolization: " + err.Error())
 		}
 	}
 	if remote {
-		if err = symbolz.Symbolize(sources, postURL, p, s.UI); err != nil {
+		if err = symbolzSymbolize(sources, postURL, p, s.UI); err != nil {
 			return err // Ran out of options.
 		}
 	}
@@ -97,10 +101,10 @@ func postURL(source, post string) ([]byte, error) {
 	return ioutil.ReadAll(resp.Body)
 }
 
-// localSymbolize adds symbol and line number information to all locations
+// doLocalSymbolize adds symbol and line number information to all locations
 // in a profile. mode enables some options to control
 // symbolization.
-func localSymbolize(mode string, prof *profile.Profile, obj plugin.ObjTool, ui plugin.UI) error {
+func doLocalSymbolize(mode string, prof *profile.Profile, obj plugin.ObjTool, ui plugin.UI) error {
 	force := false
 	// Disable some mechanisms based on mode string.
 	for _, o := range strings.Split(strings.ToLower(mode), ":") {

internal/symbolizer/symbolizer_test.go

@@ -17,6 +17,7 @@ package symbolizer
 import (
 	"fmt"
 	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/google/pprof/internal/plugin"
@@ -95,7 +96,66 @@ var testProfile = profile.Profile{
 }
 
 func TestSymbolization(t *testing.T) {
-	prof := testProfile
+	sSym := symbolzSymbolize
+	lSym := localSymbolize
+	defer func() {
+		symbolzSymbolize = sSym
+		localSymbolize = lSym
+	}()
+	symbolzSymbolize = symbolzMock
+	localSymbolize = localMock
+
+	type testcase struct {
+		mode        string
+		wantComment string
+	}
+
+	s := Symbolizer{
+		mockObjTool{},
+		&proftest.TestUI{T: t},
+	}
+	for i, tc := range []testcase{
+		{
+			"local",
+			"local=local",
+		},
+		{
+			"fastlocal",
+			"local=fastlocal",
+		},
+		{
+			"remote",
+			"symbolz",
+		},
+		{
+			"",
+			"local=:symbolz",
+		},
+	} {
+		prof := testProfile.Copy()
+		if err := s.Symbolize(tc.mode, nil, prof); err != nil {
+			t.Errorf("symbolize #%d: %v", i, err)
+			continue
+		}
+		if got, want := strings.Join(prof.Comments, ":"), tc.wantComment; got != want {
+			t.Errorf("got %s, want %s", got, want)
+			continue
+		}
+	}
+}
+
+func symbolzMock(sources plugin.MappingSources, syms func(string, string) ([]byte, error), p *profile.Profile, ui plugin.UI) error {
+	p.Comments = append(p.Comments, "symbolz")
+	return nil
+}
+
+func localMock(mode string, p *profile.Profile, obj plugin.ObjTool, ui plugin.UI) error {
+	p.Comments = append(p.Comments, "local="+mode)
+	return nil
+}
+
+func TestLocalSymbolization(t *testing.T) {
+	prof := testProfile.Copy()
 
 	if prof.HasFunctions() {
 		t.Error("unexpected function names")
@@ -105,8 +165,8 @@ func TestSymbolization(t *testing.T) {
 	}
 
 	b := mockObjTool{}
-	if err := localSymbolize("", &prof, b, &proftest.TestUI{T: t}); err != nil {
-		t.Fatalf("Symbolize(): %v", err)
+	if err := localSymbolize("", prof, b, &proftest.TestUI{T: t}); err != nil {
+		t.Fatalf("localSymbolize(): %v", err)
 	}
 
 	for _, loc := range prof.Location {