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

Jonathan Roelofs jonathan at codesourcery.com
Thu Feb 5 14:57:50 PST 2015


================
Comment at: test/libcxx/test/format.py:149
@@ +148,3 @@
+            # If there's a target_executor, let it do its own exe cleanup
+            if not target_executor:
+                self._clean(exec_path)
----------------
danalbert wrote:
> This should stay as it was. The target executor should clean up its own mess, but not the mess from the higher level (this is true with and without my suggestion at L122).
Good point.

================
Comment at: test/libcxx/test/remote.py:6
@@ +5,3 @@
+# Shamelessly stolen from lit/util.py, so we don't
+# have to deal with a lit import:
+import signal
----------------
danalbert wrote:
> What makes import hard here?
when imported from remoteRun.py, the imports here don't work because we do funny business somewhere in the lit.cfg that adds the llvm imports to some path that python uses to search from.

I'll add the imports, drop this duped code, and drop the remoteRun.py. Can add that back and fix the import problem later when we try to get ShTest going.

================
Comment at: test/libcxx/test/remote.py:212
@@ +211,3 @@
+        self.username = username
+        self.host     = host
+        self.scpCommand = 'scp'
----------------
danalbert wrote:
> Don't align assignments (you haven't matched all of them anyway).
ok.

I really wish phabricator used a monospaced font for diffs.... haven't been able to find a setting that changes that.

================
Comment at: test/libcxx/test/remote.py:223
@@ +222,3 @@
+        # For temp cleanup
+        self.remote_temp_files = []
+        self.remote_temp_dirs = []
----------------
danalbert wrote:
> Your naming is inconsistent. `self.scpCommand` but `self.remote_temp_files`. I prefer PEP8 stlye (with underscores), but LIT already chose the anti-Python style, so let's try to stick with that. (I'm guilty of this crime too.)
as in camelCase for vars, and underscore_separated_functions() ? Ok.

http://reviews.llvm.org/D7380

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






More information about the cfe-commits mailing list