Uh oh!
There was an error while loading. Please reload this page.
Improve test process' kill signalling - #925
Conversation
The reason for storing the symbol is to use in console tests such as debug/test/support/console_test_case.rb Line 210 in c3bfcc2 But it's ok to check each killing method(kill_remote_debuggee and kill_safely). Do you think that it'll be better? |
st0012
commented
Mar 20, 2023
Yeah definitely. Since we always check the state right after calling kill methods, using return value will be more readable and make refactoring easier. |
60f787b to
971d8c0Compareono-max
commented
Mar 20, 2023
Yes, that's what I said here: We can write fixed error messages by checking the state in all killing methods. |
ono-max
commented
Mar 20, 2023
All right, Let's go with that idea. |
ono-max
commented
Mar 20, 2023
I'll review the changes tomorrow. Thank you for the explanation. |
st0012
commented
Mar 20, 2023
Sorry I misunderstood 😅
Thanks! |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ono-max
commented
Mar 21, 2023
How about renaming |
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.
971d8c0 to
64732f6Comparest0012
commented
Mar 22, 2023
Yeah |
remote_info.failed_processstores symbol but we only use the value's presence to check if the process is force-killed.kill_safelythroughkill_remote_debuggee, which is directly called right before we check the status withremote_info.failed_process.Combining the two, we can just let
kill_safelyandkill_remote_debuggeereturn the force-killed status and not storing it inremote_info, which already contains a bunch of information.This also eliminates the need to pass
test_infotokill_safely, which makes related code easier to understand and maintain.