From d9c9e6a2483d1587bf861e01d0ee36b2055b3c09 Mon Sep 17 00:00:00 2001 From: Greg Mierzwinski Date: Wed, 22 Mar 2023 12:30:17 +0000 Subject: [PATCH] Bug 1613455 - Allow custom APK uploads for Geckoview/Fenix perftests. r=perftest-reviewers,afinder This patch allows mobile developers to upload custom APKs for testing through a commit. This allows them to run our performance tests by building locally, and then uploading to CI to run tests there. The `./mach try perf` command is modified to make this simpler. It accepts either an environment variable, or a path to an APK, and copies it in-tree. After adding it to hg, the command stops running and asks the user to commit the changes. From there the user re-runs the `./mach try perf` command to select the appropriate tests. Using --browsertime-upload-apk, users can use a custom APK for browsertime tests, and using --mozperftest-upload-apk, users can use a custom APK in mozperftest tests. The reason it's done this way is that we don't have common areas between the two frameworks. The methods are the same in both cases, i.e. for a fenix test, a fenix APK needs to be uploaded. Differential Revision: https://phabricator.services.mozilla.com/D172435 --- .../mozperftest/mozperftest/system/android.py | 25 +++- .../mozperftest/system/android_startup.py | 9 +- .../mozperftest/tests/test_android.py | 79 ++++++++++- .../mozperftest/tests/test_android_startup.py | 60 ++++++++- .../mozharness/mozilla/testing/raptor.py | 13 +- tools/tryselect/selectors/perf.py | 123 ++++++++++++++++++ tools/tryselect/test/test_perf.py | 38 ++++++ 7 files changed, 339 insertions(+), 8 deletions(-) diff --git a/python/mozperftest/mozperftest/system/android.py b/python/mozperftest/mozperftest/system/android.py index 98941e71a749..650b0fb29dd8 100644 --- a/python/mozperftest/mozperftest/system/android.py +++ b/python/mozperftest/mozperftest/system/android.py @@ -12,6 +12,8 @@ from mozperftest.layers import Layer from mozperftest.system.android_perf_tuner import tune_performance from mozperftest.utils import download_file +HERE = Path(__file__).parent + _ROOT_URL = "https://firefox-ci-tc.services.mozilla.com/api/index/v1/task/" _FENIX_NIGHTLY_BUILDS = ( "mobile.v3.firefox-android.apks.fenix-nightly.latest.{architecture}" @@ -117,9 +119,26 @@ class AndroidDevice(Layer): super(AndroidDevice, self).__init__(env, mach_cmd) self.android_activity = self.app_name = self.device = None self.capture_logcat = self.capture_file = None + self._custom_apk_path = None + + @property + def custom_apk_path(self): + if self._custom_apk_path is None: + custom_apk_path = Path(HERE, "..", "user_upload.apk") + if custom_apk_path.exists(): + self._custom_apk_path = custom_apk_path + return self._custom_apk_path + + def custom_apk_exists(self): + return self.custom_apk_path is not None def setup(self): - pass + if self.custom_apk_exists(): + self.info( + f"Replacing --android-install-apk with custom APK found at " + f"{self.custom_apk_path}" + ) + self.set_arg("android-install-apk", [self.custom_apk_path]) def teardown(self): if self.capture_file is not None: @@ -181,9 +200,9 @@ class AndroidDevice(Layer): self.info("Uninstalling old version") self.device.uninstall_app(self.get_arg("android-app-name")) self.info("Installing %s" % apk) - if apk in _PERMALINKS: + if str(apk) in _PERMALINKS: apk = _PERMALINKS[apk] - if apk.startswith("http"): + if str(apk).startswith("http"): with tempfile.TemporaryDirectory() as tmpdirname: target = Path(tmpdirname, "target.apk") self.info("Downloading %s" % apk) diff --git a/python/mozperftest/mozperftest/system/android_startup.py b/python/mozperftest/mozperftest/system/android_startup.py index c83001f6ffc7..8b13ab86201f 100644 --- a/python/mozperftest/mozperftest/system/android_startup.py +++ b/python/mozperftest/mozperftest/system/android_startup.py @@ -216,14 +216,19 @@ class AndroidStartUp(AndroidDevice): def run_performance_analysis(self, apk_metadata): # Installing the application on the device and getting ready to run the tests + install_path = apk_metadata[KEY_NAME] + if self.custom_apk_exists(): + install_path = self.custom_apk_path + self.device.uninstall_app(self.package_id) - self.info(f"Installing {apk_metadata[KEY_NAME]}...") - app_name = self.device.install_app(apk_metadata[KEY_NAME]) + self.info(f"Installing {install_path}...") + app_name = self.device.install_app(install_path) if self.device.is_app_installed(app_name): self.info(f"Successfully installed {app_name}") else: raise AndroidStartUpInstallError("The android app was not installed") self.apk_name = apk_metadata[KEY_NAME].split(".")[0] + return self.run_tests() def run_tests(self): diff --git a/python/mozperftest/mozperftest/tests/test_android.py b/python/mozperftest/mozperftest/tests/test_android.py index f442722bb1bd..881fcb1cd8d1 100644 --- a/python/mozperftest/mozperftest/tests/test_android.py +++ b/python/mozperftest/mozperftest/tests/test_android.py @@ -1,4 +1,5 @@ #!/usr/bin/env python +import pathlib from unittest import mock import mozunit @@ -162,6 +163,9 @@ def test_android_failure(): android(metadata) +@mock.patch( + "mozperftest.system.android.AndroidDevice.custom_apk_exists", new=lambda x: False +) @mock.patch("mozperftest.utils.requests.get", new=requests_content()) @mock.patch("mozperftest.system.android.ADBLoggedDevice") def test_android_apk_alias(device): @@ -177,7 +181,7 @@ def test_android_apk_alias(device): system = env.layers[SYSTEM] with system as android, silence(system): android(metadata) - # XXX really ? + assert device.mock_calls[1][1][0] == "org.mozilla.fenix" assert device.mock_calls[2][1][0].endswith("target.apk") @@ -250,5 +254,78 @@ def test_android_log_cat(device): andro.device.clear_logcat.assert_called() +@mock.patch("mozperftest.system.android.AndroidDevice.setup", new=mock.MagicMock) +@mock.patch("mozperftest.system.android.Path") +@mock.patch("mozperftest.system.android.ADBLoggedDevice", new=FakeDevice) +def test_android_custom_apk(mozperftest_android_path): + args = { + "flavor": "mobile-browser", + "android": True, + } + + with temp_file(name="user_upload.apk", content="") as sample_apk: + sample_apk = pathlib.Path(sample_apk) + mozperftest_android_path.return_value = sample_apk + + mach_cmd, metadata, env = get_running_env(**args) + system = env.layers[SYSTEM] + android = system.layers[1] + + with system as _, silence(system): + assert android._custom_apk_path is None + assert android.custom_apk_exists() + assert android.custom_apk_path == sample_apk + + mozperftest_android_path.assert_called_once() + + +@mock.patch("mozperftest.system.android.AndroidDevice.setup", new=mock.MagicMock) +@mock.patch("mozperftest.system.android.Path.exists") +@mock.patch("mozperftest.system.android.ADBLoggedDevice", new=FakeDevice) +def test_android_custom_apk_nonexistent(path_exists): + args = { + "flavor": "mobile-browser", + "android": True, + } + + path_exists.return_value = False + + mach_cmd, metadata, env = get_running_env(**args) + system = env.layers[SYSTEM] + android = system.layers[1] + + with system as _, silence(system): + assert android._custom_apk_path is None + assert not android.custom_apk_exists() + assert android.custom_apk_path is None + + path_exists.assert_called() + + +@mock.patch("mozperftest.system.android.Path") +@mock.patch("mozperftest.system.android.ADBLoggedDevice", new=FakeDevice) +def test_android_setup_custom_apk(mozperftest_android_path): + args = { + "flavor": "mobile-browser", + "android": True, + } + + with temp_file(name="user_upload.apk", content="") as sample_apk: + sample_apk = pathlib.Path(sample_apk) + mozperftest_android_path.return_value = sample_apk + + mach_cmd, metadata, env = get_running_env(**args) + system = env.layers[SYSTEM] + android = system.layers[1] + + with system as _, silence(system): + # The custom apk should be found immediately, and it + # should replace any --android-install-apk settings + assert android._custom_apk_path == sample_apk + assert env.get_arg("android-install-apk") == [sample_apk] + + mozperftest_android_path.assert_called_once() + + if __name__ == "__main__": mozunit.main() diff --git a/python/mozperftest/mozperftest/tests/test_android_startup.py b/python/mozperftest/mozperftest/tests/test_android_startup.py index 957a96666188..9344875507d1 100644 --- a/python/mozperftest/mozperftest/tests/test_android_startup.py +++ b/python/mozperftest/mozperftest/tests/test_android_startup.py @@ -1,10 +1,12 @@ import copy import json +import pathlib import random import time from datetime import date from unittest import mock +import mozunit import pytest import requests @@ -14,7 +16,11 @@ from mozperftest.system.android_startup import ( AndroidStartUpMatchingError, AndroidStartUpUnknownTestError, ) -from mozperftest.tests.support import EXAMPLE_ANDROID_STARTUP_TEST, get_running_env +from mozperftest.tests.support import ( + EXAMPLE_ANDROID_STARTUP_TEST, + get_running_env, + temp_file, +) SAMPLE_APK_METADATA = { "name": "fenix_nightly_armeabi-v7a_2022_09_27.apk", @@ -95,6 +101,9 @@ def init_mocked_request(status_code, **kwargs): return mock_request +@mock.patch( + "mozperftest.system.android.AndroidDevice.custom_apk_exists", new=lambda x: False +) @mock.patch( "mozdevice.ADBDevice", new=FakeDevice, @@ -132,6 +141,9 @@ def test_invalid_test_name(*mocked): pass +@mock.patch( + "mozperftest.system.android.AndroidDevice.custom_apk_exists", new=lambda x: False +) @mock.patch( "mozdevice.ADBDevice", new=FakeDevice, @@ -151,6 +163,9 @@ def test_multiple_matching_lines(*mocked): pass +@mock.patch( + "mozperftest.system.android.AndroidDevice.custom_apk_exists", new=lambda x: False +) @mock.patch( "mozdevice.ADBDevice", new=FakeDevice, @@ -170,6 +185,9 @@ def test_multiple_total_time_prefix(*mocked): pass +@mock.patch( + "mozperftest.system.android.AndroidDevice.custom_apk_exists", new=lambda x: False +) @mock.patch( "mozdevice.ADBDevice", new=FakeDevice, @@ -190,6 +208,9 @@ def test_multiple_start_proc_lines(*mocked): pass +@mock.patch( + "mozperftest.system.android.AndroidDevice.custom_apk_exists", new=lambda x: False +) @mock.patch( "mozdevice.ADBDevice", new=FakeDevice, @@ -209,6 +230,39 @@ def test_perfherder_layer(*mocked): test.run(metadata) +@mock.patch("mozperftest.system.android.Path") +@mock.patch( + "mozdevice.ADBDevice", + new=FakeDevice, +) +@mock.patch("time.sleep", return_value=time.sleep(0)) +@mock.patch( + "mozperftest.system.android_startup.AndroidStartUp.get_measurement", + return_value=random.randint(500, 1000), +) +def test_custom_apk_startup(get_measurement_mock, time_sleep_mock, path_mock): + SAMPLE_APK_METADATA["name"] = "name_for_multiple_Totaltime_strings" + ARGS["apk_metadata"] = SAMPLE_APK_METADATA + mach_cmd, metadata, env = running_env( + tests=[str(EXAMPLE_ANDROID_STARTUP_TEST)], **ARGS + ) + + with temp_file(name="user_upload.apk", content="") as sample_apk: + sample_apk = pathlib.Path(sample_apk) + path_mock.return_value = sample_apk + + with mock.patch( + "mozperftest.system.android_startup.AndroidStartUp.run_tests" + ) as _: + test = android_startup.AndroidStartUp(env, mach_cmd) + test.run_tests = lambda: True + test.package_id = "FakeID" + assert test.run_performance_analysis(SAMPLE_APK_METADATA) + + +@mock.patch( + "mozperftest.system.android.AndroidDevice.custom_apk_exists", new=lambda x: False +) @mock.patch( "mozdevice.ADBDevice", new=FakeDevice, @@ -224,3 +278,7 @@ def test_get_measurement_from_nav_start_logcat_match_error(*mocked): test = android_startup.AndroidStartUp(env, mach_cmd) with pytest.raises(AndroidStartUpMatchingError): test.run(metadata) + + +if __name__ == "__main__": + mozunit.main() diff --git a/testing/mozharness/mozharness/mozilla/testing/raptor.py b/testing/mozharness/mozharness/mozilla/testing/raptor.py index 569aee08a873..062e00e7d718 100644 --- a/testing/mozharness/mozharness/mozilla/testing/raptor.py +++ b/testing/mozharness/mozharness/mozilla/testing/raptor.py @@ -9,6 +9,7 @@ import copy import glob import multiprocessing import os +import pathlib import re import subprocess import sys @@ -1235,7 +1236,17 @@ class Raptor( if not self.config.get("noinstall", False): if self.app in self.firefox_android_browsers: self.device.uninstall_app(self.binary_path) - self.install_android_app(self.installer_path) + + # Check if the user supplied their own APK, and install + # that instead + installer_path = pathlib.Path( + self.raptor_path, "raptor", "user_upload.apk" + ) + if not installer_path.exists(): + installer_path = self.installerpath + + self.info(f"Installing APK from: {installer_path}") + self.install_android_app(str(installer_path)) else: super(Raptor, self).install() diff --git a/tools/tryselect/selectors/perf.py b/tools/tryselect/selectors/perf.py index 87e15c7977ec..ffb74b448dd4 100644 --- a/tools/tryselect/selectors/perf.py +++ b/tools/tryselect/selectors/perf.py @@ -6,7 +6,10 @@ import copy import enum import itertools import os +import pathlib import re +import shutil +import subprocess import sys from contextlib import redirect_stdout @@ -39,6 +42,13 @@ REVISION_MATCHER = re.compile(r"remote:.*/try/rev/([\w]*)[ \t]*$") # Name of the base category with no variants applied to it BASE_CATEGORY_NAME = "base" +# Add environment variable for firefox-android integration. +# This will let us find the APK to upload automatically. However, +# the following option will need to be supplied: +# --browsertime-upload-apk firefox-android +# OR --mozperftest-upload-apk firefox-android +MOZ_FIREFOX_ANDROID_APK_OUTPUT = os.getenv("MOZ_FIREFOX_ANDROID_APK_OUTPUT", None) + class InvalidCategoryException(Exception): """Thrown when a category is found to be invalid. @@ -49,6 +59,12 @@ class InvalidCategoryException(Exception): pass +class APKNotFound(Exception): + """Raised when a user-supplied path to an APK is invalid.""" + + pass + + class LogProcessor: def __init__(self): self.buf = "" @@ -489,6 +505,34 @@ class PerfParser(CompareParser): "or the fuzzy selector if --show-all is provided.", }, ], + [ + ["--browsertime-upload-apk"], + { + "type": str, + "default": None, + "help": "Path to an APK to upload. Note that this " + "will replace the APK installed in all Android Performance " + "tests. If the Activity, Binary Path, or Intents required " + "change at all relative to the existing GeckoView, and Fenix " + "tasks, then you will need to make fixes in the associated " + "taskcluster files (e.g. taskcluster/ci/test/browsertime-mobile.yml). " + "Alternatively, set MOZ_FIREFOX_ANDROID_APK_OUTPUT to a path to " + "an APK, and then run the command with --browsertime-upload-apk " + "firefox-android. This option will only copy the APK for browsertime, see " + "--mozperftest-upload-apk to upload APKs for startup tests.", + }, + ], + [ + ["--mozperftest-upload-apk"], + { + "type": str, + "default": None, + "help": "See --browsertime-upload-apk. This option does the same " + "thing except it's for mozperftest tests such as the startup ones. " + "Note that those tests only exist through --show-all, as they " + "aren't contained in any existing categories. ", + }, + ], [ ["--variants"], { @@ -1300,8 +1344,87 @@ class PerfParser(CompareParser): return True + def setup_apk_upload(framework, apk_upload_path): + """Setup the APK for uploading to test on try. + + There are two ways of performing the upload: + (1) Passing a path to an APK with: + --browsertime-upload-apk + --mozperftest-upload-apk + (2) Setting MOZ_FIREFOX_ANDROID_APK_OUTPUT to a path that will + always point to an APK () that we can upload. + + The file is always copied to testing/raptor/raptor/user_upload.apk to + integrate with minimal changes for simpler cases when using raptor-browsertime. + + For mozperftest, the APK is always uploaded here for the same reasons: + python/mozperftest/mozperftest/user_upload.apk + """ + frameworks_to_locations = { + "browsertime": pathlib.Path( + build.topsrcdir, "testing", "raptor", "raptor", "user_upload.apk" + ), + "mozperftest": pathlib.Path( + build.topsrcdir, + "python", + "mozperftest", + "mozperftest", + "user_upload.apk", + ), + } + + print("Setting up custom APK upload") + if apk_upload_path in ("firefox-android"): + apk_upload_path = MOZ_FIREFOX_ANDROID_APK_OUTPUT + if apk_upload_path is None: + raise APKNotFound( + "MOZ_FIREFOX_ANDROID_APK_OUTPUT is not defined. It should " + "point to an APK to upload." + ) + apk_upload_path = pathlib.Path(apk_upload_path) + if not apk_upload_path.exists() or apk_upload_path.is_dir(): + raise APKNotFound( + "MOZ_FIREFOX_ANDROID_APK_OUTPUT needs to point to an APK." + ) + else: + apk_upload_path = pathlib.Path(apk_upload_path) + if not apk_upload_path.exists(): + raise APKNotFound(f"Path does not exist: {str(apk_upload_path)}") + + print("\nCopying file in-tree for upload...") + shutil.copyfile( + str(apk_upload_path), + frameworks_to_locations[framework], + ) + + hg_cmd = ["hg", "add", str(frameworks_to_locations[framework])] + print( + f"\nRunning the following hg command (RAM warnings are expected):\n" + f" {hg_cmd}" + ) + subprocess.check_output(hg_cmd) + print( + "\nAPK is setup for uploading. Please commit the changes, " + "and re-run this command. \nEnsure you supply the --android, " + "and select the correct tasks (fenix, geckoview) or use " + "--show-all for mozperftest task selection.\n" + ) + def run(**kwargs): + if ( + kwargs.get("browsertime_upload_apk") is not None + or kwargs.get("mozperftest_upload_apk") is not None + ): + framework = "browsertime" + upload_apk = kwargs.get("browsertime_upload_apk") + if upload_apk is None: + framework = "mozperftest" + upload_apk = kwargs.get("mozperftest_upload_apk") + + PerfParser.setup_apk_upload(framework, upload_apk) + return + # Make sure the categories are following # the rules we've setup PerfParser.run_category_checks() diff --git a/tools/tryselect/test/test_perf.py b/tools/tryselect/test/test_perf.py index adc430ca8ef3..8e627b08b44d 100644 --- a/tools/tryselect/test/test_perf.py +++ b/tools/tryselect/test/test_perf.py @@ -3,6 +3,9 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. import os +import pathlib +import shutil +import tempfile from unittest import mock import mozunit @@ -872,5 +875,40 @@ def test_category_rules(query, should_fail): assert PerfParser.run_category_checks() +@pytest.mark.parametrize( + "apk_name, apk_content, should_fail, failure_message", + [ + ( + "real-file", + "file-content", + False, + None, + ), + ("bad-file", None, True, "Path does not exist:"), + ], +) +def test_apk_upload(apk_name, apk_content, should_fail, failure_message): + with mock.patch("tryselect.selectors.perf.subprocess") as _, mock.patch( + "tryselect.selectors.perf.shutil" + ) as _: + temp_dir = None + try: + temp_dir = tempfile.mkdtemp() + sample_apk = pathlib.Path(temp_dir, apk_name) + if apk_content is not None: + with sample_apk.open("w") as f: + f.write(apk_content) + + if should_fail: + with pytest.raises(Exception) as exc_info: + PerfParser.setup_apk_upload("browsertime", str(sample_apk)) + assert failure_message in str(exc_info) + else: + PerfParser.setup_apk_upload("browsertime", str(sample_apk)) + finally: + if temp_dir is not None: + shutil.rmtree(temp_dir) + + if __name__ == "__main__": mozunit.main()