From 600d034817e65ad8c9b3521be6d8fb2475ee0213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tarek=20Ziad=C3=A9?= Date: Tue, 21 Jan 2020 07:17:05 +0000 Subject: [PATCH] Bug 1568092 - don't ignore tooltool_download() exit code and add a retry r=AlexandruIonescu,Bebe This patch will check the exit code of the ProcessHandler() that calls the tooltool script, and will also retry when it fails. Differential Revision: https://phabricator.services.mozilla.com/D60397 --HG-- extra : moz-landing-system : lando --- testing/mozbase/mozproxy/mozproxy/utils.py | 10 ++- testing/mozbase/mozproxy/setup.py | 2 +- testing/mozbase/mozproxy/tests/test_proxy.py | 63 +++++++++++++++++-- .../tests/tp6/mobile/raptor-tp6m-27-cold.ini | 4 +- 4 files changed, 68 insertions(+), 11 deletions(-) diff --git a/testing/mozbase/mozproxy/mozproxy/utils.py b/testing/mozbase/mozproxy/mozproxy/utils.py index d83c836b70fd..a7af4bf0d865 100644 --- a/testing/mozbase/mozproxy/mozproxy/utils.py +++ b/testing/mozbase/mozproxy/mozproxy/utils.py @@ -13,8 +13,9 @@ import os import signal import sys import socket -from six.moves.urllib.request import urlretrieve +from six.moves.urllib.request import urlretrieve +from redo import retriable try: import zstandard except ImportError: @@ -67,6 +68,7 @@ def transform_platform(str_to_transform, config_platform, config_processor=None) return str_to_transform +@retriable(sleeptime=2) def tooltool_download(manifest, run_local, raptor_dir): """Download a file from tooltool using the provided tooltool manifest""" @@ -116,9 +118,11 @@ def tooltool_download(manifest, run_local, raptor_dir): command, processOutputLine=outputHandler, storeOutput=False, cwd=raptor_dir ) proc.run() - proc.wait() + if proc.wait() != 0: + raise Exception("Command failed") except Exception as e: - LOG.critical("Error while downloading the hostutils from tooltool: {}".format(str(e))) + LOG.critical("Error while downloading {} from tooltool:{}".format( + manifest, str(e))) if proc.poll() is None: proc.kill(signal.SIGTERM) raise diff --git a/testing/mozbase/mozproxy/setup.py b/testing/mozbase/mozproxy/setup.py index e79142354f37..10ae36a7c752 100644 --- a/testing/mozbase/mozproxy/setup.py +++ b/testing/mozbase/mozproxy/setup.py @@ -10,7 +10,7 @@ PACKAGE_NAME = "mozproxy" PACKAGE_VERSION = "1.0" # dependencies -deps = [] +deps = ["redo"] setup( name=PACKAGE_NAME, diff --git a/testing/mozbase/mozproxy/tests/test_proxy.py b/testing/mozbase/mozproxy/tests/test_proxy.py index 45d725b9a085..31a2fa65a1e9 100644 --- a/testing/mozbase/mozproxy/tests/test_proxy.py +++ b/testing/mozbase/mozproxy/tests/test_proxy.py @@ -15,21 +15,43 @@ class Process: def __init__(self, *args, **kw): pass + def run(self): + print("I am running something") + def poll(self): return None - wait = poll + def wait(self): + return 0 def kill(self, sig=9): pass + proc = object() pid = 1234 stderr = stdout = None + returncode = 0 + + +_RETRY = 0 + + +class ProcessWithRetry(Process): + def __init__(self, *args, **kw): + Process.__init__(self, *args, **kw) + + def wait(self): + global _RETRY + _RETRY += 1 + if _RETRY >= 2: + _RETRY = 0 + return 0 + return -1 -@mock.patch("mozprocess.processhandler.ProcessHandlerMixin.Process", new=Process) -@mock.patch("mozproxy.backends.mitm.tooltool_download", new=mock.DEFAULT) @mock.patch("mozproxy.backends.mitm.Mitmproxy.check_proxy") +@mock.patch("mozproxy.backends.mitm.mitm.ProcessHandler", new=Process) +@mock.patch("mozproxy.utils.ProcessHandler", new=Process) def test_mitm(*args): bin_name = "mitmproxy-rel-bin-4.0.4-{platform}.manifest" pageset_name = "mitm4-linux-firefox-amazon.manifest" @@ -58,9 +80,9 @@ def test_mitm(*args): playback.stop() -@mock.patch("mozprocess.processhandler.ProcessHandlerMixin.Process", new=Process) -@mock.patch("mozproxy.backends.mitm.tooltool_download", new=mock.DEFAULT) @mock.patch("mozproxy.backends.mitm.Mitmproxy.check_proxy") +@mock.patch("mozproxy.backends.mitm.mitm.ProcessHandler", new=Process) +@mock.patch("mozproxy.utils.ProcessHandler", new=Process) def test_playback_setup_failed(*args): class SetupFailed(Exception): pass @@ -103,5 +125,36 @@ def test_playback_setup_failed(*args): raise +@mock.patch("mozproxy.backends.mitm.Mitmproxy.check_proxy") +@mock.patch("mozproxy.backends.mitm.mitm.ProcessHandler", new=ProcessWithRetry) +@mock.patch("mozproxy.utils.ProcessHandler", new=ProcessWithRetry) +def test_mitm_with_retry(*args): + bin_name = "mitmproxy-rel-bin-4.0.4-{platform}.manifest" + pageset_name = "mitm4-linux-firefox-amazon.manifest" + + config = { + "playback_tool": "mitmproxy", + "playback_binary_manifest": bin_name, + "playback_pageset_manifest": pageset_name, + "playback_version": '4.0.4', + "platform": mozinfo.os, + "playback_recordings": os.path.join(here, "paypal.mp"), + "run_local": True, + "binary": "firefox", + "app": "firefox", + "host": "example.com", + } + + with tempdir() as obj_path: + config["obj_path"] = obj_path + playback = get_playback(config) + playback.config['playback_files'] = config['playback_recordings'] + assert playback is not None + try: + playback.start() + finally: + playback.stop() + + if __name__ == "__main__": mozunit.main(runwith="pytest") diff --git a/testing/raptor/raptor/tests/tp6/mobile/raptor-tp6m-27-cold.ini b/testing/raptor/raptor/tests/tp6/mobile/raptor-tp6m-27-cold.ini index fb22e283c059..bb3efeeedbe7 100644 --- a/testing/raptor/raptor/tests/tp6/mobile/raptor-tp6m-27-cold.ini +++ b/testing/raptor/raptor/tests/tp6/mobile/raptor-tp6m-27-cold.ini @@ -8,8 +8,8 @@ type = pageload test_url = https://edition.cnn.com playback = mitmproxy-android -playback_pageset_manifest = mitm4-motog5-gve-cnn.manifest -playback_recordings = android-cnn.mp +playback_pageset_manifest = mitm4-pixel2-fennec-cnn.manifest +playback_recordings = mitm4-pixel2-fennec-cnn.mp browser_cycles = 15 unit = ms lower_is_better = true