From bbc5416037094c44806e7b4c50bd02b74c99098e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 6 Jun 2019 16:22:18 -0300 Subject: [PATCH] Use "nodes * 4" as default for --max-worker-restart Fix #226 --- changelog/226.feature.rst | 2 ++ src/xdist/dsession.py | 25 +++++++++++++++---------- testing/test_dsession.py | 20 +++++++++++++++++++- 3 files changed, 36 insertions(+), 11 deletions(-) create mode 100644 changelog/226.feature.rst diff --git a/changelog/226.feature.rst b/changelog/226.feature.rst new file mode 100644 index 0000000..c4cb23a --- /dev/null +++ b/changelog/226.feature.rst @@ -0,0 +1,2 @@ +``--max-worker-restart`` now assumes a more reasonable value (4 times the number of +nodes) when not given explicitly. This prevents test suites from running forever when the suite crashes during collection. diff --git a/src/xdist/dsession.py b/src/xdist/dsession.py index 8bb559c..81a03a6 100644 --- a/src/xdist/dsession.py +++ b/src/xdist/dsession.py @@ -46,9 +46,8 @@ class DSession(object): self._failed_collection_errors = {} self._active_nodes = set() self._failed_nodes_count = 0 - self._max_worker_restart = self.config.option.maxworkerrestart - if self._max_worker_restart is not None: - self._max_worker_restart = int(self._max_worker_restart) + self._max_worker_restart = get_default_max_worker_restart(self.config) + try: self.terminal = config.pluginmanager.getplugin("terminalreporter") except KeyError: @@ -390,10 +389,16 @@ class TerminalDistReporter(object): return self.write_line("[%s] node down: %s" % (node.gateway.id, error)) - # def pytest_xdist_rsyncstart(self, source, gateways): - # targets = ",".join([gw.id for gw in gateways]) - # msg = "[%s] rsyncing: %s" %(targets, source) - # self.write_line(msg) - # def pytest_xdist_rsyncfinish(self, source, gateways): - # targets = ", ".join(["[%s]" % gw.id for gw in gateways]) - # self.write_line("rsyncfinish: %s -> %s" %(source, targets)) + +def get_default_max_worker_restart(config): + """gets the default value of --max-worker-restart option if it is not provided. + + Use a reasonable default to avoid workers from restarting endlessly due to crashing collections (#226). + """ + result = config.option.maxworkerrestart + if result is not None: + result = int(result) + elif config.option.numprocesses: + # if --max-worker-restart was not provided, use a reasonable default (#226) + result = config.option.numprocesses * 4 + return result diff --git a/testing/test_dsession.py b/testing/test_dsession.py index a75866f..8d0373e 100644 --- a/testing/test_dsession.py +++ b/testing/test_dsession.py @@ -1,4 +1,4 @@ -from xdist.dsession import DSession +from xdist.dsession import DSession, get_default_max_worker_restart from xdist.report import report_collection_diff from xdist.scheduler import EachScheduling, LoadScheduling @@ -268,6 +268,24 @@ def test_report_collection_diff_equal(): assert report_collection_diff(from_collection, to_collection, 1, 2) is None +def test_default_max_worker_restart(): + class config: + class option: + maxworkerrestart = None + numprocesses = 0 + + assert get_default_max_worker_restart(config) is None + + config.option.numprocesses = 2 + assert get_default_max_worker_restart(config) == 8 + + config.option.maxworkerrestart = "1" + assert get_default_max_worker_restart(config) == 1 + + config.option.maxworkerrestart = "0" + assert get_default_max_worker_restart(config) == 0 + + def test_report_collection_diff_different(): """Test reporting of different collections.""" from_collection = ["aaa", "bbb", "ccc", "YYY"]