Skip to content

Commit 118c5da

Browse files
committed
Use better approach to signal remote process' exit status
1. `remote_info.failed_process` stores symbol but we only use the value's presence to check if the process is force-killed. 2. The force-killed status is directly controlled by `kill_safely` through `kill_remote_debuggee`, which is directly called right before we check the status with `remote_info.failed_process`. Combining the two, we can just let `kill_safely` and `kill_remote_debuggee` to return the force-killed status and not storing it in `remote_info`, which already contains a bunch of information. This also eliminates the need to pass `test_info` to `kill_safely`, which makes related code easier to understand and maintain.
1 parent 6d0c267 commit 118c5da

File tree

3 files changed

+11
-12
lines changed

3 files changed

+11
-12
lines changed

test/support/console_test_case.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ def run_test_scenario cmd, test_info
200200
# kill debug console process
201201
read.close
202202
write.close
203-
kill_safely pid, :debugger, test_info
203+
kill_safely pid
204204
end
205205
end
206206
end

test/support/protocol_test_case.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,8 +338,7 @@ def execute_dap_scenario scenario
338338
is_assertion_failure = true
339339
raise e
340340
ensure
341-
kill_remote_debuggee test_info
342-
if test_info.failed_process && !is_assertion_failure
341+
if kill_remote_debuggee(test_info) && !is_assertion_failure
343342
flunk create_protocol_message "Expected the debuggee program to finish"
344343
end
345344
# Because the debuggee may be terminated by executing the following operations, we need to run them after `kill_remote_debuggee` method.
@@ -372,8 +371,7 @@ def execute_cdp_scenario_ scenario
372371
is_assertion_failure = true
373372
raise e
374373
ensure
375-
kill_remote_debuggee test_info
376-
if test_info.failed_process && !is_assertion_failure
374+
if kill_remote_debuggee(test_info) && !is_assertion_failure
377375
flunk create_protocol_message "Expected the debuggee program to finish"
378376
end
379377
# Because the debuggee may be terminated by executing the following operations, we need to run them after `kill_remote_debuggee` method.

test/support/test_case.rb

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,17 +122,17 @@ def wait_pid pid, sec
122122
true
123123
end
124124

125-
def kill_safely pid, name, test_info
126-
return if wait_pid pid, TIMEOUT_SEC
127-
128-
test_info.failed_process = name
125+
def kill_safely pid
126+
return false if wait_pid pid, TIMEOUT_SEC
129127

130128
Process.kill :TERM, pid
131-
return if wait_pid pid, 0.2
129+
return true if wait_pid pid, 0.2
132130

133131
Process.kill :KILL, pid
134132
Process.waitpid(pid)
133+
true
135134
rescue Errno::EPERM, Errno::ESRCH
135+
true
136136
end
137137

138138
def check_error(error, test_info)
@@ -142,13 +142,14 @@ def check_error(error, test_info)
142142
end
143143

144144
def kill_remote_debuggee test_info
145-
return unless r = test_info.remote_info
145+
return false unless r = test_info.remote_info
146146

147-
kill_safely r.pid, :remote, test_info
147+
force_killed = kill_safely r.pid
148148
r.reader_thread.kill
149149
# Because the debuggee may be terminated by executing the following operations, we need to run them after `kill_safely` method.
150150
r.r.close
151151
r.w.close
152+
force_killed
152153
end
153154

154155
def setup_remote_debuggee(cmd)

0 commit comments

Comments
 (0)