jenkins-bot merged this change.

View Change

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(-)

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.
 

To view, visit change 524641. To unsubscribe, or for help writing mail filters, visit settings.