[PATCH] Add per-test timeouts to lit

[Daniel Dunbar]

Looks good, some nits inline. I would like to see a test or two before this goes in, though.

================
Comment at: utils/lit/lit/util.py:11
@@ -9,1 +10,3 @@
 
+import Test
+
----------------
This import isn't needed anymore.

================
Comment at: utils/lit/lit/util.py:156
@@ +155,3 @@
+
+    class Watchdog(object):
+        def __init__(self, popen, timeout):
----------------
jroelofs wrote:
> thanks for the suggestion... I like this a lot more than what I had before.
Can this be lifted to the top-level of this file just to avoid the break in the code?

================
Comment at: utils/lit/lit/util.py:160
@@ +159,3 @@
+            self.timer = None
+            if timeout is not None:
+                def timeout_handler():
----------------
I would prefer if this class assumed timeout was non-None and the caller assumed the responsibility of handling the optionality of timeout (and just didn't create an instance)...

With this change the watchdog implementation gets a tad simpler.

================
Comment at: utils/lit/lit/util.py:161
@@ +160,3 @@
+            if timeout is not None:
+                def timeout_handler():
+                    try:
----------------
This might be more readable as a regular method on the class instead of a nested function.

================
Comment at: utils/lit/lit/util.py:166
@@ +165,3 @@
+                        # The Popen already terminated before the watchdog
+                        # got acqauinted with it. Too bad they couldn't be
+                        # friends
----------------
Sic.

================
Comment at: utils/lit/lit/util.py:171
@@ +170,3 @@
+                timer = threading.Timer(timeout, timeout_handler)
+                timer.start()
+
----------------
This is currently broken, this should be self.timer, but with the change above there should only be a single assignment anyway.

================
Comment at: utils/lit/lit/util.py:178
@@ +177,3 @@
+        def timed_out(self):
+            return self.timeout_tripped
+
----------------
There isn't a good reason to use a getter method here, clients can just use the property directly.

================
Comment at: utils/lit/lit/util.py:186
@@ +185,3 @@
+    if wd.timed_out():
+      err += 'Test timed out after %d seconds\n' % (timeout)
+    wd.cancel()
----------------
I'd add a newline (or two) at the start of this string, as there isn't much guarantee the existing stderr data has one.

http://reviews.llvm.org/D6584