From 75eb1f8bca573041c9cbe8625df78556ac828d0b Mon Sep 17 00:00:00 2001 From: Alexey Alexandrov Date: Sat, 31 Mar 2018 11:28:37 -0700 Subject: [PATCH] Switch to use goroutine profile in TestHttpsInsecure. 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. --- internal/driver/fetch_test.go | 33 ++------------------------------- 1 file changed, 2 insertions(+), 31 deletions(-) diff --git a/internal/driver/fetch_test.go b/internal/driver/fetch_test.go index 0e6c9306..f15328bf 100644 --- a/internal/driver/fetch_test.go +++ b/internal/driver/fetch_test.go @@ -399,18 +399,6 @@ 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) @@ -418,7 +406,7 @@ func TestHttpsInsecure(t *testing.T) { 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, @@ -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) {