[NFC] Cleanup the code used to run shell commands from tests

[tberghammer]

Summary

[NFC] Cleanup the code used to run shell commands from tests

Previously we had 3 different method to run shell commands on the
target and 4 copy of code waiting until a given file appears on the
target device (used for syncronization). This CL merges these methods
to 1 run_platform_command and 1 wait_for_file_on_target functions
located in some utility classes.

[labath]

A very nice cleanup.

packages/Python/lldbsuite/test/lldbutil.py

1040 ↗

(On Diff #52681)

Please put 0.30 here. I remember needing to increase this in ChangeProcessGroup because it was too low (=> flaky).

[tberghammer]

packages/Python/lldbsuite/test/lldbutil.py

1040 ↗

(On Diff #52681)

Can we increase the default number of retries instead? 0.25 is nice from the point that the wait times are mostly integers (or how about 0.5)

[tfiala]

The exponential back off code forgot to import 'time'. This code will
trigger (and has on our end) a python exception for 'time' missing.

I'm fixing this now.

lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/attach_denied/TestAttachDenied.py

 from lldbsuite.test import lldbutil
 
 exe_name = 'AttachDenied'  # Must match Makefile
 
 class AttachDeniedTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
-    def run_platform_command(self, cmd):
-        platform = self.dbg.GetSelectedPlatform()
-        shell_command = lldb.SBPlatformShellCommand(cmd)
-        err = platform.Run(shell_command)
-        return (err, shell_command.GetStatus(), shell_command.GetOutput())
-
     @skipIfWindows
     @skipIfiOSSimulator
     def test_attach_to_process_by_id_denied(self):
         """Test attach by process id denied"""
         self.build()
         exe = os.path.join(os.getcwd(), exe_name)
 
         # Use a file as a synchronization point between test and inferior.
         pid_file_path = lldbutil.append_to_process_working_directory(
                 "pid_file_%d" % (int(time.time())))
         self.addTearDownHook(lambda: self.run_platform_command("rm %s" % (pid_file_path)))
 
         # Spawn a new process
         popen = self.spawnSubprocess(exe, [pid_file_path])
         self.addTearDownHook(self.cleanupSubprocesses)
 
-        max_attempts = 5
-        for i in range(max_attempts):
-            err, retcode, msg = self.run_platform_command("ls %s" % pid_file_path)
-            if err.Success() and retcode == 0:
-                break
-            else:
-                print(msg)
-            if i < max_attempts:
-                # Exponential backoff!
-                time.sleep(pow(2, i) * 0.25)
-        else:
-            self.fail("Child PID file %s not found even after %d attempts." % (pid_file_path, max_attempts))
-        err, retcode, pid = self.run_platform_command("cat %s" % (pid_file_path))
-        self.assertTrue(err.Success() and retcode == 0,
-                        "Failed to read file %s: %s, retcode: %d" % (pid_file_path, err.GetCString(), retcode))
+        pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
 
         self.expect('process attach -p ' + pid,
                     startstr = 'error: attach failed:',
                     error = True)

lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_group/TestChangeProcessGroup.py

def test_setpgid(self):
         # Use a file as a synchronization point between test and inferior.
         pid_file_path = lldbutil.append_to_process_working_directory(
                 "pid_file_%d" % (int(time.time())))
         self.addTearDownHook(lambda: self.run_platform_command("rm %s" % (pid_file_path)))
 
         popen = self.spawnSubprocess(exe, [pid_file_path])
         self.addTearDownHook(self.cleanupSubprocesses)
 
-        max_attempts = 5
-        for i in range(max_attempts):
-            err, retcode, msg = self.run_platform_command("ls %s" % pid_file_path)
-            if err.Success() and retcode == 0:
-                break
-            else:
-                print(msg)
-            if i < max_attempts:
-                # Exponential backoff!
-                time.sleep(pow(2, i) * 0.30)
-        else:
-            self.fail("Child PID file %s not found even after %d attempts." % (pid_file_path, max_attempts))
-
-        err, retcode, pid = self.run_platform_command("cat %s" % (pid_file_path))
-
-        self.assertTrue(err.Success() and retcode == 0,
-                "Failed to read file %s: %s, retcode: %d" % (pid_file_path, err.GetCString(), retcode))
+        pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
 
         # make sure we cleanup the forked child also
         def cleanupChild():
             if lldb.remote_platform:
                 lldb.remote_platform.Kill(int(pid))
             else:
                 if os.path.exists("/proc/" + pid):
                     os.kill(int(pid), signal.SIGKILL)
def test_setpgid(self):
         # this also checks that we are still in full control of the child
         value = thread.GetSelectedFrame().EvaluateExpression("(int)getpgid(0)")
         self.assertTrue(value.IsValid())
         self.assertEqual(value.GetValueAsUnsigned(0), int(pid));
 
         # run to completion
         process.Continue()
         self.assertEqual(process.GetState(), lldb.eStateExited)
-
-    def run_platform_command(self, cmd):
-        platform = self.dbg.GetSelectedPlatform()
-        shell_command = lldb.SBPlatformShellCommand(cmd)
-        err = platform.Run(shell_command)
-        return (err, shell_command.GetStatus(), shell_command.GetOutput())

lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

def build(self, architecture=None, compiler=None, dictionary=None, clean=True):
             return self.buildDsym(architecture, compiler, dictionary, clean)
         elif self.debug_info == "dwarf":
             return self.buildDwarf(architecture, compiler, dictionary, clean)
         elif self.debug_info == "dwo":
             return self.buildDwo(architecture, compiler, dictionary, clean)
         else:
             self.fail("Can't build for debug info: %s" % self.debug_info)
 
+    def run_platform_command(self, cmd):
+        platform = self.dbg.GetSelectedPlatform()
+        shell_command = lldb.SBPlatformShellCommand(cmd)
+        err = platform.Run(shell_command)
+        return (err, shell_command.GetStatus(), shell_command.GetOutput())
+
     # =================================================
     # Misc. helper methods for debugging test execution
     # =================================================
 
     def DebugSBValue(self, val):
         """Debug print a SBValue object, if traceAlways is True."""
         from .lldbutil import value_type_to_str
 

lldb/trunk/packages/Python/lldbsuite/test/lldbutil.py

def find_library(target, library):
                     return False
             elif hasattr(library, 'match'):
                 if library.match(filename):
                     return False
         return True
     def find_library_callable(test):
         return find_library(target, library)
     return skip_if_callable(test, find_library_callable, "could not find library matching '%s' in target %s" % (library, target))
+
+def wait_for_file_on_target(testcase, file_path, max_attempts = 6):
+    for i in range(max_attempts):
+        err, retcode, msg = testcase.run_platform_command("ls %s" % file_path)
+        if err.Success() and retcode == 0:
+            break
+        if i < max_attempts:
+            # Exponential backoff!
+            time.sleep(pow(2, i) * 0.25)
+    else:
+        testcase.fail("File %s not found even after %d attempts." % (file_path, max_attempts))
+
+    err, retcode, data = testcase.run_platform_command("cat %s" % (file_path))
+
+    testcase.assertTrue(err.Success() and retcode == 0,
+            "Failed to read file %s: %s, retcode: %d" % (file_path, err.GetCString(), retcode))
+    return data

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py

def get_stub_port_from_named_socket(self, read_timeout_seconds=5):
 
         # Trim null byte, convert to int.
         stub_port_raw = stub_port_raw[:-1]
         stub_port = int(stub_port_raw)
         self.assertTrue(stub_port > 0)
 
         return stub_port
 
-    def run_shell_cmd(self, cmd):
-        platform = self.dbg.GetSelectedPlatform()
-        shell_cmd = lldb.SBPlatformShellCommand(cmd)
-        err = platform.Run(shell_cmd)
-        if err.Fail() or shell_cmd.GetStatus():
-            m = "remote_platform.RunShellCommand('%s') failed:\n" % cmd
-            m += ">>> return code: %d\n" % shell_cmd.GetStatus()
-            if err.Fail():
-                m += ">>> %s\n" % str(err).strip()
-            m += ">>> %s\n" % (shell_cmd.GetOutput() or
-                               "Command generated no output.")
-            raise Exception(m)
-        return shell_cmd.GetOutput().strip()
-
     def init_llgs_test(self, use_named_pipe=True):
         if lldb.remote_platform:
             # Remote platforms don't support named pipe based port negotiation
             use_named_pipe = False
 
             # Grab the ppid from /proc/[shell pid]/stat
-            shell_stat = self.run_shell_cmd("cat /proc/$$/stat")
+            err, retcode, shell_stat = self.run_platform_command("cat /proc/$$/stat")
+            self.assertTrue(err.Success() and retcode == 0,
+                    "Failed to read file /proc/$$/stat: %s, retcode: %d" % (err.GetCString(), retcode))
+
             # [pid] ([executable]) [state] [*ppid*]
             pid = re.match(r"^\d+ \(.+\) . (\d+)", shell_stat).group(1)
-            ls_output = self.run_shell_cmd("ls -l /proc/%s/exe" % pid)
+            err, retcode, ls_output = self.run_platform_command("ls -l /proc/%s/exe" % pid)
+            self.assertTrue(err.Success() and retcode == 0,
+                    "Failed to read file /proc/%s/exe: %s, retcode: %d" % (pid, err.GetCString(), retcode))
             exe = ls_output.split()[-1]
 
             # If the binary has been deleted, the link name has " (deleted)" appended.
             # Remove if it's there.
             self.debug_monitor_exe = re.sub(r' \(deleted\)$', '', exe)
         else:
             self.debug_monitor_exe = get_lldb_server_exe()
             if not self.debug_monitor_exe:
def get_debug_monitor_command_line_args(self, attach_pid=None):
             commandline_args = self.debug_monitor_extra_args + ["localhost:{}".format(self.port)]
 
         if attach_pid:
             commandline_args += ["--attach=%d" % attach_pid]
         if self.named_pipe_path:
             commandline_args += ["--named-pipe", self.named_pipe_path]
         return commandline_args
 
-    def run_platform_command(self, cmd):
-        platform = self.dbg.GetSelectedPlatform()
-        shell_command = lldb.SBPlatformShellCommand(cmd)
-        err = platform.Run(shell_command)
-        return (err, shell_command.GetOutput())
-
     def launch_debug_monitor(self, attach_pid=None, logfile=None):
         # Create the command line.
         commandline_args = self.get_debug_monitor_command_line_args(attach_pid=attach_pid)
 
         # Start the server.
         server = self.spawnSubprocess(self.debug_monitor_exe, commandline_args, install_remote=False)
         self.addTearDownHook(self.cleanupSubprocesses)
         self.assertIsNotNone(server)

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py

def test_platform_process_connect(self):
             listen_url = "%s://%s" % (p.group(1), os.path.join(working_dir, "platform-%d.sock" % int(time.time())))
         else:
             listen_url = "*:0"
 
         port_file = "%s/port" % working_dir
         commandline_args = ["platform", "--listen", listen_url, "--socket-file", port_file, "--", "%s/a.out" % working_dir, "foo"]
         self.spawnSubprocess(self.debug_monitor_exe, commandline_args, install_remote=False)
         self.addTearDownHook(self.cleanupSubprocesses)
-        
-        # Wait until the port_file have been created. Doing it with 1 shell command will fail because
-        # of a bug in LLDB shell escaping code
-        _, _ = self.run_platform_command("while [ ! -f %s ]; do sleep 0.25; done" % port_file)
-        _, socket_id = self.run_platform_command("cat %s" % port_file)
+        socket_id = lldbutil.wait_for_file_on_target(self, port_file)
 
         new_debugger = lldb.SBDebugger.Create()
         new_debugger.SetAsync(False)
         def del_debugger(new_debugger=new_debugger):
             del new_debugger
         self.addTearDownHook(del_debugger)
 
         new_platform = lldb.SBPlatform(lldb.remote_platform.GetName())