Fix TestStubSetSID after unification in to a single lldb-server binary.

[sivachandra]

Summary

lldb-server needs an explicit 'g' or 'p' argument now. Since lldb-server
is started as a gdbserver in this test, 'g' should be passed to it
explicitly.

[ovyalov]

test/tools/lldb-gdbserver/commandline/TestStubSetSID.py

18

Since lldb-server is supposed to be used on all platform except OSX you may use such alternative condition:

return [] if 'darwin' in sys.platform else ['g']

[sivachandra]

Address ovyalov's comment.

[vharron]

lldb/trunk/test/tools/lldb-gdbserver/commandline/TestStubSetSID.py

42 ↗

(On Diff #20441)

since there's only 0-1 elements in the array, why bother with the join?  Why not just 

# returns "g" to make lldb-server run in gdbserver mode
def get_debugserver_mode():
    return "" if 'darwin' in sys.platform else "g"

stub_sid = self.get_stub_sid(' %{mode} --setsid'.format(mode=get_debugserver_mode()))

[sivachandra]

lldb/trunk/test/tools/lldb-gdbserver/commandline/TestStubSetSID.py

42 ↗

(On Diff #20441)

Why lists instead of strings?

It is safer to use lists for command args. See for example https://docs.python.org/2.7/library/subprocess.html#popen-constructor

"... it is recommended to pass args as a sequence."

stub_sid = self.get_stub_sid(' %{mode} --setsid'.format(mode=get_debugserver_mode()))

I do not know if |mode| is relevant terminology in case of debugserver. But if you think it holds for debugserver as well, and that you want this changed, I will do it.

[vharron]

I'm not particular in this case. If you agree, change it. If not, please leave it as is.

test/tools/lldb-gdbserver/commandline/TestStubSetSID.py

 import unittest2
 
 # Add the directory above ours to the python library path since we
 # will import from there.
 import os.path
 import sys
 sys.path.append(os.path.join(os.path.dirname(__file__), ".."))
 
 import gdbremote_testcase
 import os
 import select
 import tempfile
 import time
 from lldbtest import *
 
+
+def get_common_stub_args():
+    if 'linux' in sys.platform:
+        return ['g']
+    else:
+        return []
+
+
 class TestStubSetSIDTestCase(gdbremote_testcase.GdbRemoteTestCaseBase):
     def get_stub_sid(self, extra_stub_args=None):
         # Launch debugserver
         if extra_stub_args:
             self.debug_monitor_extra_args = extra_stub_args
         else:
             self.debug_monitor_extra_args = ""
 
         server = self.launch_debug_monitor()
         self.assertIsNotNone(server)
         self.assertTrue(server.isalive())
         server.expect("(debugserver|lldb-gdbserver)", timeout=10)
 
         # Get the process id for the stub.
         return os.getsid(server.pid)
 
     def sid_is_same_without_setsid(self):
         stub_sid = self.get_stub_sid()
         self.assertEquals(stub_sid, os.getsid(0))
 
     def sid_is_different_with_setsid(self):
-        stub_sid = self.get_stub_sid(" --setsid")
+        stub_sid = self.get_stub_sid(" %s --setsid" % ' '.join(get_common_stub_args()))
         self.assertNotEquals(stub_sid, os.getsid(0))
 
     def sid_is_different_with_S(self):
-        stub_sid = self.get_stub_sid(" -S")
+        stub_sid = self.get_stub_sid(" %s -S" % ' '.join(get_common_stub_args()))
         self.assertNotEquals(stub_sid, os.getsid(0))
 
     @debugserver_test
     @unittest2.expectedFailure() # This is the whole purpose of this feature, I would expect it to be the same without --setsid. Investigate.
     def test_sid_is_same_without_setsid_debugserver(self):
         self.init_debugserver_test()
         self.set_inferior_startup_launch()
         self.sid_is_same_without_setsid()