Back out changeset c9dbc1119342 (bug 1203085) on suspicion of breaking gaia tests
authorPhil Ringnalda <philringnalda@gmail.com>
Fri, 18 Sep 2015 23:50:56 -0700
changeset 295979 a5b1224e416a42797c9932ec03e13ae2ddb26c75
parent 295978 07711f754650f049206413f2c7a3f4adc6cc0ab6
child 295980 1083dfc4f0eef3d33f6d2f385088a9cef417f970
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1203085
milestone43.0a1
backs outc9dbc1119342145b08883bb85566a270a3081b48
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Back out changeset c9dbc1119342 (bug 1203085) on suspicion of breaking gaia tests
testing/mozharness/mozharness/base/script.py
testing/mozharness/mozharness/mozilla/taskcluster_helper.py
testing/mozharness/mozharness/mozilla/testing/gaia_test.py
testing/mozharness/mozharness/mozilla/testing/testbase.py
testing/mozharness/scripts/desktop_l10n.py
--- a/testing/mozharness/mozharness/base/script.py
+++ b/testing/mozharness/mozharness/base/script.py
@@ -385,24 +385,22 @@ class ScriptMixin(PlatformMixin):
             raise
         except socket.timeout, e:
             self.warning("Timed out accessing %s: %s" % (url, str(e)))
             raise
         except socket.error, e:
             self.warning("Socket error when accessing %s: %s" % (url, str(e)))
             raise
 
-    def _retry_download(self, url, error_level, file_name=None, retry_config=None):
-        """ Helper method to retry download methods
+    def _retry_download_file(self, url, file_name, error_level, retry_config=None):
+        """ Helper method to retry _download_file().
         Split out so we can alter the retry logic in mozharness.mozilla.testing.gaia_test.
 
         This method calls `self.retry` on `self._download_file` using the passed
-        parameters if a file_name is specified. If no file is specified, we will
-        instead call `self._urlopen`, which grabs the contents of a url but does
-        not create a file on disk.
+        parameters.
 
         Args:
             url (str): URL path where the file is located.
             file_name (str): file_name where the file will be written to.
             error_level (str): log level to use in case an error occurs.
             retry_config (dict, optional): key-value pairs to be passed to
                                            `self.retry`. Defaults to `None`
 
@@ -418,35 +416,22 @@ class ScriptMixin(PlatformMixin):
                               socket.timeout, socket.error),
             error_message="Can't download from %s to %s!" % (url, file_name),
             error_level=error_level,
         )
 
         if retry_config:
             retry_args.update(retry_config)
 
-        download_func = self._urlopen
-        kwargs = {"url": url}
-        if file_name:
-            download_func = self._download_file
-            kwargs = {"url": url, "file_name": file_name}
-
         return self.retry(
-            download_func,
-            kwargs=kwargs,
+            self._download_file,
+            args=(url, file_name),
             **retry_args
         )
 
-    def load_json_url(self, url, error_level=None, *args, **kwargs):
-        """ Returns a json object from a url (it retries). """
-        contents = self._retry_download(
-            url=url, error_level=error_level, *args, **kwargs
-        )
-        return json.loads(contents.read())
-
     # http://www.techniqal.com/blog/2008/07/31/python-file-read-write-with-urllib2/
     # TODO thinking about creating a transfer object.
     def download_file(self, url, file_name=None, parent_dir=None,
                       create_parent_dir=True, error_level=ERROR,
                       exit_code=3, retry_config=None):
         """ Python wget.
         Download the filename at `url` into `file_name` and put it on `parent_dir`.
         On error log with the specified `error_level`, on fatal exit with `exit_code`.
@@ -477,22 +462,17 @@ class ScriptMixin(PlatformMixin):
                 self.log("Unable to get filename from %s; bad url?" % url,
                          level=error_level, exit_code=exit_code)
                 return
         if parent_dir:
             file_name = os.path.join(parent_dir, file_name)
             if create_parent_dir:
                 self.mkdir_p(parent_dir, error_level=error_level)
         self.info("Downloading %s to %s" % (url, file_name))
-        status = self._retry_download(
-            url=url,
-            error_level=error_level,
-            file_name=file_name,
-            retry_config=retry_config
-        )
+        status = self._retry_download_file(url, file_name, error_level, retry_config=retry_config)
         if status == file_name:
             self.info("Downloaded %d bytes." % os.path.getsize(file_name))
         return status
 
     def move(self, src, dest, log_level=INFO, error_level=ERROR,
              exit_code=-1):
         """ recursively move a file or directory (src) to another location (dest).
 
--- a/testing/mozharness/mozharness/mozilla/taskcluster_helper.py
+++ b/testing/mozharness/mozharness/mozilla/taskcluster_helper.py
@@ -1,15 +1,13 @@
 """Taskcluster module. Defines a few helper functions to call into the taskcluster
    client.
 """
 import os
 from datetime import datetime, timedelta
-from urlparse import urljoin
-
 from mozharness.base.log import LogMixin
 
 
 # Taskcluster {{{1
 class Taskcluster(LogMixin):
     """
     Helper functions to report data to Taskcluster
     """
@@ -109,70 +107,8 @@ class Taskcluster(LogMixin):
                 "success": True,
             })
 
     def get_taskcluster_url(self, filename):
         return 'https://queue.taskcluster.net/v1/task/%s/artifacts/public/build/%s' % (
             self.task_id,
             os.path.basename(filename)
         )
-
-
-# TasckClusterArtifactFinderMixin {{{1
-class TaskClusterArtifactFinderMixin(object):
-    # This class depends that you have extended from the base script
-    QUEUE_URL = 'https://queue.taskcluster.net/v1/task/'
-    SCHEDULER_URL = 'https://scheduler.taskcluster.net/v1/task-graph/'
-
-    def get_task(self, task_id):
-        """ Get Task Definition """
-        # Signature: task(taskId) : result
-        return self.load_json_url(urljoin(self.QUEUE_URL, task_id))
-
-    def get_list_latest_artifacts(self, task_id):
-        """ Get Artifacts from Latest Run """
-        # Signature: listLatestArtifacts(taskId) : result
-
-        # Notice that this grabs the most recent run of a task since we don't
-        # know the run_id. This slightly slower, however, it is more convenient
-        return self.load_json_url(urljoin(self.QUEUE_URL, '{}/artifacts'.format(task_id)))
-
-    def url_to_artifact(self, task_id, full_path):
-        """ Return a URL for an artifact. """
-        return urljoin(self.QUEUE_URL, '{}/artifacts/{}'.format(task_id, full_path))
-
-    def get_inspect_graph(self, task_group_id):
-        """ Inspect Task Graph """
-        # Signature: inspect(taskGraphId) : result
-        return self.load_json_url(urljoin(self.SCHEDULER_URL, '{}/inspect'.format(task_group_id)))
-
-    def find_parent_task_id(self, task_id):
-        """ Returns the task_id of the parent task associated to the given task_id."""
-        # Find group id to associated to all related tasks
-        task_group_id = self.get_task(task_id)['taskGroupId']
-
-        # Find child task and determine on which task it depends on
-        for task in self.get_inspect_graph(task_group_id)['tasks']:
-            if task['taskId'] == task_id:
-                parent_task_id = task['requires'][0]
-
-        return parent_task_id
-
-    def set_artifacts(self, task_id):
-        """ Sets installer, test and symbols URLs from the artifacts of a task.
-
-        In this case we set:
-            self.installer_url
-            self.test_url (points to test_packages.json)
-            self.symbols_url
-        """
-        # The tasks which represent a buildbot job only uploads one artifact:
-        # the properties.json file
-        p = self.load_json_url(
-            self.url_to_artifact(task_id, 'public/properties.json'))
-
-        # Set importants artifacts for test jobs
-        self.installer_url = p['packageUrl'][0] if p.get('packageUrl') else None
-        self.test_url = p['testPackagesUrl'][0] if p.get('testPackagesUrl') else None
-        self.symbols_url = p['symbolsUrl'][0] if p.get('symbolsUrl') else None
-
-    def set_parent_artifacts(self, child_task_id):
-        self.set_artifacts(self.find_parent_task_id(child_task_id))
--- a/testing/mozharness/mozharness/mozilla/testing/gaia_test.py
+++ b/testing/mozharness/mozharness/mozilla/testing/gaia_test.py
@@ -169,17 +169,17 @@ class GaiaTest(MercurialScript, TestingM
             # 'proxxy' element. If 'proxxy' is not defined it uses PROXXY_CONFIG
             # For GaiaTest, if there's no proxxy element, don't use a proxxy at
             # all. To do this we must pass a special configuraion
             proxxy_conf = {'proxxy': self.config.get('proxxy', {})}
             proxxy = Proxxy(proxxy_conf, self.log_obj)
             self.proxxy = proxxy
         return self.proxxy
 
-    def _retry_download(self, url, file_name, error_level=FATAL, retry_config=None):
+    def _retry_download_file(self, url, file_name, error_level=FATAL, retry_config=None):
         if self.config.get("bypass_download_cache"):
             n = 0
             # ignore retry_config in this case
             max_attempts = 5
             sleeptime = 60
 
             while n < max_attempts:
                 n += 1
@@ -191,19 +191,17 @@ class GaiaTest(MercurialScript, TestingM
                 except Exception:
                     if n >= max_attempts:
                         self.log("Can't download from %s to %s!" % (url, file_name),
                                  level=error_level, exit_code=3)
                         return None
                     self.info("Sleeping %s before retrying..." % sleeptime)
                     time.sleep(sleeptime)
         else:
-            # Since we're overwritting _retry_download() we can't call download_file()
-            # directly
-            return super(GaiaTest, self)._retry_download(
+            return super(GaiaTest, self)._retry_download_file(
                 url, file_name, error_level, retry_config=retry_config,
             )
 
     def run_tests(self):
         """
         Run the test suite.
         """
         pass
--- a/testing/mozharness/mozharness/mozilla/testing/testbase.py
+++ b/testing/mozharness/mozharness/mozilla/testing/testbase.py
@@ -19,17 +19,16 @@ from mozharness.base.log import FATAL, W
 from mozharness.base.python import (
     ResourceMonitoringMixin,
     VirtualenvMixin,
     virtualenv_config_options,
 )
 from mozharness.mozilla.buildbot import BuildbotMixin, TBPL_WARNING
 from mozharness.mozilla.proxxy import Proxxy
 from mozharness.mozilla.structuredlog import StructuredOutputParser
-from mozharness.mozilla.taskcluster_helper import TaskClusterArtifactFinderMixin
 from mozharness.mozilla.testing.unittest import DesktopUnittestOutputParser
 from mozharness.mozilla.testing.try_tools import TryToolsMixin
 from mozharness.mozilla.tooltool import TooltoolMixin
 
 from mozharness.lib.python.authentication import get_credentials
 
 INSTALLER_SUFFIXES = ('.tar.bz2', '.zip', '.dmg', '.exe', '.apk', '.tar.gz')
 
@@ -82,18 +81,18 @@ testing_config_options = [
      "type": "choice",
      "choices": ['ondemand', 'true'],
      "help": "Download and extract crash reporter symbols.",
       }],
 ] + copy.deepcopy(virtualenv_config_options)
 
 
 # TestingMixin {{{1
-class TestingMixin(VirtualenvMixin, BuildbotMixin, ResourceMonitoringMixin,
-                   TaskClusterArtifactFinderMixin, TooltoolMixin, TryToolsMixin):
+class TestingMixin(VirtualenvMixin, BuildbotMixin, ResourceMonitoringMixin, TooltoolMixin,
+                   TryToolsMixin):
     """
     The steps to identify + download the proper bits for [browser] unit
     tests and Talos.
     """
 
     installer_url = None
     installer_path = None
     binary_path = None
@@ -192,21 +191,20 @@ class TestingMixin(VirtualenvMixin, Buil
         def _replace_url(url, changes):
             for from_, to_ in changes:
                 if url.startswith(from_):
                     new_url = url.replace(from_, to_)
                     self.info("Replacing url %s -> %s" % (url, new_url))
                     return new_url
             return url
 
-        if c.get("installer_url") is None:
-            self.exception("You must use --installer-url with developer_config.py")
+        assert c["installer_url"], "You must use --installer-url with developer_config.py"
         if c.get("require_test_zip"):
             if not c.get('test_url') and not c.get('test_packages_url'):
-                self.exception("You must use --test-url or --test-packages-url with developer_config.py")
+                raise AssertionError("You must use --test-url or --test-packages-url with developer_config.py")
 
         c["installer_url"] = _replace_url(c["installer_url"], c["replace_urls"])
         if c.get("test_url"):
             c["test_url"] = _replace_url(c["test_url"], c["replace_urls"])
         if c.get("test_packages_url"):
             c["test_packages_url"] = _replace_url(c["test_packages_url"], c["replace_urls"])
 
         for key, value in self.config.iteritems():
@@ -243,83 +241,64 @@ class TestingMixin(VirtualenvMixin, Buil
         # URLs to the right place and enable http authentication
         if "developer_config.py" in self.config["config_files"]:
             return _urlopen_basic_auth(url, **kwargs)
         else:
             return urllib2.urlopen(url, **kwargs)
 
     # read_buildbot_config is in BuildbotMixin.
 
-    def find_artifacts_from_buildbot_changes(self):
-        c = self.config
-        try:
-            files = self.buildbot_config['sourcestamp']['changes'][-1]['files']
-            buildbot_prop_branch = self.buildbot_config['properties']['branch']
-
-            # Bug 868490 - Only require exactly two files if require_test_zip;
-            # otherwise accept either 1 or 2, since we'll be getting a
-            # test_zip url that we don't need.
-            expected_length = [1, 2, 3]
-            if c.get("require_test_zip") and not self.test_url:
-                expected_length = [2, 3]
-            if buildbot_prop_branch.startswith('gaia-try'):
-                expected_length = range(1, 1000)
-            actual_length = len(files)
-            if actual_length not in expected_length:
-                self.fatal("Unexpected number of files in buildbot config %s.\nExpected these number(s) of files: %s, but got: %d" %
-                           (c['buildbot_json_path'], str(expected_length), actual_length))
-            for f in files:
-                if f['name'].endswith('tests.zip'):  # yuk
-                    if not self.test_url:
-                        # str() because of unicode issues on mac
-                        self.test_url = str(f['name'])
-                        self.info("Found test url %s." % self.test_url)
-                elif f['name'].endswith('crashreporter-symbols.zip'):  # yuk
-                    self.symbols_url = str(f['name'])
-                    self.info("Found symbols url %s." % self.symbols_url)
-                elif f['name'].endswith('test_packages.json'):
-                    self.test_packages_url = str(f['name'])
-                    self.info("Found a test packages url %s." % self.test_packages_url)
-                elif not any(f['name'].endswith(s) for s in ('code-coverage-gcno.zip',)):
-                    if not self.installer_url:
-                        self.installer_url = str(f['name'])
-                        self.info("Found installer url %s." % self.installer_url)
-        except IndexError, e:
-            self.error(str(e))
-
-    def find_artifacts_from_taskcluster(self):
-        self.info("Finding installer, test and symbols from parent task. ")
-        task_id = self.buildbot_config['properties']['taskId']
-        self.set_parent_artifacts(task_id)
-
     def postflight_read_buildbot_config(self):
         """
         Determine which files to download from the buildprops.json file
         created via the buildbot ScriptFactory.
         """
         if self.buildbot_config:
             c = self.config
             message = "Unable to set %s from the buildbot config"
             if c.get("installer_url"):
                 self.installer_url = c['installer_url']
             if c.get("test_url"):
                 self.test_url = c['test_url']
             if c.get("test_packages_url"):
                 self.test_packages_url = c['test_packages_url']
+            try:
+                files = self.buildbot_config['sourcestamp']['changes'][-1]['files']
+                buildbot_prop_branch = self.buildbot_config['properties']['branch']
 
-            if self.buildbot_config['sourcestamp']['changes']:
-                self.find_artifacts_from_buildbot_changes()
-            elif 'taskId' in self.buildbot_config['properties']:
-                self.find_artifacts_from_taskcluster()
-            else:
-                self.exception(
-                    "We have not been able to determine which artifacts "
-                    "to use in order to run the tests."
-                )
-
+                # Bug 868490 - Only require exactly two files if require_test_zip;
+                # otherwise accept either 1 or 2, since we'll be getting a
+                # test_zip url that we don't need.
+                expected_length = [1, 2, 3]
+                if c.get("require_test_zip") and not self.test_url:
+                    expected_length = [2, 3]
+                if buildbot_prop_branch.startswith('gaia-try'):
+                    expected_length = range(1, 1000)
+                actual_length = len(files)
+                if actual_length not in expected_length:
+                    self.fatal("Unexpected number of files in buildbot config %s.\nExpected these number(s) of files: %s, but got: %d" %
+                               (c['buildbot_json_path'], str(expected_length), actual_length))
+                for f in files:
+                    if f['name'].endswith('tests.zip'):  # yuk
+                        if not self.test_url:
+                            # str() because of unicode issues on mac
+                            self.test_url = str(f['name'])
+                            self.info("Found test url %s." % self.test_url)
+                    elif f['name'].endswith('crashreporter-symbols.zip'):  # yuk
+                        self.symbols_url = str(f['name'])
+                        self.info("Found symbols url %s." % self.symbols_url)
+                    elif f['name'].endswith('test_packages.json'):
+                        self.test_packages_url = str(f['name'])
+                        self.info("Found a test packages url %s." % self.test_packages_url)
+                    elif not any(f['name'].endswith(s) for s in ('code-coverage-gcno.zip',)):
+                        if not self.installer_url:
+                            self.installer_url = str(f['name'])
+                            self.info("Found installer url %s." % self.installer_url)
+            except IndexError, e:
+                self.error(str(e))
             missing = []
             if not self.installer_url:
                 missing.append("installer_url")
             if c.get("require_test_zip") and not self.test_url and not self.test_packages_url:
                 missing.append("test_url")
             if missing:
                 self.fatal("%s!" % (message % ('+'.join(missing))))
         else:
--- a/testing/mozharness/scripts/desktop_l10n.py
+++ b/testing/mozharness/scripts/desktop_l10n.py
@@ -692,18 +692,17 @@ class DesktopSingleLocale(LocalesMixin, 
             # specified EN_US_BINARY url is an installer file...
             dst_filename = binary_file.split('/')[-1].strip()
             dst_filename = os.path.join(dirs['abs_objdir'], 'dist', dst_filename)
             # we need to set ZIP_IN so make unpack finds this binary file.
             # Please note this is required only if the en-us-binary-url provided
             # has a different version number from the one in the current
             # checkout.
             self.bootstrap_env['ZIP_IN'] = dst_filename
-            return self.download_file(url=binary_file, file_name=dst_filename,
-                                      error_level=FATAL)
+            return self._retry_download_file(binary_file, dst_filename, error_level=FATAL)
 
         # binary url is not an installer, use make wget-en-US to download it
         return self._make(target=["wget-en-US"], cwd=cwd, env=env)
 
     def make_upload(self, locale):
         """wrapper for make upload command"""
         config = self.config
         env = self.query_l10n_env()