Commit 787b280e authored by Gregory Mierzwinski's avatar Gregory Mierzwinski
Browse files

Bug 1594795 - Fix local browsertime issues on Windows and MacOS.... 

Bug 1594795 - Fix local browsertime issues on Windows and MacOS. r=perftest-reviewers,barret,stephendonner,davehunt

This patch fixes local `./mach browsertime` issues on Windows.

First issue that is fixed is that the archive is extracted into the browsertime folder instead of a separate folder for imagemagick within the browsertime folder. Now, it is extracted into it's own folder whose name depends on the `fetches` path entry. Second issue fixed is that the environment was being contaminated with bad strings because of the `GECKODRIVER_BASE_URL` addition - using `str` on the entries solves this issue.

Finally, convert and compare were not passing in the visualmetrics.py check call. This is because ImageMagick expects to find the path to them within `ProgramFiles` on windows. Since we don't have them there, we have to add an entry in the `PATH` environment variable to point to the imagemagick directory to get around this.

There is also an issue with the macosx imagemagick directory which was being extracted as 7.0.9, but we expected 7.0.8. The imagemagick version used is modified to the correct version with the expected directory.

Differential Revision: https://phabricator.services.mozilla.com/D52751

--HG--
extra : moz-landing-system : lando
parent 4c9531de

tools/browsertime/mach_commands.py

+18 −3
Original line number Diff line number Diff line
@@ -112,7 +112,7 @@ host_fetches = { 
            # It's sad that the macOS URLs don't include version numbers.  If

            # ImageMagick is released frequently, we'll need to be more

            # accommodating of multiple versions here.

            'url': 'https://ftp.icm.edu.pl/packages/ImageMagick/binaries/ImageMagick-x86_64-apple-darwin18.7.0.tar.gz',  # noqa

            'url': 'https://ftp.icm.edu.pl/packages/ImageMagick/binaries/ImageMagick-x86_64-apple-darwin17.7.0.tar.gz',  # noqa

            # An extension to `fetch` syntax.

            'path': 'ImageMagick-7.0.8',

        },
@@ -210,6 +210,15 @@ class MachBrowsertime(MachCommandBase): 
                        'browsertime',

                        {'path': archive},

                        'Unpacking temporary location {path}')



                    if 'win64' in host_platform() and 'imagemagick' in tool.lower():

                        # Windows archive does not contain a subfolder

                        # so we make one for it here

                        mkdir(fetch.get('path'))

                        os.chdir(os.path.join(self.state_path, fetch.get('path')))

                        unpack_file(archive)

                        os.chdir(self.state_path)

                    else:

                        unpack_file(archive)



                    # Make sure the expected path exists after extraction
@@ -243,7 +252,7 @@ class MachBrowsertime(MachCommandBase): 
        if 'GECKODRIVER_BASE_URL' not in os.environ:

            # Use custom `geckodriver` with pre-release Android support.

            url = 'https://github.com/ncalexan/geckodriver/releases/download/v0.24.0-android/'

            os.environ['GECKODRIVER_BASE_URL'] = url

            os.environ[str('GECKODRIVER_BASE_URL')] = str(url)



        self.log(

            logging.INFO,
@@ -304,6 +313,12 @@ class MachBrowsertime(MachCommandBase): 
        node_dir = os.path.dirname(node_path())

        path = [node_dir] + path



        # On windows, we need to add the ImageMagick directory to the path

        # otherwise compare won't be found, and the built-in OS convert

        # method will be used instead of the ImageMagick one.

        if 'win64' in host_platform() and path_to_imagemagick:

            path.insert(0, path_to_imagemagick)



        append_env = {

            'PATH': os.pathsep.join(path),