-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
dlv_test: fix TestChildProcessExitWhenNoDebugInfo to test stripped binary #2949
dlv_test: fix TestChildProcessExitWhenNoDebugInfo to test stripped binary #2949
Conversation
// Test only for dlv's prefix of the error like "could not launch process: could not open debug info" | ||
if !strings.Contains(string(out), "could not launch process") { | ||
t.Fatalf("Expected logged error 'could not launch process: ...'") | ||
} | ||
|
||
// search the running process named fix.Name | ||
cmd := exec.Command("ps", "-aux") |
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.
Is this part designed to test that a process won't leak after the debug command exits?
Technically, when process leaks, the debugger leaks with it and the test will times out.
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.
I don't remember, I think it didn't hang when the process leaks on linux.
// Test only for dlv's prefix of the error like "could not launch process: could not open debug info" | ||
if !strings.Contains(string(out), "could not launch process") { | ||
t.Fatalf("Expected logged error 'could not launch process: ...'") | ||
} | ||
|
||
// search the running process named fix.Name | ||
cmd := exec.Command("ps", "-aux") |
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.
I don't remember, I think it didn't hang when the process leaks on linux.
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.
LGTM
Re-running CI real quick. |
Following up on discussion here: #2018 (comment)
The test was bailing out early before actually trying to debug a stripped binary at this check:
delve/cmd/dlv/cmds/commands.go
Lines 891 to 902 in 2b97231
Modified the flags and added logging and more checks to make sure the session exits for the right reasons.