[Lldb-commits] [PATCH] D14952: Create new "platform process connect" command

[Pavel Labath via lldb-commits]

labath added a comment.

Thank you for writing the test. I have just a couple of more comments and them I'm done... :)

Please wait for @clayborg to sign off on this as well.


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:13
@@ +12,3 @@
+    def test_platform_process_connect(self):
+        if not lldb.remote_platform:
+            self.skipTest("Only supported on remote platforms")
----------------
I'm quite opposed to "inline skips", I'd like them to be more declarative. Let's add a "remote" parameter to the skipIf decorator and use that. Suggested semantics:
- None: don't care
- True: skip if the target is remote
- False: skip if the target is *not* remote


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:26
@@ +25,3 @@
+        port_file = "%s/port" % working_dir
+        commandline_args = ["p", "-L", "*:0", "-f", port_file, "--", "%s/a.out" % working_dir, "foo"]
+        new_platform = self.spawnSubprocess(self.debug_monitor_exe, commandline_args, install_remote=False)
----------------
Please spell out the arguments in full. It makes it much more obvious what is going to happen.

================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:29
@@ +28,3 @@
+        new_port = self.run_shell_cmd("while [ ! -f %s ]; do sleep 0.25; done && cat %s" % (port_file, port_file))
+        lldb.remote_platform.DisconnectRemote()
+
----------------
What will happen when the next test runs? Will we be able to reconnect to the old platform instance? Please check whether this works when run in --no-multiprocess mode with other tests...
If it does not work, we might be able to create a brand new SBDebugger instance and use that to connect...


http://reviews.llvm.org/D14952