[PATCH] Add per-test timeouts to lit

Eric Fiselier eric at efcs.ca
Wed May 27 09:02:53 PDT 2015


Misc comments. More to come.


================
Comment at: utils/lit/lit/util.py:19
@@ +18,3 @@
+        self.timer = threading.Timer(timeout, self.handler)
+        self.timer.start()
+
----------------
Why not start the timer when `watch(...)` is called for the first time? It seems like some code takes steps between creating the Watchdog and the call to `watch(...)`. Those steps may unfairly count against the test timeout.

================
Comment at: utils/lit/lit/util.py:31
@@ +30,3 @@
+
+    def handler(self):
+        self.timed_out = True
----------------
What thread of execution does `Threading.timer` execute this function in? I think we would need to add a lock to `Watchdog` if the handler runs in a separate thread.

================
Comment at: utils/lit/lit/util.py:45
@@ +44,3 @@
+    def cancel(self):
+        if self.timer is not None:
+            self.timer.cancel()
----------------
How can `self.timer` be `None`?

================
Comment at: utils/lit/lit/util.py:200
@@ -160,3 +199,3 @@
 kUseCloseFDs = not (platform.system() == 'Windows')
-def executeCommand(command, cwd=None, env=None):
+def executeCommand(command, cwd=None, env=None, timeout=None):
     p = subprocess.Popen(command, cwd=cwd,
----------------
How does executeCommand communicate to the caller that a timeout has occurred?

http://reviews.llvm.org/D6584

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list