Bug 1860170 - Use mozprocess.run_and_wait in puppeteer r=hneiva,webdr…

…iver-reviewers,Sasha

Migrate away from mozprocess ProcessHandler, which is poorly supported.

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

remote/mach_commands.py

@@ -134,7 +134,7 @@ def vendor_puppeteer(command_context, repository, commitish, install):
 
     if install:
         env = {"HUSKY": "0", "PUPPETEER_SKIP_DOWNLOAD": "1"}
-        npm(
+        run_npm(
             "install",
             cwd=os.path.join(command_context.topsrcdir, puppeteer_dir),
             env=env,
@@ -171,43 +171,43 @@ def git(*args, **kwargs):
     return out
 
 
-def npm(*args, **kwargs):
-    from mozprocess import processhandler
+def run_npm(*args, **kwargs):
+    from mozprocess import run_and_wait
+
+    def output_timeout_handler(proc):
+        # In some cases, we wait longer for a mocha timeout
+        print(
+            "Timed out after {} seconds of no output".format(kwargs["output_timeout"])
+        )
 
     env = None
     npm, _ = nodeutil.find_npm_executable()
     if kwargs.get("env"):
         env = os.environ.copy()
         env.update(kwargs["env"])
 
-    proc_kwargs = {}
-    if "processOutputLine" in kwargs:
-        proc_kwargs["processOutputLine"] = kwargs["processOutputLine"]
+    proc_kwargs = {"output_timeout_handler": output_timeout_handler}
+    for kw in ["output_line_handler", "output_timeout"]:
+        if kw in kwargs:
+            proc_kwargs[kw] = kwargs[kw]
 
-    p = processhandler.ProcessHandler(
-        cmd=npm,
-        args=list(args),
+    cmd = [npm]
+    cmd.extend(list(args))
+
+    p = run_and_wait(
+        args=cmd,
         cwd=kwargs.get("cwd"),
         env=env,
-        universal_newlines=True,
+        text=True,
         **proc_kwargs,
     )
-    if not kwargs.get("wait", True):
-        return p
-
-    wait_proc(p, cmd=npm, exit_on_fail=kwargs.get("exit_on_fail", True))
+    post_wait_proc(p, cmd=npm, exit_on_fail=kwargs.get("exit_on_fail", True))
 
     return p.returncode
 
 
-def wait_proc(p, cmd=None, exit_on_fail=True, output_timeout=None):
-    try:
-        p.run(outputTimeout=output_timeout)
-        p.wait()
-        if p.timedOut:
-            # In some cases, we wait longer for a mocha timeout
-            print("Timed out after {} seconds of no output".format(output_timeout))
-    finally:
+def post_wait_proc(p, cmd=None, exit_on_fail=True):
+    if p.poll() is None:
         p.kill()
     if exit_on_fail and p.returncode > 0:
         msg = (
@@ -243,7 +243,9 @@ def __init__(self, logger, expected):
     def pid(self):
         return self.proc and self.proc.pid
 
-    def __call__(self, line):
+    def __call__(self, proc, line):
+        self.proc = proc
+        line = line.rstrip("\r\n")
         event = None
         try:
             if line.startswith("[") and line.endswith("]"):
@@ -486,26 +488,24 @@ def run_test(self, logger, *tests, **params):
         ]
 
         output_handler = MochaOutputHandler(logger, expectations)
-        proc = npm(
+        return_code = run_npm(
             *command,
             cwd=self.puppeteer_dir,
             env=env,
-            processOutputLine=output_handler,
-            wait=False,
+            output_line_handler=output_handler,
+            # Puppeteer unit tests don't always clean-up child processes in case of
+            # failure, so use an output_timeout as a fallback
+            output_timeout=60,
+            exit_on_fail=False,
         )
-        output_handler.proc = proc
-
-        # Puppeteer unit tests don't always clean-up child processes in case of
-        # failure, so use an output_timeout as a fallback
-        wait_proc(proc, "npm", output_timeout=60, exit_on_fail=False)
 
         output_handler.after_end()
 
         # Non-zero return codes are non-fatal for now since we have some
         # issues with unresolved promises that shouldn't otherwise block
         # running the tests
-        if proc.returncode != 0:
-            logger.warning("npm exited with code %s" % proc.returncode)
+        if return_code != 0:
+            logger.warning("npm exited with code %s" % return_code)
 
         if output_handler.has_unexpected:
             exit(1, "Got unexpected results")
@@ -718,7 +718,7 @@ def install_puppeteer(command_context, product, ci):
         env["PUPPETEER_SKIP_DOWNLOAD"] = "1"
 
     if not ci:
-        npm(
+        run_npm(
             "run",
             "clean",
             cwd=puppeteer_dir_full_path,
@@ -727,8 +727,8 @@ def install_puppeteer(command_context, product, ci):
         )
 
     command = "ci" if ci else "install"
-    npm(command, cwd=puppeteer_dir_full_path, env=env)
-    npm(
+    run_npm(command, cwd=puppeteer_dir_full_path, env=env)
+    run_npm(
         "run",
         "build",
         cwd=os.path.join(command_context.topsrcdir, puppeteer_test_dir),