Skip to content

Commit

Permalink
Switch to use goroutine profile in TestHttpsInsecure. (#350)
Browse files Browse the repository at this point in the history
Trying to test against /debug/pprof/profile has been giving some flakes
and trouble so far (#146, #253, #328, golang/go#24611, golang/go#22594). While
some of the discussions the failures triggered appear to be useful (such
as whether CPU profiles are expected to be working on Windows XP or
not), that kind of testing is not really in the scope for this
particular test. This change switches the test to use goroutine profile
instead which is hopefully much less dependent on the environment. The
change also makes the test much faster to run.
  • Loading branch information
aalexand authored and nolanmar511 committed Apr 2, 2018
1 parent 6084089 commit 36d5638
Showing 1 changed file with 2 additions and 31 deletions.
33 changes: 2 additions & 31 deletions internal/driver/fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,26 +399,14 @@ func TestHttpsInsecure(t *testing.T) {
}()
defer l.Close()

go func() {
deadline := time.Now().Add(5 * time.Second)
for time.Now().Before(deadline) {
// Simulate a hotspot function. Spin in the inner loop for 100M iterations
// to ensure we get most of the samples landed here rather than in the
// library calls. We assume Go compiler won't elide the empty loop.
for i := 0; i < 1e8; i++ {
}
runtime.Gosched()
}
}()

outputTempFile, err := ioutil.TempFile("", "profile_output")
if err != nil {
t.Fatalf("Failed to create tempfile: %v", err)
}
defer os.Remove(outputTempFile.Name())
defer outputTempFile.Close()

address := "https+insecure://" + l.Addr().String() + "/debug/pprof/profile"
address := "https+insecure://" + l.Addr().String() + "/debug/pprof/goroutine"
s := &source{
Sources: []string{address},
Seconds: 10,
Expand All @@ -437,31 +425,14 @@ func TestHttpsInsecure(t *testing.T) {
if len(p.SampleType) == 0 {
t.Fatalf("fetchProfiles(%s) got empty profile: len(p.SampleType)==0", address)
}
switch runtime.GOOS {
case "plan9":
// CPU profiling is not supported on Plan9; see golang.org/issues/22564.
return
case "darwin":
if runtime.GOARCH == "arm" || runtime.GOARCH == "arm64" {
// CPU profiling on iOS os not symbolized; see golang.org/issues/22612.
return
}
}
if len(p.Function) == 0 {
t.Fatalf("fetchProfiles(%s) got non-symbolized profile: len(p.Function)==0", address)
}
if err := checkProfileHasFunction(p, "TestHttpsInsecure"); !badSigprofOS[runtime.GOOS] && err != nil {
if err := checkProfileHasFunction(p, "TestHttpsInsecure"); err != nil {
t.Fatalf("fetchProfiles(%s) %v", address, err)
}
}

// Some operating systems don't trigger the profiling signal right.
// See https://github.com/golang/go/issues/13841.
var badSigprofOS = map[string]bool{
"darwin": true,
"netbsd": true,
}

func checkProfileHasFunction(p *profile.Profile, fname string) error {
for _, f := range p.Function {
if strings.Contains(f.Name, fname) {
Expand Down

0 comments on commit 36d5638

Please sign in to comment.