5 minute timeout for tests

[chaoren]

Summary

Some of the test will hang when run. This lets the hanging tests die, and lets you get on with your life.

[emaste]

test/dosep.py

15

The bots run under an overall timeout, but it's much more valuable to catch single tests that hang, so I think this is a good idea. We'll need to check that timeout is available first though. timeout is non-standard - it's on FreeBSD & Linux, but not OS X or I suspect other operating systems.

[zturner]

test/dosep.py

15

Certainly won't work on Windows. We should find another way to do this.

[zturner]

Maybe this is a good starting point:

http://stackoverflow.com/questions/1191374/subprocess-with-timeout

[chaoren]

Only enable for linux for now, since the problem doesn't affect OS X and Windows (yet). Also marks which tests timed out.

[zturner]

test/dosep.py

54

This appears to be equivalent to the previous code, but using subprocess instead of os. If this is the only reason in the entire file for importing subprocess, then can we change it back to os.system to avoid importing the extra module? Or alternatively, if the previous call to os.system was the only reason in the entire file for importing os, then keep the subprocess.call but remove the import of os.system.

Other than that, LGTM

[chaoren]

os.system didn't return the correct exit code (124). And I believe
subprocess.call is intended to replace os.system anyway according to this
https://docs.python.org/2/library/subprocess.html#module-subprocess.

[chaoren]

There are other uses of the os module, so it can't be removed either.

[zturner]

Fair enough, no need to change anything then.

Weird though that os.system doesn't return the same value, I always thought
os.system(x) was equivalent to subprocess.call(x, shell=True).

Anyway, LGTM

[emaste]

test/dosep.py

34

please enable for FreeBSD too

[chaoren]

Enabling for FreeBSD as well.

[chaoren]

Is GNU coreutils already a requirement on FreeBSD or OS X? Because I don't
think timeout exists on FreeBSD, but gtimeout does. Same goes for OS X.

[emaste]

Is GNU coreutils already a requirement on FreeBSD or OS X? Because I don't
think timeout exists on FreeBSD, but gtimeout does. Same goes for OS X.

Ah, a mistake on my part. We have a BSD licensed timeout implementation in FreeBSD-current, but it is not yet in the stable/10 branch or in the 10.1 release.

[chaoren]

I think it'll be better if we just try timeout, then gtimeout, and default
to the original if neither exists. Do you know if there are any targets
other than Windows with a timeout command that does something completely
different http://ss64.com/nt/timeout.html?

[chaoren]

Timeout if command exists.

[zturner]

I guess there's a possibility a test on Windows could return 124 and mean
something completely different than timeout. Also 124 is a magic number
that people won't understand unless they check the man page for timeout and
gtimeout.

Can you move the return code checking into the try_timeout function, and
then return an enum like eTimedOut, eFailed, eSucceeded?

Other than that, looks good.

[chaoren]

Move timeout magic number into try_timeout. Windows shouldn't have to deal with it anymore.

[zturner]

looks good.

test/dosep.py

 import multiprocessing
 import os
 import platform
+import shlex
+import subprocess
 import sys
 
 from optparse import OptionParser
 
-# Command template of the invocation of the test driver.
-template = '%s %s/dotest.py %s -p %s %s'
+def try_timeout(command):
+    if not sys.platform.startswith("win32"):
+        try:
+            return {0: "passed", 124: "timed out"}.get(
+                subprocess.call(["timeout", "5m"] + command), "failed")
+        except OSError:
+            pass
+        try:
+            return {0: "passed", 124: "timed out"}.get(
+                subprocess.call(["gtimeout", "5m"] + command), "failed")
+        except OSError:
+            pass
+    return "passed" if subprocess.call(command) == 0 else "failed"
 
 def process_dir(root, files, test_root, dotest_options):
     """Examine a directory for tests, and invoke any found within it."""
+    timed_out = []
     failed = []
     passed = []
     for name in files:

         if os.path.islink(path):
             continue
 
-        command = template % (sys.executable, test_root, dotest_options if dotest_options else "", name, root)
-        if 0 != os.system(command):
+        command = ([sys.executable, "%s/dotest.py" % test_root] +
+                   (shlex.split(dotest_options) if dotest_options else []) +
+                   ["-p", name, root])
+        exit_status = try_timeout(command)
+        if "passed" != exit_status:
+            if "timed out" == exit_status:
+                timed_out.append(name)
             failed.append(name)
         else:
             passed.append(name)
-    return (failed, passed)
+    return (timed_out, failed, passed)
 
 in_q = None
 out_q = None

         for work_item in test_work_items:
             test_results.append(process_dir_worker(work_item))
 
+    timed_out = []
     failed = []
     passed = []
 
     for test_result in test_results:
-        (dir_failed, dir_passed) = test_result
+        (dir_timed_out, dir_failed, dir_passed) = test_result
+        timed_out += dir_timed_out
         failed += dir_failed
         passed += dir_passed
 
-    return (failed, passed)
+    return (timed_out, failed, passed)
 
 def main():
     test_root = sys.path[0]

         num_threads = 1
 
     system_info = " ".join(platform.uname())
-    (failed, passed) = walk_and_invoke(test_root, dotest_options, num_threads)
+    (timed_out, failed, passed) = walk_and_invoke(test_root, dotest_options,
+                                                  num_threads)
     num_tests = len(failed) + len(passed)
 
     print "Ran %d tests." % num_tests

         failed.sort()
         print "Failing Tests (%d)" % len(failed)
         for f in failed:
-          print "FAIL: LLDB (suite) :: %s (%s)" % (f, system_info)
+            print "%s: LLDB (suite) :: %s (%s)" % (
+                "TIMEOUT" if f in timed_out else "FAIL", f, system_info
+            )
         sys.exit(1)
     sys.exit(0)