[Android] Enable uploading instrumentation tests to flakiness dashboard.

build/android/pylib/utils/flakiness_dashboard_results_uploader.py

 # Copyright (c) 2012 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.
 
 """Uploads the results to the flakiness dashboard server."""
 
 import logging
 import os
 import shutil
 import subprocess
 import sys
 import tempfile
 
-sys.path.append(os.path.join(sys.path[0], '..', '..', 'third_party',
-                             'WebKit', 'Tools', 'Scripts'))
+sys.path.append(os.path.join(os.path.dirname(os.path.realpath( __file__ )),

[Isaac (away), 2013/01/14 19:59:44]

Nit: put abspath outside of os.path.join so it resolves the '..'

 realpath probably not necessary since it resolves symlinks. 

[frankf, 2013/01/14 22:54:42]

Done.

+                             os.pardir, os.pardir, os.pardir, os.pardir,
+                             'third_party', 'WebKit', 'Tools', 'Scripts'))
 from webkitpy.common.system import executive, filesystem
 from webkitpy.layout_tests.layout_package import json_results_generator
 
+#TODO(craigdh): pylib/utils/ should not depend on pylib/.
+from pylib import constants
 
 # The JSONResultsGenerator gets the filesystem.join operation from the Port
 # object. Creating a Port object requires specifying information that only
 # makes sense for running WebKit layout tests, so we provide a dummy object
 # that contains the fields required by the generator.
 class PortDummy(object):
   def __init__(self):
     self._executive = executive.Executive()
     self._filesystem = filesystem.FileSystem()
 
@@ skipping 20 matching lines @@
 
   #override
   def _get_modifier_char(self, test_name):
     if test_name not in self._test_results_map:
       return self.__class__.NO_DATA_RESULT
 
     return self._test_results_map[test_name].modifier
 
   #override
   def _get_svn_revision(self, in_directory):
-    """Returns the git revision for the given directory.
+    """Returns the git/svn revision for the given directory.
 
     Args:
-      in_directory: The directory where git is to be run.
+      in_directory: The directory relative to src.
     """
-    git_dir =  self._filesystem.join(os.environ.get('CHROME_SRC'),
-                                     in_directory,
-                                     '.git')
-    if self._filesystem.exists(git_dir):
-      # Note: Not thread safe: http://bugs.python.org/issue2320
-      output = subprocess.Popen(
-          ['git', '--git-dir=%s' % git_dir, 'show-ref', '--head',
-           '--hash=10', 'HEAD'],
-          stdout=subprocess.PIPE).communicate()[0].strip()
-      return output
+    def _is_git_directory(in_directory):
+      """Returns true if the given directory is in a git repository.
+
+      Args:
+        in_directory: The directory path to be tested.
+      """
+      if os.path.exists(os.path.join(in_directory, '.git')):
+        return True
+      parent = os.path.dirname(in_directory)
+      if parent == constants.CHROME_DIR or parent == in_directory:
+        return False
+      return _is_git_directory(parent)

[Isaac (away), 2013/01/14 19:59:44]

We could alternatively look for the absence of a .svn folder, which is in every
directory and wouldn't require a parent traversal. 

[frankf, 2013/01/14 22:54:42]

This gives us a strong guarantee of this being a git directory. The traversal is
negligible. 

On 2013/01/14 19:59:44, Isaac wrote:

[Isaac (use chromium), 2013/01/14 23:37:27]

What about the situation where you have an svn checkout inside a gitignored
folder of a git repo?  This case could occur on buildbot slaves:

/b/build: git repo
/b/build/slave/<name>/build/src/ <chromium svn repo>

[frankf, 2013/01/14 23:58:47]

In that case, we'll encounter .svn first, and don't bother with this function.

On 2013/01/14 23:37:27, Isaac (use chromium acct) wrote:

[Isaac (use chromium), 2013/01/15 00:03:07]

SGTM

+
+    def _get_git_revision(in_directory):
+      """Returns the git hash tag for the given directory.
+
+      Args:
+        in_directory: The directory where git is to be run.
+      """
+      command_line = ['git', 'log', '-1', '--pretty=oneline']

[Isaac (away), 2013/01/14 19:59:44]

nit you can have git output just the sha with the right argument to --pretty

[frankf, 2013/01/14 22:54:42]

Done.

+      output = subprocess.Popen(command_line,
+                                cwd=in_directory,
+                                stdout=subprocess.PIPE).communicate()[0]
+      return output[0:40]
+
+    in_directory = os.path.join(constants.CHROME_DIR, in_directory)
+
+    if not os.path.exists(os.path.join(in_directory, '.svn')):
+      if _is_git_directory(in_directory):
+        return _get_git_revision(in_directory)
+      else:
+        return ''
+
+    # Note: Not thread safe: http://bugs.python.org/issue2320
+    output = subprocess.Popen(['svn', 'info', '--xml'],
+                              cwd=in_directory,
+                              stdout=subprocess.PIPE).communicate()[0]
+    try:
+      dom = xml.dom.minidom.parseString(output)
+      return dom.getElementsByTagName('entry')[0].getAttribute('revision')
+    except xml.parsers.expat.ExpatError:
+      return ''
     return ''
 
 
 class ResultsUploader(object):
   """Handles uploading buildbot tests results to the flakiness dashboard."""
   def __init__(self, tests_type):
     self._build_number = os.environ.get('BUILDBOT_BUILDNUMBER')
     self._builder_name = os.environ.get('BUILDBOT_BUILDERNAME')
     self._tests_type = tests_type
-    self._build_name = 'chromium-android'
 
-    if not self._builder_name:
+    if not self._build_number or not self._builder_name:
       raise Exception('You should not be uploading tests results to the server'
                       'from your local machine.')
 
-    buildbot_branch = os.environ.get('BUILDBOT_BRANCH')
-    if not buildbot_branch:
-      buildbot_branch = 'master'
-    self._master_name = '%s-%s' % (self._build_name, buildbot_branch)
+    upstream = (tests_type != 'Chromium_Android_Instrumentation')
+    if upstream:

[Isaac (away), 2013/01/14 19:59:44]

Would be a safer to pull this from the factory_properties json that we already
have, see bb_device_steps.py for an example of decoding for optionparser. 
Directory structure is also not reliable for builder lookup. 

[frankf, 2013/01/14 22:54:42]

Currently, the mastername is specified as:
"mastername": "chromium.fyi"

Whereas, I need the actual name "ChormiumFYI". I added a TODO here, since this
shouldn't block the CL and we can revisit this.

On 2013/01/14 19:59:44, Isaac wrote:

[Isaac (use chromium), 2013/01/14 23:37:27]

Sounds good to me, but as this increases the risk of a build repo change
breaking the bot, I would like to see this fixed before this code runs on the
main waterfall.

+      from slave import slave_utils
+      self._build_name = slave_utils.SlaveBuildName(constants.CHROME_DIR)
+      self._master_name = slave_utils.GetActiveMaster()
+    else:
+      self._build_name = 'chromium-android'
+      buildbot_branch = os.environ.get('BUILDBOT_BRANCH')
+      if not buildbot_branch:
+        buildbot_branch = 'master'
+      self._master_name = '%s-%s' % (self._build_name, buildbot_branch)
+
     self._test_results_map = {}
 
   def AddResults(self, test_results):
     conversion_map = [
         (test_results.ok, False,
             json_results_generator.JSONResultsGeneratorBase.PASS_RESULT),
         (test_results.failed, True,
             json_results_generator.JSONResultsGeneratorBase.FAIL_RESULT),
         (test_results.crashed, True,
             "C"),
@@ skipping 46 matching lines @@
   """Reports test results to the flakiness dashboard for Chrome for Android.
 
   Args:
     flakiness_dashboard_server: the server to upload the results to.
     test_type: the type of the tests (as displayed by the flakiness dashboard).
     results: test results.
   """
   uploader = ResultsUploader(test_type)
   uploader.AddResults(results)
   uploader.Upload(flakiness_dashboard_server)