Terminate proccess's when experiencing a fatal error in ductape runner #323
Conversation
|
|
|
tested by running |
| self.service = SimpleEchoService(self.test_context) | ||
|
|
||
| @cluster(num_nodes=1) | ||
| def timeout_test(self): |
There was a problem hiding this comment.
I would probably want to test with multiple tests in flight - some scheduled in parallel, some still yet to schedule (maybe just run for all systests?)
There was a problem hiding this comment.
well i run in parallel for the unit test as well, but yes ran in systest with some tests yet to schedule etc.
There was a problem hiding this comment.
in #323 you're saying that you've tested with test_runner_operations, which is a single test method - maybe worth testing with simply systests folder?
There was a problem hiding this comment.
yeah for sure ill give it a run, ran it with repeat to simulate a bunch of test being run but yeah lets get the complete coverage.
|
Also, this PR cannot be built, please don't merge without fixing that 😄 |
| @@ -210,6 +210,9 @@ def run_all_tests(self): | |||
| self._log(logging.ERROR, err_str) | |||
|
|
|||
| # All processes are on the same machine, so treat communication failure as a fatal error | |||
| for proc in self._client_procs.values(): | |||
| proc.terminate() | |||
There was a problem hiding this comment.
Also compare this to https://github.com/confluentinc/ducktape/blob/master/ducktape/tests/runner.py#L124 which uses os.kill vs terminate() - what's the difference and pros/cons?
There was a problem hiding this comment.
would probably be good to have a unified cleanup_child_processes method or smth
There was a problem hiding this comment.
if you read the docs for terminate:
Terminate the process. On Unix this is done using the SIGTERM signal; on Windows TerminateProcess()
which seems to be more platform agnostic
There was a problem hiding this comment.
What about modifying that other line too then? Can be a separate PR though.
There was a problem hiding this comment.
Yeah might be best to touch it in another pr with more testing.
confluentinc#323) * update test runner * update docstring * readd newline * add a simple test to run against * fix formating (cherry picked from commit a214102)
confluentinc#323) * update test runner * update docstring * readd newline * add a simple test to run against * fix formating
closes #322
Simply terminates process when an exception is caught in the run cycle