[chromedriver] Update run_buildbot_steps.py to work with git hashes

chrome/test/chromedriver/run_buildbot_steps.py

Index: chrome/test/chromedriver/run_buildbot_steps.py
diff --git a/chrome/test/chromedriver/run_buildbot_steps.py b/chrome/test/chromedriver/run_buildbot_steps.py
index dd1a3714713285524bc72bb0dba642bdc2ae9846..ee993febde65e4bf012a3c46782354957ff960c3 100755
--- a/chrome/test/chromedriver/run_buildbot_steps.py
+++ b/chrome/test/chromedriver/run_buildbot_steps.py
@@ -31,10 +31,11 @@ GS_SERVER_LOGS_URL = GS_CHROMEDRIVER_DATA_BUCKET + '/server_logs'
 SERVER_LOGS_LINK = (
     'http://chromedriver-data.storage.googleapis.com/server_logs')
 TEST_LOG_FORMAT = '%s_log.json'
-GS_GITHASH_TO_SVN_URL = (
+GS_GIT_LOG_URL = (
     'https://chromium.googlesource.com/chromium/src/+/%s?format=json')
 GS_SEARCH_PATTERN = (
-    r'.*git-svn-id: svn://svn.chromium.org/chrome/trunk/src@(\d+) ')
+    r'Cr-Commit-Position: refs/heads/master@{#(\d+)}')
+CR_REV_URL = 'https://cr-rev.appspot.com/_ah/api/crrev/v1/redirect/%s'
 
 SCRIPT_DIR = os.path.join(_THIS_DIR, os.pardir, os.pardir, os.pardir, os.pardir,
                           os.pardir, os.pardir, os.pardir, 'scripts')
@@ -384,8 +385,8 @@ def _CleanTmpDir():
       os.remove(file_path)
 
 
-def _GetSVNRevisionFromGitHash(snapshot_hashcode):
-  json_url = GS_GITHASH_TO_SVN_URL % snapshot_hashcode
+def _GetCommitPositionFromGitHash(snapshot_hashcode):
+  json_url = GS_GIT_LOG_URL % snapshot_hashcode
   try:
     response = urllib2.urlopen(json_url)
   except urllib2.HTTPError as error:
@@ -406,6 +407,22 @@ def _GetSVNRevisionFromGitHash(snapshot_hashcode):
   return None
 
 
+def _GetGitHashFromCommitPosition(commit_position):
+  json_url = CR_REV_URL % commit_position
+  try:
+    response = urllib2.urlopen(json_url)
+  except urllib2.HTTPError as error:
+    util.PrintAndFlush('HTTP Error %d' % error.getcode())

[stgao, 2014/08/25 20:41:27]

Why no return?

[samuong, 2014/08/25 20:51:11]

I'm not sure. I was blindly copying and pasting code from
_GetCommitPositionFromGitHash(), which didn't have a return there either. Looks
like this was an existing bug, so I've fixed both functions.

+  except urllib2.URLError as error:
+    util.PrintAndFlush('URL Error %s' % error.message)
+    return None
+  data = json.loads(response.read())
+  if 'git_sha' in data:
+    return data['git_sha']
+  util.PrintAndFlush('Failed to get git hash for %s' % commit_position)
+  return None
+
+
 def _WaitForLatestSnapshot(revision):
   util.MarkBuildStepStart('wait_for_snapshot')
   def _IsRevisionNumber(revision):
@@ -415,10 +432,8 @@ def _WaitForLatestSnapshot(revision):
       return revision.isdigit()
   while True:
     snapshot_revision = archive.GetLatestSnapshotVersion()
-    if not _IsRevisionNumber(revision):
-      revision = _GetSVNRevisionFromGitHash(revision)
     if not _IsRevisionNumber(snapshot_revision):
-      snapshot_revision = _GetSVNRevisionFromGitHash(snapshot_revision)
+      snapshot_revision = _GetCommitPositionFromGitHash(snapshot_revision)
     if revision is not None and snapshot_revision is not None:
       if int(snapshot_revision) >= int(revision):
         break
@@ -464,7 +479,7 @@ def main():
       help=('Comma separated list of application package names, '
             'if running tests on Android.'))
   parser.add_option(
-      '-r', '--revision', type='int', help='Chromium revision')
+      '-r', '--revision', help='Chromium revision')
   parser.add_option(
       '', '--update-log', action='store_true',
       help='Update the test results log (only applicable to Android)')
@@ -479,6 +494,13 @@ def main():
 
   _CleanTmpDir()
 
+  if not options.revision:
+    commit_position = None

[stgao, 2014/08/25 20:41:27]

In this case, I think we should fail the test. Because we don't know which
commit position we should wait for the snapshot, right?

[samuong, 2014/08/25 20:51:11]

I was trying to maintain the script's original behavior. There's a case where if
the platform is android and --update-log is not passed, we're allowed to have no
--revision.

In this case it will download the prebuilt chromedriver and test it against the
chrome build that is installed on the device (so there's no snapshot to wait
for).

If we're doing a non-android build, there are still checks to make sure that a
revision is supplied.

[stgao, 2014/08/25 23:17:12]

OK. Sound good.

+  elif options.revision.isdigit():
+    commit_position = options.revision
+  else:
+    commit_position = _GetCommitPositionFromGitHash(options.revision)
+
   if platform == 'android':
     if not options.revision and options.update_log:
       parser.error('Must supply a --revision with --update-log')
@@ -487,8 +509,8 @@ def main():
     if not options.revision:
       parser.error('Must supply a --revision')
     if platform == 'linux64':
-      _ArchivePrebuilts(options.revision)
-    _WaitForLatestSnapshot(options.revision)
+      _ArchivePrebuilts(commit_position)
+    _WaitForLatestSnapshot(commit_position)
 
   _AddToolsToPath(platform)
 
@@ -506,9 +528,9 @@ def main():
   if platform == 'android':
     if options.update_log:
       util.MarkBuildStepStart('update test result log')
-      _UpdateTestResultsLog(platform, options.revision, passed)
+      _UpdateTestResultsLog(platform, commit_position, passed)
   elif passed:
-    _ArchiveGoodBuild(platform, options.revision)
+    _ArchiveGoodBuild(platform, commit_position)
     _MaybeRelease(platform)
 
   if not passed: