Merge "releasetools: Fix an issue in GetMinSdkVersion."

am: f18ba63095

Change-Id: I43cf88b9394a077fa76166a2b94734f6bd3b7147

tools/releasetools/common.py

@@ -720,18 +720,31 @@ def GetKeyPasswords(keylist):
 
 
 def GetMinSdkVersion(apk_name):
-  """Get the minSdkVersion delared in the APK. This can be both a decimal number
+  """Gets the minSdkVersion declared in the APK.
-  (API Level) or a codename.
+
+  It calls 'aapt' to query the embedded minSdkVersion from the given APK file.
+  This can be both a decimal number (API Level) or a codename.
+
+  Args:
+    apk_name: The APK filename.
+
+  Returns:
+    The parsed SDK version string.
+
+  Raises:
+    ExternalError: On failing to obtain the min SDK version.
   """
+  proc = Run(
+      ["aapt", "dump", "badging", apk_name], stdout=subprocess.PIPE,
+      stderr=subprocess.PIPE)
+  stdoutdata, stderrdata = proc.communicate()
+  if proc.returncode != 0:
+    raise ExternalError(
+        "Failed to obtain minSdkVersion: aapt return code {}:\n{}\n{}".format(
+            proc.returncode, stdoutdata, stderrdata))
 
-  p = Run(["aapt", "dump", "badging", apk_name], stdout=subprocess.PIPE)
+  for line in stdoutdata.split("\n"):
-  output, err = p.communicate()
+    # Looking for lines such as sdkVersion:'23' or sdkVersion:'M'.
-  if err:
-    raise ExternalError("Failed to obtain minSdkVersion: aapt return code %s"
-        % (p.returncode,))
-
-  for line in output.split("\n"):
-    # Looking for lines such as sdkVersion:'23' or sdkVersion:'M'
     m = re.match(r'sdkVersion:\'([^\']*)\'', line)
     if m:
       return m.group(1)
@@ -739,11 +752,20 @@ def GetMinSdkVersion(apk_name):
 
 
 def GetMinSdkVersionInt(apk_name, codename_to_api_level_map):
-  """Get the minSdkVersion declared in the APK as a number (API Level). If
+  """Returns the minSdkVersion declared in the APK as a number (API Level).
-  minSdkVersion is set to a codename, it is translated to a number using the
-  provided map.
-  """
 
+  If minSdkVersion is set to a codename, it is translated to a number using the
+  provided map.
+
+  Args:
+    apk_name: The APK filename.
+
+  Returns:
+    The parsed SDK version number.
+
+  Raises:
+    ExternalError: On failing to get the min SDK version number.
+  """
   version = GetMinSdkVersion(apk_name)
   try:
     return int(version)
@@ -752,8 +774,9 @@ def GetMinSdkVersionInt(apk_name, codename_to_api_level_map):
     if version in codename_to_api_level_map:
       return codename_to_api_level_map[version]
     else:
-      raise ExternalError("Unknown minSdkVersion: '%s'. Known codenames: %s"
+      raise ExternalError(
-                          % (version, codename_to_api_level_map))
+          "Unknown minSdkVersion: '{}'. Known codenames: {}".format(
+              version, codename_to_api_level_map))
 
 
 def SignFile(input_name, output_name, key, password, min_api_level=None,

tools/releasetools/test_common.py

@@ -504,6 +504,23 @@ class CommonApkUtilsTest(unittest.TestCase):
       actual = common.ParseCertificate(cert_fp.read())
     self.assertEqual(expected, actual)
 
+  def test_GetMinSdkVersion(self):
+    test_app = os.path.join(self.testdata_dir, 'TestApp.apk')
+    self.assertEqual('24', common.GetMinSdkVersion(test_app))
+
+  def test_GetMinSdkVersion_invalidInput(self):
+    self.assertRaises(
+        common.ExternalError, common.GetMinSdkVersion, 'does-not-exist.apk')
+
+  def test_GetMinSdkVersionInt(self):
+    test_app = os.path.join(self.testdata_dir, 'TestApp.apk')
+    self.assertEqual(24, common.GetMinSdkVersionInt(test_app, {}))
+
+  def test_GetMinSdkVersionInt_invalidInput(self):
+    self.assertRaises(
+        common.ExternalError, common.GetMinSdkVersionInt, 'does-not-exist.apk',
+        {})
+
 
 class CommonUtilsTest(unittest.TestCase):