[PATCH] Add remote testing support to the lit config.
Dan Albert
danalbert at google.com
Tue Feb 10 22:33:37 PST 2015
Looking better. At first glance, I think there's some more cleanup that can be done in `remote.py`, but I'll finish the review first thing tomorrow.
BTW, I like the `TargetInfo` bit. It looks like that change is actually cleanly separates from the rest of this. That part LGTM, so go ahead and submit that portion as its own commit.
================
Comment at: test/libcxx/test/config.py:116
@@ -115,2 +115,3 @@
self.execute_external,
+ self.target_executor,
exec_env=self.env)
----------------
Now that even the base case is a `LocalExecutor`, I think the `target_` prefix of this is misleading.
================
Comment at: test/libcxx/test/config.py:120
@@ +119,3 @@
+ def configure_target_executor(self):
+ exec_str = self.get_lit_conf('target_executor', "None")
+ te = eval(exec_str)
----------------
Default should be `"LocalExecutor"`. Then the if/else can disappear.
================
Comment at: test/libcxx/test/config.py:128
@@ +127,3 @@
+ # that the user wants it at the end, but we have no
+ # way of getting at that easily.
+ selt.lit_config.fatal("Cannot infer how to create a Valgrind "
----------------
`useValgrind` should just go away. If the user wants valgrind, they can add the `ValgrindExecutor` themselves.
================
Comment at: test/libcxx/test/config.py:592
@@ -558,2 +591,3 @@
def configure_env(self):
- if sys.platform == 'darwin' and not self.use_system_cxx_lib:
+ if self.target_info.platform() == 'darwin' and not \
+ self.use_system_cxx_lib:
----------------
Use parens rather than the \
And keep the `not` with the thing it's negating.
================
Comment at: test/libcxx/test/format.py:89
@@ -83,1 +88,3 @@
+ return self._evaluate_pass_test(test, tmpBase, execDir, lit_config,
+ self.target_executor)
else:
----------------
No reason to pass this as an argument. Just refer to it as `self.target_executor` in `_evaluate_pass_test`.
================
Comment at: test/libcxx/test/format.py:118
@@ -108,8 +117,3 @@
if self.exec_env:
- cmd += ['env']
- cmd += ['%s=%s' % (k, v) for k, v in self.exec_env.items()]
- if lit_config.useValgrind:
- cmd = lit_config.valgrindArgs + cmd
- cmd += [exec_path]
- out, err, rc = lit.util.executeCommand(
- cmd, cwd=os.path.dirname(source_path))
+ env = self.exec_env
+ out, err, rc = target_executor.copy_run(exec_path, [], local_cwd, env)
----------------
The above three lines are just `env = self.exec_env`.
================
Comment at: test/libcxx/test/format.py:118
@@ -108,8 +117,3 @@
if self.exec_env:
- cmd += ['env']
- cmd += ['%s=%s' % (k, v) for k, v in self.exec_env.items()]
- if lit_config.useValgrind:
- cmd = lit_config.valgrindArgs + cmd
- cmd += [exec_path]
- out, err, rc = lit.util.executeCommand(
- cmd, cwd=os.path.dirname(source_path))
+ env = self.exec_env
+ out, err, rc = target_executor.copy_run(exec_path, [], local_cwd, env)
----------------
danalbert wrote:
> The above three lines are just `env = self.exec_env`.
Ignore the above. I traced it back and found that the default `self.exec_env` is `{}`, not `None`. Phabricator isn't letting me delete the comment though...
http://reviews.llvm.org/D7380
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list