Skip to content

WIP: Kill a debuggee process fastly if a test fails #933

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions test/support/console_test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,20 @@ def run_test_scenario cmd, test_info
# result of `gets` return this exception in some platform
rescue Timeout::Error
assert_block(create_message("TIMEOUT ERROR (#{TIMEOUT_SEC} sec)", test_info)) { false }
rescue Test::Unit::AssertionFailedError
is_fail = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we align the name with what we have in protocol test case (is_assertion_failure)?

raise
ensure
kill_remote_debuggee test_info
kill_remote_debuggee(test_info, failed: is_fail == true)
# kill debug console process
read.close
write.close
kill_safely pid, :debugger, test_info
if is_fail == true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The == true seems to be redundant?

kill_safely pid, :debugger, test_info, sec: 0
end
if name = test_info.failed_process && !is_fail
flunk(create_message("Expected the #{name} program to finish", test_info))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we only enter this case when is_fail is false, wouldn't name always be remote 🤔

end
end
end
end
Expand Down
12 changes: 8 additions & 4 deletions test/support/test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ def wait_pid pid, sec
true
end

def kill_safely pid, name, test_info
return if wait_pid pid, TIMEOUT_SEC
def kill_safely pid, name, test_info, sec: TIMEOUT_SEC
return if wait_pid pid, sec

test_info.failed_process = name

Expand All @@ -141,10 +141,14 @@ def check_error(error, test_info)
end
end

def kill_remote_debuggee test_info
def kill_remote_debuggee test_info, failed: false
return unless r = test_info.remote_info

kill_safely r.pid, :remote, test_info
if failed
kill_safely r.pid, :remote, test_info, sec: 2
else
kill_safely r.pid, :remote, test_info
end
r.reader_thread.kill
# Because the debuggee may be terminated by executing the following operations, we need to run them after `kill_safely` method.
r.r.close
Expand Down