Bug 1258539 - [mozharness] Refactor name and arguments of download and unpack methods. r=jlund

Given that we have a universal unpack method now do not keep 'unzip' in method names.
Also adapt arguments to be better understandable.

MozReview-Commit-ID: ClDB5mSVcI2

--HG--
extra : rebase_source : a98bb26748536115d254842df8257ba050ec8eac

testing/mozharness/mozharness/base/script.py

@@ -458,25 +458,28 @@ class ScriptMixin(PlatformMixin):
             **retry_args
         )
 
-    def download_unzip(self, url, parent_dir, target_unzip_dirs=None):
-        """Generic method to download and extract a zip file.
+    def download_unpack(self, url, extract_to, extract_dirs=None,
+                        error_level=FATAL):
+        """Generic method to download and extract a compressed file.
 
         The downloaded file will always be saved to the working directory and is not getting
         deleted after extracting.
 
         Args:
             url (str): URL where the file to be downloaded is located.
-            parent_dir (str): directory where the downloaded file will
+            extract_to (str): directory where the downloaded file will
                               be extracted to.
-            target_unzip_dirs (list, optional): directories inside the zip file to extract.
-                                                Defaults to `None`.
+            extract_dirs (list, optional): directories inside the archive to extract.
+                                           Defaults to `None`.
+            error_level (str, optional): log level to use in case an error occurs.
+                                         Defaults to `FATAL`.
 
         """
         dirs = self.query_abs_dirs()
-        zipfile = self.download_file(url, parent_dir=dirs['abs_work_dir'],
-                                     error_level=FATAL)
-
-        self.unpack(zipfile, parent_dir, target_unzip_dirs)
+        archive = self.download_file(url, parent_dir=dirs['abs_work_dir'],
+                                     error_level=error_level)
+        self.unpack(archive, extract_to, extract_dirs=extract_dirs,
+                    error_level=error_level)
 
     def load_json_url(self, url, error_level=None, *args, **kwargs):
         """ Returns a json object from a url (it retries). """
@@ -1389,18 +1392,14 @@ class ScriptMixin(PlatformMixin):
         os.utime(file_name, times)
 
     def unpack(self, filename, extract_to, extract_dirs=None,
-               error_level=ERROR, halt_on_failure=True, fatal_exit_code=2,
-               verbose=False):
-        """
-        This method allows us to extract a file regardless of its extension
+               error_level=ERROR, fatal_exit_code=2, verbose=False):
+        """The method allows to extract a file regardless of its extension.
 
         Args:
             filename (str): filename of the compressed file.
             extract_to (str): where to extract the compressed file.
             extract_dirs (list, optional): directories inside the archive file to extract.
                                            Defaults to `None`.
-            halt_on_failure (bool, optional): whether or not to redefine the
-                                              log level as `FATAL` on errors. Defaults to True.
             fatal_exit_code (int, optional): call `self.fatal` if the return value
               of the command is not in `success_codes`. Defaults to 2.
             verbose (bool, optional): whether or not extracted content should be displayed.
@@ -1419,8 +1418,6 @@ class ScriptMixin(PlatformMixin):
         if not os.path.isfile(filename):
             raise IOError('Could not find file to extract: %s' % filename)
 
-        level = FATAL if halt_on_failure else error_level
-
         if zipfile.is_zipfile(filename):
             try:
                 self.info('Using ZipFile to extract {} to {}'.format(filename, extract_to))
@@ -1440,7 +1437,7 @@ class ScriptMixin(PlatformMixin):
                             os.chmod(fname, mode)
             except zipfile.BadZipfile as e:
                 self.log('%s (%s)' % (e.message, filename),
-                         level=level, exit_code=fatal_exit_code)
+                         level=error_level, exit_code=fatal_exit_code)
 
         # Bug 1211882 - is_tarfile cannot be trusted for dmg files
         elif tarfile.is_tarfile(filename) and not filename.lower().endswith('.dmg'):
@@ -1453,10 +1450,10 @@ class ScriptMixin(PlatformMixin):
                         bundle.extract(entry, path=extract_to)
             except tarfile.TarError as e:
                 self.log('%s (%s)' % (e.message, filename),
-                         level=level, exit_code=fatal_exit_code)
+                         level=error_level, exit_code=fatal_exit_code)
         else:
             self.log('No extraction method found for: %s' % filename,
-                     level=level, exit_code=fatal_exit_code)
+                     level=error_level, exit_code=fatal_exit_code)
 
 
 def PreScriptRun(func):

testing/mozharness/mozharness/mozilla/testing/firefox_media_tests.py

@@ -163,15 +163,14 @@ class FirefoxMediaTestsBase(TestingMixin, VCSToolsScript):
         harness, puppeteer, and tests from and how to set them up.
 
         """
-        target_unzip_dirs = ['config/*',
-                             'external-media-tests/*',
-                             'marionette/*',
-                             'mozbase/*',
-                             'puppeteer/*',
-                             'tools/wptserve/*',
-                             ]
-        super(FirefoxMediaTestsBase, self).download_and_extract(
-                target_unzip_dirs=target_unzip_dirs)
+        extract_dirs = ['config/*',
+                        'external-media-tests/*',
+                        'marionette/*',
+                        'mozbase/*',
+                        'puppeteer/*',
+                        'tools/wptserve/*',
+                        ]
+        super(FirefoxMediaTestsBase, self).download_and_extract(extract_dirs=extract_dirs)
 
     def query_abs_dirs(self):
         if self.abs_dirs:

testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py

@@ -153,14 +153,14 @@ class FirefoxUITests(TestingMixin, VCSToolsScript):
         harness, puppeteer, and tests from and how to set them up.
 
         """
-        target_unzip_dirs = ['config/*',
-                             'firefox-ui/*',
-                             'marionette/*',
-                             'mozbase/*',
-                             'puppeteer/*',
-                             'tools/wptserve/*',
-                             ]
-        super(FirefoxUITests, self).download_and_extract(target_unzip_dirs=target_unzip_dirs)
+        extract_dirs = ['config/*',
+                        'firefox-ui/*',
+                        'marionette/*',
+                        'mozbase/*',
+                        'puppeteer/*',
+                        'tools/wptserve/*',
+                        ]
+        super(FirefoxUITests, self).download_and_extract(extract_dirs=extract_dirs)
 
     def query_abs_dirs(self):
         if self.abs_dirs:

testing/mozharness/mozharness/mozilla/testing/talos.py

@@ -297,7 +297,7 @@ class Talos(TestingMixin, MercurialScript, BlobUploadMixin):
     # clobber defined in BaseScript
     # read_buildbot_config defined in BuildbotMixin
 
-    def download_and_extract(self, target_unzip_dirs=None, suite_categories=None):
+    def download_and_extract(self, extract_dirs=None, suite_categories=None):
         return super(Talos, self).download_and_extract(
             suite_categories=['common', 'talos']
         )

testing/mozharness/mozharness/mozilla/testing/testbase.py

@@ -418,7 +418,7 @@ You can set this by:
                   pprint.pformat(package_requirements))
         return package_requirements
 
-    def _download_test_packages(self, suite_categories, target_unzip_dirs):
+    def _download_test_packages(self, suite_categories, extract_dirs):
         # Some platforms define more suite categories/names than others.
         # This is a difference in the convention of the configs more than
         # to how these tests are run, so we pave over these differences here.
@@ -454,21 +454,21 @@ You can set this by:
                       (target_packages, category))
             for file_name in target_packages:
                 target_dir = test_install_dir
-                unzip_dirs = target_unzip_dirs
+                unpack_dirs = extract_dirs
                 if "jsshell-" in file_name or file_name == "target.jsshell.zip":
                     self.info("Special-casing the jsshell zip file")
-                    unzip_dirs = None
+                    unpack_dirs = None
                     target_dir = dirs['abs_test_bin_dir']
                 url = self.query_build_dir_url(file_name)
-                self.download_unzip(url, target_dir,
-                                     target_unzip_dirs=unzip_dirs)
+                self.download_unpack(url, target_dir,
+                                     extract_dirs=unpack_dirs)
 
-    def _download_test_zip(self, target_unzip_dirs=None):
+    def _download_test_zip(self, extract_dirs=None):
         dirs = self.query_abs_dirs()
         test_install_dir = dirs.get('abs_test_install_dir',
                                     os.path.join(dirs['abs_work_dir'], 'tests'))
-        self.download_unzip(self.test_url, test_install_dir,
-                             target_unzip_dirs=target_unzip_dirs)
+        self.download_unpack(self.test_url, test_install_dir,
+                             extract_dirs=extract_dirs)
 
     def structured_output(self, suite_category):
         """Defines whether structured logging is in use in this configuration. This
@@ -515,9 +515,9 @@ You can set this by:
 
         self.set_buildbot_property("symbols_url", self.symbols_url,
                                    write_to_file=True)
-        self.download_unzip(self.symbols_url, self.symbols_path)
+        self.download_unpack(self.symbols_url, self.symbols_path)
 
-    def download_and_extract(self, target_unzip_dirs=None, suite_categories=None):
+    def download_and_extract(self, extract_dirs=None, suite_categories=None):
         """
         download and extract test zip / download installer
         """
@@ -540,7 +540,7 @@ You can set this by:
                            ' package data at "%s" will be ignored.' %
                            (self.config.get('test_url'), self.test_packages_url))
 
-            self._download_test_zip(target_unzip_dirs)
+            self._download_test_zip(extract_dirs)
         else:
             if not self.test_packages_url:
                 # The caller intends to download harness specific packages, but doesn't know
@@ -550,7 +550,7 @@ You can set this by:
                 self.test_packages_url = self.query_prefixed_build_dir_url('.test_packages.json')
 
             suite_categories = suite_categories or ['common']
-            self._download_test_packages(suite_categories, target_unzip_dirs)
+            self._download_test_packages(suite_categories, extract_dirs)
 
         self._download_installer()
         if self.config.get('download_symbols'):

testing/mozharness/mozharness/mozilla/testing/unittest.py

@@ -235,7 +235,7 @@ class EmulatorMixin(object):
         dirs = self.query_abs_dirs()
         self.mkdir_p(dirs['abs_emulator_dir'])
         if self.config.get('emulator_url'):
-            self.download_unzip(self.config['emulator_url'], dirs['abs_emulator_dir'])
+            self.download_unpack(self.config['emulator_url'], dirs['abs_emulator_dir'])
         elif self.config.get('emulator_manifest'):
             manifest_path = self.create_tooltool_manifest(self.config['emulator_manifest'])
             do_unzip = True

testing/mozharness/scripts/b2g_emulator_unittest.py

@@ -211,8 +211,7 @@ class B2GEmulatorTest(TestingMixin, VCSMixin, BaseScript, BlobUploadMixin):
                          error_list=TarErrorList,
                          halt_on_failure=True, fatal_exit_code=3)
 
-        self.download_unzip(self.config['xre_url'],
-                             dirs['abs_xre_dir'])
+        self.download_unpack(self.config['xre_url'], dirs['abs_xre_dir'])
 
         if self.config.get('busybox_url'):
             self.download_file(self.config['busybox_url'],

testing/mozharness/scripts/desktop_unittest.py

@@ -480,20 +480,20 @@ class DesktopUnittest(TestingMixin, MercurialScript, BlobUploadMixin, MozbaseMix
         """
         c = self.config
 
-        target_unzip_dirs = None
+        extract_dirs = None
         if c['specific_tests_zip_dirs']:
-            target_unzip_dirs = list(c['minimum_tests_zip_dirs'])
+            extract_dirs = list(c['minimum_tests_zip_dirs'])
             for category in c['specific_tests_zip_dirs'].keys():
                 if c['run_all_suites'] or self._query_specified_suites(category) \
                         or 'run-tests' not in self.actions:
-                    target_unzip_dirs.extend(c['specific_tests_zip_dirs'][category])
+                    extract_dirs.extend(c['specific_tests_zip_dirs'][category])
 
         if c.get('run_all_suites'):
             target_categories = SUITE_CATEGORIES
         else:
             target_categories = [cat for cat in SUITE_CATEGORIES
                                  if self._query_specified_suites(cat) is not None]
-        super(DesktopUnittest, self).download_and_extract(target_unzip_dirs=target_unzip_dirs,
+        super(DesktopUnittest, self).download_and_extract(extract_dirs=extract_dirs,
                                                           suite_categories=target_categories)
 
     def stage_files(self):

testing/mozharness/scripts/web_platform_tests.py

@@ -166,12 +166,12 @@ class WebPlatformTest(TestingMixin, MercurialScript, BlobUploadMixin):
 
     def download_and_extract(self):
         super(WebPlatformTest, self).download_and_extract(
-            target_unzip_dirs=["bin/*",
-                               "config/*",
-                               "mozbase/*",
-                               "marionette/*",
-                               "tools/wptserve/*",
-                               "web-platform/*"],
+            extract_dirs=["bin/*",
+                          "config/*",
+                          "mozbase/*",
+                          "marionette/*",
+                          "tools/wptserve/*",
+                          "web-platform/*"],
             suite_categories=["web-platform"])
 
     def run_tests(self):