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

Jonathan Roelofs jonathan at codesourcery.com
Sat Feb 7 12:25:44 PST 2015


================
Comment at: test/libcxx/test/config.py:123
@@ +122,3 @@
+        if exec_str:
+            self.target_executor = eval(exec_str)
+            self.lit_config.note("inferred target_executor as: %r" % exec_str)
----------------
danalbert wrote:
> Ewwwwwww eval? Unless there's a really good reason to use an expression that can be evaluated over just naming a type, I think this should be:
> 
>     mod_path, _, executor = exec_str.rpartition('.')
>     mod = importlib.import_module(mod_path)
>     self.target_executor = getattr(mod, executor)()
> 
> Where `exec_str` is the fully qualified type name (i.e. `libcxx.test.remote.LocalExecutor`). Then the wildcard import and the ugly `eval` can go away.
This means that the executors cannot be configured or chained via their constructor arguments, which is something I think is necessary to make, say, SSHExecutors work.... unless I start parsing the args, which leads to just re-inventing eval.

================
Comment at: test/libcxx/test/config.py:126
@@ +125,3 @@
+            self.target_executor = eval(exec_str)
+            self.target_executor = DebugExecutor('local', self.target_executor)
+            self.lit_config.note("inferred target_executor as: %r" % exec_str)
----------------
Drop this line.

================
Comment at: test/libcxx/test/format.py:122
@@ +121,3 @@
+
+            # TODO move this into target_executor.run
+            target_exec_path = target_executor.remote_from_local_file(exec_path)
----------------
Still need to push most of this into target_executor.run

================
Comment at: test/libcxx/test/remote.py:115
@@ +114,3 @@
+        for src, dst in zip(srcs, dsts):
+            if src is not dst:
+                if delete:
----------------
!=

================
Comment at: test/libcxx/test/remote.py:283
@@ +282,3 @@
+
+class DebugExecutor(Executor):
+    def __init__(self, label, chain, before=True, after=True):
----------------
I think this is much better, but it's also extremely complicated now. :/

http://reviews.llvm.org/D7380

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






More information about the cfe-commits mailing list