[PATCH] [libcxx] Add support for Android targets to lit.cfg.

Saleem Abdulrasool abdulras at fb.com
Mon Jul 21 11:59:28 PDT 2014


================
Comment at: test/lit.cfg:94
@@ -93,1 +93,3 @@
 
+    def _build(self, exec_path, source_path, compile_only=False):
+        cmd = [self.cxx_under_test, self.cxx_under_test, '-o', exec_path,
----------------
The compile_only is misleading.  !compile_only implies compile and link.  I think that using an enum would be much nicer here:

    (COMPILE_ONLY, LINK_ONLY, COMPILE_AND_LINK) = xrange(3)

================
Comment at: test/lit.cfg:209
@@ +208,3 @@
+            exec_file = os.path.basename(exec_path)
+            device_path = os.path.join('/data/nativetest/', exec_file)
+            cmd = ['adb', 'push', exec_path, device_path]
----------------
Does this have to be /data/nativetest?  Personally, I would vote for /data/local/tmp/nativetest/.

================
Comment at: test/lit.cfg:210
@@ +209,3 @@
+            device_path = os.path.join('/data/nativetest/', exec_file)
+            cmd = ['adb', 'push', exec_path, device_path]
+            out, err, exit_code = self.execute_command(cmd)
----------------
Might be nice to make a parameter to specify where adb is.

================
Comment at: test/lit.cfg:212
@@ +211,3 @@
+            out, err, exit_code = self.execute_command(cmd)
+            cmd = build_cmd + ['&&'] + cmd
+            return cmd, out, err, exit_code
----------------
Ick.  Can you chain this together using exit code checks please?  This feels fragile (quoting and reliance on the exec passing it through to the shell appropriately).

================
Comment at: test/lit.cfg:219
@@ +218,3 @@
+        exec_file = os.path.basename(exec_path)
+        device_path = os.path.join('/data/nativetest/', exec_file)
+        cmd = ['adb', 'shell', 'rm', device_path]
----------------
Hmm... perhaps make the test location a member variable?

http://reviews.llvm.org/D4594






More information about the cfe-commits mailing list