[PATCH] Add remote testing support to the lit config.

Eric Fiselier eric at efcs.ca
Tue Feb 10 16:34:45 PST 2015


Drive by comments.


================
Comment at: test/libcxx/test/config.py:120
@@ +119,3 @@
+    def configure_target_executor(self):
+        exec_str = self.get_lit_conf('target_executor', None)
+        if exec_str:
----------------
Isn't this always set to the default by CMake?

================
Comment at: test/libcxx/test/config.py:122
@@ +121,3 @@
+        if exec_str:
+            print exec_str
+            self.target_executor = eval(exec_str)
----------------
Remove this. Use the lit_config reporting methods.

================
Comment at: test/libcxx/test/format.py:78
@@ -75,1 +77,3 @@
         if is_sh_test:
+            if self.target_executor:
+                # We can't run ShTest tests with a target_executor yet.
----------------
Isn't this always true? Can we detect wether we have a local executor for the time being?

================
Comment at: test/libcxx/test/remote.py:11-21
@@ +10,13 @@
+class TargetInfo(object):
+    def platform(self):
+        pass
+
+    def platform_ver(self):
+        pass
+
+    def platform_name(self):
+        pass
+
+    def supports_locale(self, loc):
+        pass
+
----------------
I would throw a `NotImplemented` or similar error from these "abstract" methods

================
Comment at: test/libcxx/test/remote.py:52
@@ +51,3 @@
+
+class Executor(object):
+    def __init__(self):
----------------
I would probably throw from these abstract methods too if they shouldn't be called.

http://reviews.llvm.org/D7380

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






More information about the cfe-commits mailing list