TBR: davidjames

Change-Id: I0b8d4277b142185e69abcd8e8bb2dce6395abc56
diff --git a/lib/cros_build_lib.py b/lib/cros_build_lib.py
index e382445..1d2860b 100644
--- a/lib/cros_build_lib.py
+++ b/lib/cros_build_lib.py
@@ -5,6 +5,7 @@
 """Common python commands used by various build scripts."""
 
 import os
+import re
 import subprocess
 import sys
 
@@ -31,19 +32,24 @@
                cwd=None, input=None, enter_chroot=False, shell=False):
   """Runs a command.
 
-  Keyword arguments:
-    cmd - cmd to run.  Should be input to subprocess.Popen.
-    print_cmd -- prints the command before running it.
-    error_ok -- does not raise an exception on error.
-    error_message -- prints out this message when an error occurrs.
-    exit_code -- returns the return code of the shell command.
-    redirect_stdout -- returns the stdout.
-    redirect_stderr -- holds stderr output until input is communicated.
-    cwd -- the working directory to run this cmd.
-    input -- input to pipe into this command through stdin.
-    enter_chroot -- this command should be run from within the chroot.  If set,
+  Args:
+    cmd: cmd to run.  Should be input to subprocess.Popen.
+    print_cmd: prints the command before running it.
+    error_ok: does not raise an exception on error.
+    error_message: prints out this message when an error occurrs.
+    exit_code: returns the return code of the shell command.
+    redirect_stdout: returns the stdout.
+    redirect_stderr: holds stderr output until input is communicated.
+    cwd: the working directory to run this cmd.
+    input: input to pipe into this command through stdin.
+    enter_chroot: this command should be run from within the chroot.  If set,
       cwd must point to the scripts directory.
-    shell -- If shell is True, the specified command will be executed through the shell.
+    shell: If shell is True, the specified command will be executed through
+      the shell.
+
+  Returns:
+    A CommandResult object.
+
   Raises:
     Exception:  Raises generic exception on error with optional error_message.
   """
@@ -51,13 +57,14 @@
   stdout = None
   stderr = None
   stdin = None
-  output = ''
   cmd_result = CommandResult()
 
   # Modify defaults based on parameters.
-  if redirect_stdout:  stdout = subprocess.PIPE
-  if redirect_stderr:  stderr = subprocess.PIPE
-  if input:  stdin = subprocess.PIPE
+  if redirect_stdout: stdout = subprocess.PIPE
+  if redirect_stderr: stderr = subprocess.PIPE
+  # TODO(sosa): gpylint complains about redefining built-in 'input'.
+  #   Can we rename this variable?
+  if input: stdin = subprocess.PIPE
   if isinstance(cmd, basestring):
     if enter_chroot: cmd = './enter_chroot.sh -- ' + cmd
     cmd_str = cmd
@@ -82,7 +89,8 @@
       msg = ('Command "%s" failed.\n' % cmd_str +
              (error_message or cmd_result.error or cmd_result.output or ''))
       raise RunCommandError(msg)
-  except Exception,e:
+  # TODO(sosa): is it possible not to use the catch-all Exception here?
+  except Exception, e:
     if not error_ok:
       raise
     else:
@@ -105,7 +113,7 @@
   def Color(self, color, text):
     """Returns text with conditionally added color escape sequences.
 
-    Keyword arguments:
+    Args:
       color: Text color -- one of the color constants defined in this class.
       text: The text to color.
 
@@ -125,7 +133,7 @@
 def Die(message):
   """Emits a red error message and halts execution.
 
-  Keyword arguments:
+  Args:
     message: The message to be emitted before exiting.
   """
   print >> sys.stderr, (
@@ -133,10 +141,11 @@
   sys.exit(1)
 
 
+# pylint: disable-msg=W0622
 def Warning(message):
   """Emits a yellow warning message and continues execution.
 
-  Keyword arguments:
+  Args:
     message: The message to be emitted.
   """
   print >> sys.stderr, (
@@ -146,7 +155,7 @@
 def Info(message):
   """Emits a blue informational message and continues execution.
 
-  Keyword arguments:
+  Args:
     message: The message to be emitted.
   """
   print >> sys.stderr, (
@@ -156,7 +165,7 @@
 def ListFiles(base_dir):
   """Recurively list files in a directory.
 
-  Keyword arguments:
+  Args:
     base_dir: directory to start recursively listing in.
 
   Returns:
@@ -175,3 +184,69 @@
         directories.append(fullpath)
 
   return files_list
+
+
+def IsInsideChroot():
+  """Returns True if we are inside chroot."""
+  return os.path.exists('/etc/debian_chroot')
+
+
+def GetSrcRoot():
+  """Get absolute path to src/scripts/ directory.
+
+  Assuming test script will always be run from descendent of src/scripts.
+
+  Returns:
+    A string, absolute path to src/scripts directory. None if not found.
+  """
+  src_root = None
+  match_str = '/src/scripts/'
+  test_script_path = os.path.abspath('.')
+
+  path_list = re.split(match_str, test_script_path)
+  if path_list:
+    src_root = os.path.join(path_list[0], match_str.strip('/'))
+    Info ('src_root = %r' % src_root)
+  else:
+    Info ('No %r found in %r' % (match_str, test_script_path))
+
+  return src_root
+
+
+def GetChromeosVersion(str_obj):
+  """Helper method to parse output for CHROMEOS_VERSION_STRING.
+
+  Args:
+    str_obj: a string, which may contain Chrome OS version info.
+
+  Returns:
+    A string, value of CHROMEOS_VERSION_STRING environment variable set by
+      chromeos_version.sh. Or None if not found.
+  """
+  if str_obj is not None:
+    match = re.search('CHROMEOS_VERSION_STRING=([0-9_.]+)', str_obj)
+    if match and match.group(1):
+      Info ('CHROMEOS_VERSION_STRING = %s' % match.group(1))
+      return match.group(1)
+
+  Info ('CHROMEOS_VERSION_STRING NOT found')
+  return None
+
+
+def GetOutputImageDir(board, cros_version):
+  """Construct absolute path to output image directory.
+
+  Args:
+    board: a string.
+    cros_version: a string, Chrome OS version.
+
+  Returns:
+    a string: absolute path to output directory.
+  """
+  src_root = GetSrcRoot()
+  rel_path = 'build/images/%s' % board
+  # ASSUME: --build_attempt always sets to 1
+  version_str = '-'.join([cros_version, 'a1'])
+  output_dir = os.path.join(os.path.dirname(src_root), rel_path, version_str)
+  Info ('output_dir = %s' % output_dir)
+  return output_dir
diff --git a/lib/cros_build_lib_unittest.py b/lib/cros_build_lib_unittest.py
index 064dd7c..120a866 100755
--- a/lib/cros_build_lib_unittest.py
+++ b/lib/cros_build_lib_unittest.py
@@ -28,7 +28,7 @@
     self.mox.UnsetStubs()
     self.mox.VerifyAll()
 
-  def _assertCrEqual(self, expected, actual):
+  def _AssertCrEqual(self, expected, actual):
     """Helper method to compare two CommandResult objects.
 
     This is needed since assertEqual does not know how to compare two
@@ -43,7 +43,7 @@
     self.assertEqual(expected.output, actual.output)
     self.assertEqual(expected.returncode, actual.returncode)
 
-  def _testCmd(self, cmd, sp_kv=dict(), rc_kv=dict()):
+  def _TestCmd(self, cmd, sp_kv=dict(), rc_kv=dict()):
     """Factor out common setup logic for testing --cmd.
 
     Args:
@@ -72,26 +72,26 @@
     self.proc_mock.communicate(None).AndReturn((self.output, self.error))
     self.mox.ReplayAll()
     actual_result = cros_build_lib.RunCommand(cmd, **rc_kv)
-    self._assertCrEqual(expected_result, actual_result)
+    self._AssertCrEqual(expected_result, actual_result)
 
   def testReturnCodeZeroWithArrayCmd(self):
     """--enter_chroot=False and --cmd is an array of strings."""
     self.proc_mock.returncode = 0
     cmd_list = ['foo', 'bar', 'roger']
     self.cmd = 'foo bar roger'
-    self._testCmd(cmd_list, rc_kv=dict(exit_code=True))
+    self._TestCmd(cmd_list, rc_kv=dict(exit_code=True))
 
   def testReturnCodeZeroWithArrayCmdEnterChroot(self):
     """--enter_chroot=True and --cmd is an array of strings."""
     self.proc_mock.returncode = 0
     cmd_list = ['foo', 'bar', 'roger']
     self.cmd = './enter_chroot.sh -- %s' % ' '.join(cmd_list)
-    self._testCmd(cmd_list, rc_kv=dict(enter_chroot=True))
+    self._TestCmd(cmd_list, rc_kv=dict(enter_chroot=True))
 
   def testReturnCodeNotZeroErrorOkNotRaisesError(self):
     """Raise error when proc.communicate() returns non-zero."""
     self.proc_mock.returncode = 1
-    self._testCmd(self.cmd, rc_kv=dict(error_ok=True))
+    self._TestCmd(self.cmd, rc_kv=dict(error_ok=True))
 
   def testSubprocessCommunicateExceptionRaisesError(self):
     """Verify error raised by communicate() is caught."""
@@ -113,7 +113,7 @@
     self.mox.ReplayAll()
     actual_result = cros_build_lib.RunCommand(self.cmd, error_ok=True,
                                               enter_chroot=True)
-    self._assertCrEqual(expected_result, actual_result)
+    self._AssertCrEqual(expected_result, actual_result)
 
 
 class TestListFiles(unittest.TestCase):
@@ -124,7 +124,7 @@
   def tearDown(self):
     shutil.rmtree(self.root_dir)
 
-  def _createNestedDir(self, dir_structure):
+  def _CreateNestedDir(self, dir_structure):
     for entry in dir_structure:
       full_path = os.path.join(os.path.join(self.root_dir, entry))
       # ensure dirs are created
@@ -147,18 +147,18 @@
     """Test that we are traversing the directory properly."""
     dir_structure = ['one/two/test.txt', 'one/blah.py',
                      'three/extra.conf']
-    self._createNestedDir(dir_structure)
+    self._CreateNestedDir(dir_structure)
 
     files = cros_build_lib.ListFiles(self.root_dir)
-    for file in files:
-      file = file.replace(self.root_dir, '').lstrip('/')
-      if file not in dir_structure:
-        self.fail('%s was not found in %s' % (file, dir_structure))
+    for f in files:
+      f = f.replace(self.root_dir, '').lstrip('/')
+      if f not in dir_structure:
+        self.fail('%s was not found in %s' % (f, dir_structure))
 
   def testEmptyFilePath(self):
     """Test that we return nothing when directories are empty."""
     dir_structure = ['one/', 'two/', 'one/a/']
-    self._createNestedDir(dir_structure)
+    self._CreateNestedDir(dir_structure)
     files = cros_build_lib.ListFiles(self.root_dir)
     self.assertEqual(files, [])
 
@@ -169,5 +169,62 @@
       self.assertEqual(err.errno, errno.ENOENT)
 
 
+class HelperMethodMoxTests(unittest.TestCase):
+  """Tests for various helper methods using mox."""
+
+  def setUp(self):
+    self.mox = mox.Mox()
+    self.mox.StubOutWithMock(os.path, 'abspath')
+
+  def tearDown(self):
+    self.mox.UnsetStubs()
+    self.mox.VerifyAll()
+
+  def testGetSrcRoot(self):
+    test_path = '/tmp/foo/src/scripts/bar/more'
+    expected = '/tmp/foo/src/scripts'
+    os.path.abspath('.').AndReturn(test_path)
+    self.mox.ReplayAll()
+    actual = cros_build_lib.GetSrcRoot()
+    self.assertEqual(expected, actual)
+
+  def testGetOutputImageDir(self):
+    expected = '/tmp/foo/src/build/images/x86-generic/0.0.1-a1'
+    self.mox.StubOutWithMock(cros_build_lib, 'GetSrcRoot')
+    cros_build_lib.GetSrcRoot().AndReturn('/tmp/foo/src/scripts')
+    self.mox.ReplayAll()
+    actual = cros_build_lib.GetOutputImageDir('x86-generic', '0.0.1')
+    self.assertEqual(expected, actual)
+
+
+class HelperMethodSimpleTests(unittest.TestCase):
+  """Tests for various helper methods without using mox."""
+
+  def _TestChromeosVersion(self, test_str, expected=None):
+    actual = cros_build_lib.GetChromeosVersion(test_str)
+    self.assertEqual(expected, actual)
+
+  def testGetChromeosVersionWithValidVersionReturnsValue(self):
+    expected = '0.8.71.2010_09_10_1530'
+    test_str = ' CHROMEOS_VERSION_STRING=0.8.71.2010_09_10_1530 '
+    self._TestChromeosVersion(test_str, expected)
+
+  def testGetChromeosVersionWithMultipleVersionReturnsFirstMatch(self):
+    expected = '0.8.71.2010_09_10_1530'
+    test_str = (' CHROMEOS_VERSION_STRING=0.8.71.2010_09_10_1530 '
+                ' CHROMEOS_VERSION_STRING=10_1530 ')
+    self._TestChromeosVersion(test_str, expected)
+
+  def testGetChromeosVersionWithInvalidVersionReturnsDefault(self):
+    test_str = ' CHROMEOS_VERSION_STRING=invalid_version_string '
+    self._TestChromeosVersion(test_str)
+
+  def testGetChromeosVersionWithEmptyInputReturnsDefault(self):
+    self._TestChromeosVersion('')
+
+  def testGetChromeosVersionWithNoneInputReturnsDefault(self):
+    self._TestChromeosVersion(None)
+
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/tests/build_image_test.py b/tests/build_image_test.py
index 55ee91c..8c0c7fa 100644
--- a/tests/build_image_test.py
+++ b/tests/build_image_test.py
@@ -4,100 +4,30 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-"""Tests for build_image.
+"""Tests for build_image shell script.
 
 Note:
-  Some of the Shell scripts invoked in child process require sudo access.
-  Be prepared to enter your password when prompted.
+  This script must be run from INSIDE chroot.
 
 Sample usage:
-  # run all test cases
-  python build_image_test.py
+  # (inside chroot) pushd ~/trunk/src/scripts/
+  # run all test cases in this script
+  python chromite/tests/build_image_test.py
+
+  # run all test cases in a test suite
+  python chromite/tests/build_image_test.py BuildImageTest
 
   # run a specific test
-  python build_image_test.py BuildImageTest.testBuildImageWithoutBoardExit
+  python chromite/tests/build_image_test.py BuildImageTest.testWithoutBoardExit
 """
 
-import logging
 import os
 import re
 import sys
 import unittest
 sys.path.append(os.path.join(os.path.dirname(__file__), '../lib'))
-from cros_build_lib import RunCommand
-
-
-logging.basicConfig(level=logging.INFO)
-
-
-# TODO(tgao): move this into cros_build_lib.py
-def IsInsideChroot():
-  """Returns True if we are inside chroot."""
-  return os.path.exists('/etc/debian_chroot')
-
-
-def GetSrcRoot():
-  """Get absolute path to src/scripts/ directory.
-
-  Assuming test script will always be run from descendent of src/scripts.
-
-  Returns:
-    A string, absolute path to src/scripts directory. None if not found.
-  """
-  src_root = None
-  match_str = '/src/scripts/'
-  test_script_path = os.path.abspath('.')
-  logging.debug('test_script_path = %r', test_script_path)
-
-  path_list = re.split(match_str, test_script_path)
-  logging.debug('path_list = %r', path_list)
-  if path_list:
-    src_root = os.path.join(path_list[0], match_str.strip('/'))
-    logging.debug('src_root = %r', src_root)
-  else:
-    logging.debug('No %r found in %r', match_str, test_script_path)
-
-  return src_root
-
-
-# TODO(tgao): move this into cros_build_lib.py
-def GetChromeosVersion(str_obj):
-  """Helper method to parse output for CHROMEOS_VERSION_STRING.
-
-  Args:
-    str_obj: a string, which may contain Chrome OS version info.
-
-  Returns:
-    A string, value of CHROMEOS_VERSION_STRING environment variable set by
-      chromeos_version.sh. Or None if not found.
-  """
-  match = re.search('CHROMEOS_VERSION_STRING=([0-9_.]+)', str_obj)
-  if match and match.group(1):
-    logging.info('CHROMEOS_VERSION_STRING = %s', match.group(1))
-    return match.group(1)
-
-  logging.info('CHROMEOS_VERSION_STRING NOT found')
-  return None
-
-
-# TODO(tgao): move this into cros_build_lib.py
-def GetOutputImageDir(board, cros_version):
-  """Construct absolute path to output image directory.
-
-  Args:
-    board: a string.
-    cros_version: a string, Chrome OS version.
-
-  Returns:
-    a string: absolute path to output directory.
-  """
-  src_root = GetSrcRoot()
-  rel_path = 'build/images/%s' % board
-  # ASSUME: --build_attempt always sets to 1
-  version_str = '-'.join([cros_version, 'a1'])
-  output_dir = os.path.join(os.path.dirname(src_root), rel_path, version_str)
-  logging.info('output_dir = %s', output_dir)
-  return output_dir
+from cros_build_lib import (RunCommand, IsInsideChroot, GetChromeosVersion,
+                            GetOutputImageDir)
 
 
 class BuildImageTest(unittest.TestCase):
@@ -130,14 +60,14 @@
       assert_success: a boolean. True == check child process return code is 0.
         False otherwise.
     """
-    logging.info('About to run command: %s', cmd)
+    Info ('About to run command: %s' % cmd)
     cmd_result = RunCommand(
       cmd, error_ok=True, exit_code=True, redirect_stdout=True,
       redirect_stderr=True, shell=True)
     self.output = cmd_result.output
     self.error = cmd_result.error
-    logging.info('output =\n%r', self.output)
-    logging.info('error =\n%s', self.error)
+    Info ('output =\n%r' % self.output)
+    Info ('error =\n%r' % self.error)
 
     message = 'cmd should have failed! error:\n%s' % self.error
     if assert_success:
@@ -182,7 +112,7 @@
       image_list: a list of strings, names of output images.
     """
     cmd = './build_image --board=%s' % board
-    logging.info('If all goes well, it takes ~5 min. to build an image...')
+    Info ('If all goes well, it takes ~5 min. to build an image...')
     self._RunBuildImageCmd(cmd)
     self._CheckStringPresent(['Image created in', 'copy to USB keyfob'],
                              check_stdout=True)