-
Notifications
You must be signed in to change notification settings - Fork 596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create a fake mapping for profile.proto profiles #135
Conversation
If a profile has mappings but no profiles, pprof may be unable to symbolize it offline, as it uses the mappings to keep track of which locations need symbolization. This fixes google#120. Added the test, verified it fails on Mac with Go 1.7 before the fix, and passes with the fix. The test is done by augmenting the existing test for handling https+insecure:// schema in URLs. This is a bit vague but I figured that this test needed an updated anyway since as we moved it recently we stopped exercising the symbolization as part of the test which was its original intention in fixing google#94. Can split the tests if things do look too ugly.
internal/proftest/proftest.go
Outdated
if ui.IgnoreRx != "" { | ||
if matched, err := regexp.MatchString(ui.IgnoreRx, fmt.Sprint(args)); matched || err != nil { | ||
if err != nil { | ||
ui.T.Error(args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably also include the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, fixed.
Codecov Report
@@ Coverage Diff @@
## master #135 +/- ##
=========================================
+ Coverage 54.59% 54.9% +0.31%
=========================================
Files 29 29
Lines 5697 5697
=========================================
+ Hits 3110 3128 +18
+ Misses 2251 2230 -21
- Partials 336 339 +3
Continue to review full report at Codecov.
|
53204fa
to
1c8f1ab
Compare
This reverts commit 1047541.
This reverts commit 1047541.
* Create a fake mapping for profile.proto profiles If a profile has mappings but no profiles, pprof may be unable to symbolize it offline, as it uses the mappings to keep track of which locations need symbolization. This fixes google#120. Added the test, verified it fails on Mac with Go 1.7 before the fix, and passes with the fix. The test is done by augmenting the existing test for handling https+insecure:// schema in URLs. This is a bit vague but I figured that this test needed an updated anyway since as we moved it recently we stopped exercising the symbolization as part of the test which was its original intention in fixing google#94. Can split the tests if things do look too ugly. * Fix the test to include the failed regex matching error in the message.
If a profile has mappings but no profiles, pprof may be unable to
symbolize it offline, as it uses the mappings to keep track of which
locations need symbolization.
This fixes #120.
Added the test, verified it fails on Mac with Go 1.7 before the fix, and
passes with the fix. The test is done by augmenting the existing test
for handling https+insecure:// scheme in URLs. This is a bit vague but I
figured that this test needed an updated anyway since as we moved it
recently we stopped exercising the symbolization as part of the test
which was its original intention in fixing #94. Can split the tests if
things do look too ugly.