Skip to content

Commit 8f7ed05

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 64552e2 commit 8f7ed05

File tree

3 files changed

+13
-11
lines changed

3 files changed

+13
-11
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: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,18 @@ 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
125+
def kill_safely pid
126+
needs_kill = false
127+
return needs_kill if wait_pid pid, TIMEOUT_SEC
127128

128-
test_info.failed_process = name
129+
needs_kill = true
129130

130131
Process.kill :TERM, pid
131-
return if wait_pid pid, 0.2
132+
return needs_kill if wait_pid pid, 0.2
132133

133134
Process.kill :KILL, pid
134135
Process.waitpid(pid)
136+
needs_kill
135137
rescue Errno::EPERM, Errno::ESRCH
136138
end
137139

@@ -142,13 +144,15 @@ def check_error(error, test_info)
142144
end
143145

144146
def kill_remote_debuggee test_info
145-
return unless r = test_info.remote_info
147+
needs_kill = false
148+
return needs_kill unless r = test_info.remote_info
146149

147-
kill_safely r.pid, :remote, test_info
150+
needs_kill = kill_safely r.pid
148151
r.reader_thread.kill
149152
# Because the debuggee may be terminated by executing the following operations, we need to run them after `kill_safely` method.
150153
r.r.close
151154
r.w.close
155+
needs_kill
152156
end
153157

154158
def setup_remote_debuggee(cmd)

0 commit comments

Comments
 (0)