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

@@ -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

testing/mozbase/mozproxy/setup.py

@@ -10,7 +10,7 @@ PACKAGE_NAME = "mozproxy"
 PACKAGE_VERSION = "1.0"
 
 # dependencies
-deps = []
+deps = ["redo"]
 
 setup(
     name=PACKAGE_NAME,

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")

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