Move some android platform functions to lldbplatformutil

[zturner]

Summary

Move some android platform functions to lldbplatformutil.

My eventual goal is to move all of the test decorators to their
own module such as `decorators.py`.  But some of the decorators
use existing functions in `lldbtest.py` and conceptually the
functions are probably more appropriately placed in lldbplatformutil.
Moreover, `lldbtest.py` is a huge file with a ton of random utility
functions scattered around, so this patch also works toward the
goal of reducing the footprint of this one module to a more
reasonable size.

So this patch starts by moveing some utility functions over to
`lldbplatformutil` with the eventual goal of moving decorators over
 to their own module as well.

[labath]

Seems to be working after applying the fixes below.

I was considering whether this shouldn't be moved to an even more platform-specific file (say androidutil.py or something), but I'll leave that up to you...

packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py

63

valid_api_levels=...

packages/Python/lldbsuite/test/lldbplatformutil.py

38

subprocess.PIPE

[tberghammer]

Looks reasonable and I agree with Pavel in the idea of moving the android related utility functions into their own file for better separation

packages/Python/lldbsuite/test/lldbplatformutil.py

73–81

Can we just return True/False? I don't see any usecase where we are caring about the reason why the device isn't matched.

packages/Python/lldbsuite/test/lldbtest.py

683–685

Can we move this function to a higher scope so we don't have to duplicate it in every android specific decorator?

[zturner]

In D16830#342842, @tberghammer wrote:

Looks reasonable and I agree with Pavel in the idea of moving the android related utility functions into their own file for better separation

I thought about creating an android specific module but I decided taht there is so little code right now it might not make sense. And then we already had this lldbplatformutil file that had only 1 function in it, so nobody was even really using it. So rather than end up with 2 different modules with 3 functions each, just have just have 1 module with 6 functions. On the other hand, if they continue to grow, then splitting apart makes a lot of sense.

It's not a big deal to me either way, so if you guys feel strongly I can move it. Since you're in a different time zone though and might not get this for another 24 hours, maybe I'll commit like this and then if you

[zturner]

In D16830#343232, @zturner wrote:

In D16830#342842, @tberghammer wrote:

Looks reasonable and I agree with Pavel in the idea of moving the android related utility functions into their own file for better separation

I thought about creating an android specific module but I decided taht there is so little code right now it might not make sense. And then we already had this lldbplatformutil file that had only 1 function in it, so nobody was even really using it. So rather than end up with 2 different modules with 3 functions each, just have just have 1 module with 6 functions. On the other hand, if they continue to grow, then splitting apart makes a lot of sense.

It's not a big deal to me either way, so if you guys feel strongly I can move it. Since you're in a different time zone though and might not get this for another 24 hours, maybe I'll commit like this and then if you

(somehow my previous message got cut off).

if you want it changed let me know and I'll move it to an android-specific file tomorrow.

[sivachandra]

This change broke Android testsuite run:
http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-android/builds/5147

[zturner]

I ahad 3 CLs in progress locally and I guess I messed up a merge. There's
supposed to be a subprocess.PIPE instead of just PIPE. I'll check in a fix
shortly.

[zturner]

r259724

packages/Python/lldbsuite/test/functionalities/inferior-assert/TestInferiorAssert.py

 
 import os, time
 import lldb
-import lldbsuite.test.lldbutil as lldbutil
-import lldbsuite.test.lldbplatformutil as lldbplatformutil
+from lldbsuite.test import lldbutil
+from lldbsuite.test import lldbplatformutil
 from lldbsuite.test.lldbtest import *
 
 class AssertingInferiorTestCase(TestBase):

         lldbutil.run_break_set_by_file_and_line (self, "main.c", line, num_expected_locations=1, loc_exact=True)
 
     def check_stop_reason(self):
-        match_result = matchAndroid(api_levels=list(range(1, 16+1)))(self)
-        if match_result is not None:
+        match_result = lldbplatformutil.match_android_device(self.getArchitecture(), api_levels=list(range(1, 16+1)))
+        if match_result == lldbplatformutil.AndroidMatchResult.Matched:
             # On android until API-16 the abort() call ended in a sigsegv instead of in a sigabrt
             stop_reason = 'stop reason = signal SIGSEGV'
         else:

packages/Python/lldbsuite/test/lldbplatformutil.py

 from __future__ import absolute_import
 
 # System modules
+import re
+import subprocess
 
 # Third-party modules
+from six.moves.urllib import parse as urlparse
 
 # LLDB modules
-
-import re
+from . import configuration
+import use_lldb_suite
+import lldb
 
 def check_first_register_readable(test_case):
     arch = test_case.getArchitecture()

     else:
         # TODO: Add check for other architectures
         test_case.fail("Unsupported architecture for test case (arch: %s)" % test_case.getArchitecture())
+
+def _run_adb_command(cmd, device_id):
+    device_id_args = []
+    if device_id:
+        device_id_args = ["-s", device_id]
+    full_cmd = ["adb"] + device_id_args + cmd
+    p = subprocess.Popen(full_cmd, stdout=PIPE, stderr=PIPE)
+    stdout, stderr = p.communicate()
+    return p.returncode, stdout, stderr
+
+def _target_is_android():
+    if not hasattr(_target_is_android, 'result'):
+        triple = lldb.DBG.GetSelectedPlatform().GetTriple()
+        match = re.match(".*-.*-.*-android", triple)
+        _target_is_android.result = match is not None
+    return _target_is_android.result
+
+def android_device_api():
+    if not hasattr(android_device_api, 'result'):
+        assert configuration.lldb_platform_url is not None
+        device_id = None
+        parsed_url = urlparse.urlparse(configuration.lldb_platform_url)
+        host_name = parsed_url.netloc.split(":")[0]
+        if host_name != 'localhost':
+            device_id = host_name
+            if device_id.startswith('[') and device_id.endswith(']'):
+                device_id = device_id[1:-1]
+        retcode, stdout, stderr = _run_adb_command(
+            ["shell", "getprop", "ro.build.version.sdk"], device_id)
+        if retcode == 0:
+            android_device_api.result = int(stdout)
+        else:
+            raise LookupError(
+                ">>> Unable to determine the API level of the Android device.\n"
+                ">>> stdout:\n%s\n"
+                ">>> stderr:\n%s\n" % (stdout, stderr))
+    return android_device_api.result
+
+class AndroidMatchResult:
+    Matched, InvalidTarget, InvalidArch, InvalidApiLevel = range(4)
+
+def match_android_device(device_arch, valid_archs=None, valid_api_levels=None):
+    if not _target_is_android():
+        return AndroidMatchResult.InvalidTarget
+    if valid_archs is not None and device_arch not in valid_archs:
+        return AndroidMatchResult.InvalidArch
+    if valid_api_levels is not None and android_device_api() not in valid_api_levels:
+        return AndroidMatchResult.InvalidApiLevel
+
+    return AndroidMatchResult.Matched
+
+def finalize_build_dictionary(dictionary):
+    if _target_is_android():
+        if dictionary is None:
+            dictionary = {}
+        dictionary["OS"] = "Android"
+        if android_device_api() >= 16:
+            dictionary["PIE"] = 1
+    return dictionary

packages/Python/lldbsuite/test/lldbtest.py

 import unittest2
 from six import add_metaclass
 from six import StringIO as SixStringIO
-from six.moves.urllib import parse as urlparse
 import six
 
 # LLDB modules
 import use_lldb_suite
 import lldb
 from . import configuration
+from . import lldbplatformutil
 from . import lldbtest_config
 from . import lldbutil
 from . import test_categories

     else:
         return True
 
-def run_adb_command(cmd, device_id):
-    device_id_args = []
-    if device_id:
-        device_id_args = ["-s", device_id]
-    full_cmd = ["adb"] + device_id_args + cmd
-    p = Popen(full_cmd, stdout=PIPE, stderr=PIPE)
-    stdout, stderr = p.communicate()
-    return p.returncode, stdout, stderr
-
-def append_android_envs(dictionary):
-    if dictionary is None:
-        dictionary = {}
-    dictionary["OS"] = "Android"
-    if android_device_api() >= 16:
-        dictionary["PIE"] = 1
-    return dictionary
-
-def target_is_android():
-    if not hasattr(target_is_android, 'result'):
-        triple = lldb.DBG.GetSelectedPlatform().GetTriple()
-        match = re.match(".*-.*-.*-android", triple)
-        target_is_android.result = match is not None
-    return target_is_android.result
-
-def android_device_api():
-    if not hasattr(android_device_api, 'result'):
-        assert configuration.lldb_platform_url is not None
-        device_id = None
-        parsed_url = urlparse.urlparse(configuration.lldb_platform_url)
-        host_name = parsed_url.netloc.split(":")[0]
-        if host_name != 'localhost':
-            device_id = host_name
-            if device_id.startswith('[') and device_id.endswith(']'):
-                device_id = device_id[1:-1]
-        retcode, stdout, stderr = run_adb_command(
-            ["shell", "getprop", "ro.build.version.sdk"], device_id)
-        if retcode == 0:
-            android_device_api.result = int(stdout)
-        else:
-            raise LookupError(
-                ">>> Unable to determine the API level of the Android device.\n"
-                ">>> stdout:\n%s\n"
-                ">>> stderr:\n%s\n" % (stdout, stderr))
-    return android_device_api.result
-
 def check_expected_version(comparison, expected, actual):
     def fn_leq(x,y): return x <= y
     def fn_less(x,y): return x < y

 def expectedFailureHostWindows(bugnumber=None, compilers=None):
     return expectedFailureHostOS(['windows'], bugnumber, compilers)
 
-def matchAndroid(api_levels=None, archs=None):
-    def match(self):
-        if not target_is_android():
-            return None     # Doesn't match, return false
-        if archs is not None and self.getArchitecture() not in archs:
-            return None     # Invalid architecture, return false
-        if api_levels is not None and android_device_api() not in api_levels:
-            return None     # API level doesn't match, return false
-
-        # This is a matching android distribution, return true
-        return "Android [arch={}] [api_level={}]".format(self.getArchitecture(), android_device_api())
-    return match
-
-
 def expectedFailureAndroid(bugnumber=None, api_levels=None, archs=None):
     """ Mark a test as xfail for Android.
 

         arch - A sequence of architecture names specifying the architectures
             for which a test is expected to fail. None means all architectures.
     """
-    return expectedFailure(matchAndroid(api_levels, archs), bugnumber)
+    def skip_for_android(self):
+        result = lldbplatformutil.match_android_device(self.getArchitecture(), archs, api_levels)
+        return "skipping for android" if result == lldbplatformutil.AndroidMatchResult.Matched else None
+    return expectedFailure(skip_for_android, bugnumber)
 
 # Flakey tests get two chances to run. If they fail the first time round, the result formatter
 # makes sure it is run one more time.

     return expectedFlakeyCompiler('gcc', compiler_version, bugnumber)
 
 def expectedFlakeyAndroid(bugnumber=None, api_levels=None, archs=None):
-    return expectedFlakey(matchAndroid(api_levels, archs), bugnumber)
+    def skip_for_android(self):
+        result = lldbplatformutil.match_android_device(self.getArchitecture(), archs, api_levels)
+        return "skipping for android" if result == lldbplatformutil.AndroidMatchResult.Matched else None
+    return expectedFlakey(skip_for_android, bugnumber)
 
 def skipIfRemote(func):
     """Decorate the item to skip tests if testing remotely."""

         arch - A sequence of architecture names specifying the architectures
             for which a test is skipped. None means all architectures.
     """
-    def is_target_android(self):
-        return matchAndroid(api_levels, archs)(self)
-    return skipTestIfFn(is_target_android)
+    def skip_for_android(self):
+        result = lldbplatformutil.match_android_device(self.getArchitecture(), archs, api_levels)
+        return "skipping for android" if result == lldbplatformutil.AndroidMatchResult.Matched else None
+    return skipTestIfFn(skip_for_android)
 
 def skipUnlessCompilerRt(func):
     """Decorate the item to skip tests if testing remotely."""

     def buildDefault(self, architecture=None, compiler=None, dictionary=None, clean=True):
         """Platform specific way to build the default binaries."""
         module = builder_module()
-        if target_is_android():
-            dictionary = append_android_envs(dictionary)
+        dictionary = lldbplatformutil.finalize_build_dictionary(dictionary)
         if not module.buildDefault(self, architecture, compiler, dictionary, clean):
             raise Exception("Don't know how to build default binary")
 

     def buildDwarf(self, architecture=None, compiler=None, dictionary=None, clean=True):
         """Platform specific way to build binaries with dwarf maps."""
         module = builder_module()
-        if target_is_android():
-            dictionary = append_android_envs(dictionary)
+        dictionary = lldbplatformutil.finalize_build_dictionary(dictionary)
         if not module.buildDwarf(self, architecture, compiler, dictionary, clean):
             raise Exception("Don't know how to build binary with dwarf")
 
     def buildDwo(self, architecture=None, compiler=None, dictionary=None, clean=True):
         """Platform specific way to build binaries with dwarf maps."""
         module = builder_module()
-        if target_is_android():
-            dictionary = append_android_envs(dictionary)
+        dictionary = lldbplatformutil.finalize_build_dictionary(dictionary)
         if not module.buildDwo(self, architecture, compiler, dictionary, clean):
             raise Exception("Don't know how to build binary with dwo")
 

     def build(self, architecture=None, compiler=None, dictionary=None, clean=True):
         """Platform specific way to build the default binaries."""
         module = builder_module()
-        if target_is_android():
-            dictionary = append_android_envs(dictionary)
+        dictionary = lldbplatformutil.finalize_build_dictionary(dictionary)
         if self.debug_info is None:
             return self.buildDefault(architecture, compiler, dictionary, clean)
         elif self.debug_info == "dsym":