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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
