jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/524641 )
Change subject: test(utils.py): redesign the timeout workflow ......................................................................
test(utils.py): redesign the timeout workflow
The implentation of timeout is complex and confusingly just returns empty strings if a timeout occurs, causing T224364. Use a new implementation for Python 2 which raises TimeoutError and use the default one in Python 3.
In this design, infinite timeouts are indicated by None, not 0. Change the script_tests accordingly.
Increase the timeout of test_one_similar_script to 10 seconds. It occasionally failed for me with the previous 6s timeout.
Bug: T224364 Change-Id: Icc69d3df8ca41b86061f40fcaef0da7fa46d494e --- M tests/aspects.py M tests/pwb_tests.py M tests/script_tests.py M tests/utils.py 4 files changed, 22 insertions(+), 33 deletions(-)
Approvals: Xqt: Looks good to me, approved jenkins-bot: Verified
diff --git a/tests/aspects.py b/tests/aspects.py index fa93d87..328a279 100644 --- a/tests/aspects.py +++ b/tests/aspects.py @@ -1490,7 +1490,7 @@ if self.orig_pywikibot_dir: os.environ[str('PYWIKIBOT_DIR')] = self.orig_pywikibot_dir
- def _execute(self, args, data_in=None, timeout=0, error=None): + def _execute(self, args, data_in=None, timeout=None, error=None): site = self.get_site()
args = args + ['-family:' + site.family.name, diff --git a/tests/pwb_tests.py b/tests/pwb_tests.py index 54f0b33..cac124f 100644 --- a/tests/pwb_tests.py +++ b/tests/pwb_tests.py @@ -16,8 +16,6 @@ import io import sys
-from pywikibot.tools import PY2 - from tests import join_tests_path, create_path_func from tests.utils import execute, execute_pwb from tests.aspects import unittest, PwbTestCase @@ -91,7 +89,6 @@ self.assertEqual(stderr.readline().strip(), 'ERROR: pywikibot.py not found! Misspelling?')
- @unittest.skipIf(PY2, 'cannot be safely tested with Python 2 (T224364)') def test_one_similar_script(self): """Test shell.py script call which gives one similar result.""" result = [ @@ -99,7 +96,7 @@ 'NOTE: Starting the most similar script shell.py', 'in 5.0 seconds; type CTRL-C to stop.', ] - stream = execute_pwb(['hello'], data_in=chr(3), timeout=6) + stream = execute_pwb(['hello'], data_in=chr(3), timeout=10) stderr = io.StringIO(stream['stderr']) with self.subTest(line=0): self.assertEqual(stderr.readline().strip(), result[0]) diff --git a/tests/script_tests.py b/tests/script_tests.py index ef3f18c..d82c938 100644 --- a/tests/script_tests.py +++ b/tests/script_tests.py @@ -234,9 +234,10 @@
data_in = script_input.get(script_name)
- timeout = 0 if is_autorun: timeout = 5 + else: + timeout = None
if self._results and script_name in self._results: error = self._results[script_name] diff --git a/tests/utils.py b/tests/utils.py index 7fdaa1b..89f54e8 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -13,7 +13,6 @@ import re from subprocess import PIPE, Popen import sys -import time import traceback import warnings
@@ -21,6 +20,8 @@ from collections.abc import Mapping except ImportError: # Python 2.7 from collections import Mapping + from multiprocessing import TimeoutError + from threading import Timer from types import ModuleType
try: @@ -636,7 +637,7 @@ self._module.http = self._old_http
-def execute(command, data_in=None, timeout=0, error=None): +def execute(command, data_in=None, timeout=None, error=None): """ Execute a command and capture outputs.
@@ -699,35 +700,25 @@ p.stdin.write(data_in.encode(config.console_encoding)) p.stdin.flush() # _communicate() otherwise has a broken pipe
- stderr_lines = b'' - waited = 0 - while (error or (waited < timeout)) and p.poll() is None: - # In order to kill 'shell' and others early, read only a single - # line per second, and kill the process as soon as the expected - # output has been seen. - # Additional lines will be collected later with p.communicate() - if error: - line = p.stderr.readline() - stderr_lines += line - if error in line.decode(config.console_encoding): - break - time.sleep(1) - waited += 1 + if PY2: # subprocess.communicate does not support timeout + def timeout_handler(): + p.kill() + raise TimeoutError
- if (timeout or error) and p.poll() is None: - p.kill() - - if p.poll() is not None: - stderr_lines += p.stderr.read() - - data_out = p.communicate() + timer = Timer(timeout, timeout_handler) + timer.start() + try: + stdout_data, stderr_data = p.communicate() + finally: + timer.cancel() + else: + stdout_data, stderr_data = p.communicate(timeout=timeout) return {'exit_code': p.returncode, - 'stdout': data_out[0].decode(config.console_encoding), - 'stderr': (stderr_lines + data_out[1]) - .decode(config.console_encoding)} + 'stdout': stdout_data.decode(config.console_encoding), + 'stderr': stderr_data.decode(config.console_encoding)}
-def execute_pwb(args, data_in=None, timeout=0, error=None, overrides=None): +def execute_pwb(args, data_in=None, timeout=None, error=None, overrides=None): """ Execute the pwb.py script and capture outputs.