[PATCH] Copy data files to the remote runner.

Jonathan Roelofs jonathan at codesourcery.com
Tue Mar 10 13:54:18 PDT 2015


LGTM with a couple of small tweaks.


================
Comment at: test/libcxx/test/executor.py:16
@@ -15,3 +15,3 @@
             cmd: [str]:      subprocess.call style command
             local_cwd: str:  Local path to the working directory
             env: {str: str}: Environment variables to execute under
----------------
Need to update the docs to reference the new parameter.

================
Comment at: test/libcxx/test/executor.py:116
@@ -115,2 +115,3 @@
             target_cwd = self.remote_temp_dir()
+            target_exe_path = os.path.join(target_cwd, 'libcxx_test')
             if cmd:
----------------
Should probably

    s/libcxx_test/libcxx_test\.exe/

... even though Linux and Darwin don't care about the extension. It makes the intent slightly clearer.

Perhaps this should go in a separate commit? This line LGTM if you want to commit that now.

================
Comment at: test/libcxx/test/executor.py:127
@@ -122,1 +126,3 @@
+                dev_path = os.path.join(target_cwd, file_basename)
+                self._copy_in_file(file_dep, dev_path)
             return self._execute_command_remote(cmd, target_cwd, env)
----------------
It would be better if this were rolled in with the above call to copy_in, that way the copy could later be implemented with compression/decompression steps at either end.

================
Comment at: test/libcxx/test/format.py:118
@@ -117,1 +117,3 @@
                 env = self.exec_env
+            data_files = [os.path.join(local_cwd, f)
+                          for f in os.listdir(local_cwd) if f.endswith('.dat')]
----------------
I would much rather these be explicitly called out in the test's metadata, but this is fine for now. Perhaps add a TODO?

http://reviews.llvm.org/D8118

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






More information about the cfe-commits mailing list