[NaCl SDK] Fix a bug in build_version.ChromeVersion()

And add tests.

BUG=408896
R=sbc@chromium.org

Review URL: https://codereview.chromium.org/524503002

Cr-Commit-Position: refs/heads/master@{#292641}

native_client_sdk/src/build_tools/build_version.py

@@ -30,7 +30,7 @@ def ChromeVersion():
   '''
   info = FetchVersionInfo()
   if info.url == 'git':
-    _, ref, revision = ParseCommitPosition(info)
+    _, ref, revision = ParseCommitPosition(info.revision)
     if ref == 'refs/heads/master':
       return 'trunk.%s' % revision
   return ChromeVersionNoTrunk()
@@ -93,8 +93,7 @@ def NaClRevision():
 
 def FetchVersionInfo(directory=None,
                      directory_regex_prior_to_src_url='chrome|blink|svn'):
-  '''
-  Returns the last change (in the form of a branch, revision tuple),
+  '''Returns the last change (in the form of a branch, revision tuple),
   from some appropriate revision control system.
 
   TODO(binji): This is copied from lastchange.py. Remove this function and use
@@ -112,47 +111,45 @@ def FetchVersionInfo(directory=None,
 
 
 def FetchGitCommitPosition(directory=None):
+  '''Return the "commit-position" of the Chromium git repo. This should be
+  equivalent to the SVN revision if one exists.
   '''
-  Return the "commit-position" of the Chromium git repo. This should be
-  equivalent to the SVN revision if one eixsts.
+  SEARCH_LIMIT = 100
+  for i in xrange(SEARCH_LIMIT):
+    cmd = ['show', '-s', '--format=%H%n%B', 'HEAD~%d' % i]
+    proc = lastchange.RunGitCommand(directory, cmd)
+    if not proc:
+      break
 
-  This is a copy of the (recently reverted) change in lastchange.py.
-  TODO(binji): Move this logic to lastchange.py when the dust settles.
-  (see crbug.com/406783)
-  '''
-  hsh = ''
-  proc = lastchange.RunGitCommand(directory, ['rev-parse', 'HEAD'])
-  if proc:
-    output = proc.communicate()[0].strip()
-    if proc.returncode == 0 and output:
-      hsh = output
-  if not hsh:
-    return None
-  pos = ''
-  proc = lastchange.RunGitCommand(directory,
-                                  ['show', '-s', '--format=%B', 'HEAD'])
-  if proc:
     output = proc.communicate()[0]
-    if proc.returncode == 0 and output:
-      for line in reversed(output.splitlines()):
-        if line.startswith('Cr-Commit-Position:'):
-          pos = line.rsplit()[-1].strip()
-          break
-  if not pos:
-    return lastchange.VersionInfo('git', hsh)
-  return lastchange.VersionInfo('git', '%s-%s' % (hsh, pos))
+    if not (proc.returncode == 0 and output):
+      break
+
+    lines = output.splitlines()
+
+    # First line is the hash.
+    hsh = lines[0]
+    if not re.match(r'[0-9a-fA-F]+', hsh):
+      break
+
+    for line in reversed(lines):
+      if line.startswith('Cr-Commit-Position:'):
+        pos = line.rsplit()[-1].strip()
+        return lastchange.VersionInfo('git', '%s-%s' % (hsh, pos))
+
+  raise Exception('Unable to fetch a Git Commit Position.')
+
 
 
 def ParseCommitPosition(commit_position):
-  '''
-  Parse a Chrome commit position into its components.
+  '''Parse a Chrome commit position into its components.
 
   Given a commit position like:
     0178d4831bd36b5fb9ff477f03dc43b11626a6dc-refs/heads/master@{#292238}
   Returns:
     ("0178d4831bd36b5fb9ff477f03dc43b11626a6dc", "refs/heads/master", "292238")
   '''
-  m = re.match(r'([0-9a-fA-F]+)-([^@]+)@{#(\d+)}', commit_position)
+  m = re.match(r'([0-9a-fA-F]+)(?:-([^@]+)@{#(\d+)})?', commit_position)
   if m:
     return m.groups()
   return None

native_client_sdk/src/build_tools/tests/build_version_test.py

+#!/usr/bin/env python
+# Copyright (c) 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import os
+import sys
+import collections
+import unittest
+
+SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+BUILD_TOOLS_DIR = os.path.dirname(SCRIPT_DIR)
+CHROME_SRC = os.path.dirname(os.path.dirname(os.path.dirname(BUILD_TOOLS_DIR)))
+MOCK_DIR = os.path.join(CHROME_SRC, "third_party", "pymock")
+
+# For the mock library
+sys.path.append(MOCK_DIR)
+import mock
+
+sys.path.append(BUILD_TOOLS_DIR)
+import build_version
+
+ProcInfo = collections.namedtuple('ProcInfo', ['returncode', 'output'])
+
+class TestCase(unittest.TestCase):
+  def setUp(self):
+    self.fetch_svn = mock.patch('lastchange.FetchSVNRevision').start()
+    self.fetch_git_svn = mock.patch('lastchange.FetchGitSVNRevision').start()
+    self.run_git = mock.patch('lastchange.RunGitCommand').start()
+
+    self.fetch_svn.return_value = None
+    self.fetch_git_svn.return_value = None
+
+  def tearDown(self):
+    mock.patch.stopall()
+
+  def mockGitCommand(self, *args):
+    side_effects = []
+    for proc_info in args:
+      mock_proc = mock.MagicMock()
+      mock_proc.returncode = proc_info.returncode
+      comm_result = mock_proc.MagicMock()
+      comm_result.__getitem__.return_value = proc_info.output
+      mock_proc.communicate.return_value = comm_result
+      side_effects.append(mock_proc)
+
+    self.run_git.side_effect = side_effects
+
+  def mockDefaultGitCommand(self):
+    output = """\
+6a8b61d6be4656e682eba005a1dd7f129789129c
+[NaCl SDK] Update build_sdk.py to display Cr-Commit-Position in README.
+
+BUG=none
+R=bradnelson@google.com, bradnelson@chromium.org
+
+Review URL: https://codereview.chromium.org/495423010
+
+Cr-Commit-Position: refs/heads/master@{#292480}"""
+    self.mockGitCommand(ProcInfo(0, output))
+
+  def mockDepthTwoGitCommand(self):
+    output0 = """\
+ae4b444a0aa09a1fa73e59b180d7d957b9a36bf2
+."""
+
+    output1 = """\
+6a8b61d6be4656e682eba005a1dd7f129789129c
+[NaCl SDK] Update build_sdk.py to display Cr-Commit-Position in README.
+
+BUG=none
+R=bradnelson@google.com, bradnelson@chromium.org
+
+Review URL: https://codereview.chromium.org/495423010
+
+Cr-Commit-Position: refs/heads/master@{#292480}"""
+    self.mockGitCommand(ProcInfo(0, output0), ProcInfo(0, output1))
+
+
+  def assertGitShowCalled(self, depth=0):
+    cmd = ['show', '-s', '--format=%H%n%B', 'HEAD~%d' % depth]
+    self.run_git.assert_called_with(None, cmd)
+
+  def testChromeVersion(self):
+    self.mockDefaultGitCommand()
+    result = build_version.ChromeVersion()
+    self.assertGitShowCalled()
+    self.assertEqual(result, 'trunk.292480')
+
+  def testChromeRevision(self):
+    self.mockDefaultGitCommand()
+    result = build_version.ChromeRevision()
+    self.assertGitShowCalled()
+    self.assertEqual(result, '292480')
+
+  def testChromeCommitPosition(self):
+    self.mockDefaultGitCommand()
+    result = build_version.ChromeCommitPosition()
+    self.assertGitShowCalled()
+    self.assertEqual(
+        result,
+        '6a8b61d6be4656e682eba005a1dd7f129789129c-refs/heads/master@{#292480}')
+
+  def testChromeCommitPositionDepthTwo(self):
+    self.mockDepthTwoGitCommand()
+    result = build_version.ChromeCommitPosition()
+    self.assertEqual(
+        result,
+        '6a8b61d6be4656e682eba005a1dd7f129789129c-refs/heads/master@{#292480}')
+
+
+if __name__ == '__main__':
+  unittest.main()

native_client_sdk/src/test_all.py

@@ -30,6 +30,7 @@ EXTRACT_PACKAGES = ['nacl_x86_glibc']
 TOOLCHAIN_OUT = os.path.join(build_paths.OUT_DIR, 'sdk_tests', 'toolchain')
 
 TEST_MODULES = [
+    'build_version_test',
     'create_html_test',
     'create_nmf_test',
     'easy_template_test',