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

Dan Albert danalbert at google.com
Mon Feb 9 14:10:41 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)
----------------
jroelofs wrote:
> 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.
Ah, right. That makes sense. Carry on.

http://reviews.llvm.org/D7380

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






More information about the cfe-commits mailing list