Fix Android and iOS emulator screen recording in GitHub Actions (#1058)

Author: dconeybe

.github/workflows/integration_tests.yml

@@ -1267,6 +1267,13 @@ jobs:
           name: log-artifact
           path: testapps/test-results-tvos-${{ matrix.build_os }}-${{ matrix.tvos_device }}*
           retention-days: ${{ env.artifactRetentionDays }}
+      - name: Upload tvOS test video artifact
+        if: ${{ !cancelled() }}
+        uses: actions/upload-artifact@v3
+        with:
+          name: mobile-simulator-test-video-artifact
+          path: testapps/video-*-tvos-${{ matrix.build_os }}-${{ matrix.tvos_device }}.mp4
+          retention-days: ${{ env.artifactRetentionDays }}
       - name: Download log artifacts
         if: ${{ needs.check_and_prepare.outputs.pr_number && failure() && !cancelled() }}
         uses: actions/download-artifact@v3

scripts/gha/test_simulator.py

@@ -79,9 +79,11 @@
 import json
 import os
 import pathlib
+import platform
 import shutil
 import signal
 import subprocess
+import tempfile
 import time
 
 from absl import app
@@ -113,7 +115,7 @@
 }
 
 _RESULT_FILE = "Results1.json"
-_TEST_RETRY = 3
+_MAX_ATTEMPTS = 3
 _CMD_TIMEOUT = 300
 
 _DEVICE_NONE = "None"
@@ -251,7 +253,7 @@ def main(argv):
 
     for app_path in ios_testapps:
       bundle_id = _get_bundle_id(app_path, config)
-      logs=_run_apple_test(testapp_dir, bundle_id, app_path, ios_helper_app, device_id, _TEST_RETRY)
+      logs=_run_apple_test(testapp_dir, bundle_id, app_path, ios_helper_app, device_id)
       tests.append(Test(testapp_path=app_path, logs=logs))
 
     _shutdown_simulator()
@@ -291,7 +293,7 @@ def main(argv):
 
     for app_path in tvos_testapps:
       bundle_id = _get_bundle_id(app_path, config)
-      logs=_run_apple_test(testapp_dir, bundle_id, app_path, tvos_helper_app, device_id, _TEST_RETRY)
+      logs=_run_apple_test(testapp_dir, bundle_id, app_path, tvos_helper_app, device_id, record_video_display="external")
       tests.append(Test(testapp_path=app_path, logs=logs))
 
     _shutdown_simulator()
@@ -326,7 +328,7 @@ def main(argv):
 
     for app_path in android_testapps:
       package_name = _get_package_name(app_path)
-      logs=_run_android_test(testapp_dir, package_name, app_path, android_helper_project, _TEST_RETRY)
+      logs=_run_android_test(testapp_dir, package_name, app_path, android_helper_project)
       tests.append(Test(testapp_path=app_path, logs=logs))
 
     _shutdown_emulator()
@@ -387,23 +389,77 @@ def _build_tvos_helper(helper_project, device_name, device_os):
         return os.path.join(file_dir, file_name)
 
 
-def _record_apple_tests(video_name):
-  command = "xcrun simctl io booted recordVideo -f --codec=h264 %s" % video_name
-  logging.info("Recording test: %s", command)
-  return subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True, preexec_fn=os.setsid)
-
-
-def _stop_recording(record_process):
-  logging.info("Stop recording test")
-  try:
-    os.killpg(record_process.pid, signal.SIGINT)
-  except:
-    logging.info("Stop recording test failed!!!")
+def _record_apple_tests(video_name, display):
+  args = [
+    "xcrun",
+    "simctl",
+    "io",
+    "booted",
+    "recordVideo",
+    "-f",
+    "--codec=h264",
+  ]
+  if display is not None:
+    args.append("--display=" + display)
+  args.append(video_name)
+  return _start_recording(args)
+
+
+def _start_recording(args):
+  logging.info("Starting screen recording: %s", subprocess.list2cmdline(args))
+
+  output_file = tempfile.TemporaryFile()
+
+  # Specify CREATE_NEW_PROCESS_GROUP on Windows because it is required for
+  # sending CTRL_C_SIGNAL, which is done by _stop_recording().
+  proc = subprocess.Popen(
+    args,
+    stdout=output_file,
+    stderr=subprocess.STDOUT,
+    creationflags=
+        subprocess.CREATE_NEW_PROCESS_GROUP
+        if platform.system() == "Windows"
+        else 0,
+  )
+  logging.info("Started screen recording with PID %s", proc.pid)
+  return (proc, output_file)
+
+
+def _stop_recording(start_recording_retval):
+  (proc, output_file) = start_recording_retval
+  logging.info("Stopping screen recording with PID %s by sending CTRL+C",
+    proc.pid)
+
+  # Make sure that `returncode` is set on the `Popen` object.
+  proc.poll()
+
+  if proc.returncode is not None:
+    logging.warning(
+      "WARNING: Screen recording process ended prematurely with exit code %s",
+      proc.returncode
+    )
+    output_file.seek(0)
+    logging.warning("==== BEGIN screen recording output ====")
+    for line in output_file.read().decode("utf8", errors="replace").splitlines():
+      logging.warning("%s", line.rstrip())
+    logging.warning("==== END screen recording output ====")
+
+  output_file.close()
+
+  proc.send_signal(
+    signal.CTRL_C_SIGNAL
+    if platform.system() == "Windows"
+    else signal.SIGINT
+  )
+  proc.wait()
   time.sleep(5)
 
 
 def _save_recorded_apple_video(video_name, summary_dir):
-  logging.info("Save test video to: %s", summary_dir)
+  logging.info("Save test video %s to: %s", video_name, summary_dir)
+  if not os.path.exists(video_name):
+    logging.warning("Save test video failed: file not found: %s", video_name)
+    return
   shutil.move(video_name, os.path.join(summary_dir, video_name))
 
 
@@ -502,23 +558,24 @@ def _has_uitests(app_path, config):
       return api.get("has_uitests", False)
 
 
-def _run_apple_test(testapp_dir, bundle_id, app_path, helper_app, device_id, retry=1):
+def _run_apple_test(testapp_dir, bundle_id, app_path, helper_app, device_id, max_attempts=_MAX_ATTEMPTS, record_video_display=None):
   """Run helper test and collect test result."""
-  logging.info("Running apple helper test: %s, %s, %s, %s", bundle_id, app_path, helper_app, device_id)
-  _install_apple_app(app_path, device_id)
-  video_name = "video-%s-%s-%s.mp4" % (bundle_id, retry, FLAGS.logfile_name)
-  record_process = _record_apple_tests(video_name)
-  _run_xctest(helper_app, device_id)
-  _stop_recording(record_process)
-  log = _get_apple_test_log(bundle_id, app_path, device_id)
-  _uninstall_apple_app(bundle_id, device_id)
-  result = test_validation.validate_results(log, test_validation.CPP)
-  if not result.complete or (FLAGS.test_type=="uitest" and result.fails>0):
+  attempt_num = 1
+  while attempt_num <= max_attempts:
+    logging.info("Running apple helper test (attempt %s of %s): %s, %s, %s, %s", attempt_num, max_attempts, bundle_id, app_path, helper_app, device_id)
+    _install_apple_app(app_path, device_id)
+    video_name = "video-%s-%s-%s.mp4" % (bundle_id, attempt_num, FLAGS.logfile_name)
+    record_process = _record_apple_tests(video_name, display=record_video_display)
+    _run_xctest(helper_app, device_id)
+    _stop_recording(record_process)
+    log = _get_apple_test_log(bundle_id, app_path, device_id)
+    _uninstall_apple_app(bundle_id, device_id)
+    result = test_validation.validate_results(log, test_validation.CPP)
+    if result.complete and (FLAGS.test_type != "uitest" or result.fails == 0):
+      break
     _save_recorded_apple_video(video_name, testapp_dir)
-    if retry > 1:
-      logging.info("Retry _run_apple_test. Remaining retry: %s", retry-1)
-      return _run_apple_test(testapp_dir, bundle_id, app_path, helper_app, device_id, retry=retry-1)
-  
+    attempt_num += 1
+
   return log
 
 
@@ -629,7 +686,7 @@ def _create_and_boot_emulator(sdk_id):
   if not FLAGS.ci: 
     command = "$ANDROID_HOME/emulator/emulator -avd test_emulator &"
   else:
-    command = "$ANDROID_HOME/emulator/emulator -avd test_emulator -no-audio -no-boot-anim -gpu auto &"
+    command = "$ANDROID_HOME/emulator/emulator -avd test_emulator -no-window -no-audio -no-boot-anim -gpu auto &"
   logging.info("Boot test emulator: %s", command)
   subprocess.Popen(command, universal_newlines=True, shell=True, stdout=subprocess.PIPE)
 
@@ -673,25 +730,27 @@ def _get_package_name(app_path):
   return package_name  
 
 
-def _run_android_test(testapp_dir, package_name, app_path, helper_project, retry=1): 
-  logging.info("Running android helper test: %s, %s, %s", package_name, app_path, helper_project)
-  _install_android_app(app_path)
-  video_name = "video-%s-%s-%s.mp4" % (package_name, retry, FLAGS.logfile_name)
-  logcat_name = "logcat-%s-%s-%s.txt" % (package_name, retry, FLAGS.logfile_name)
-  record_process = _record_android_tests(video_name)
-  _clear_android_logcat()
-  _run_instrumented_test()
-  _stop_recording(record_process)
-  log = _get_android_test_log(package_name)
-  _uninstall_android_app(package_name)
-
-  result = test_validation.validate_results(log, test_validation.CPP)
-  if not result.complete or (FLAGS.test_type=="uitest" and result.fails>0):
+def _run_android_test(testapp_dir, package_name, app_path, helper_project, max_attempts=_MAX_ATTEMPTS): 
+  attempt_num = 1
+  while attempt_num <= max_attempts:
+    logging.info("Running android helper test (attempt %s of %s): %s, %s, %s", attempt_num, max_attempts, package_name, app_path, helper_project)
+    _install_android_app(app_path)
+    video_name = "video-%s-%s-%s.mp4" % (package_name, attempt_num, FLAGS.logfile_name)
+    logcat_name = "logcat-%s-%s-%s.txt" % (package_name, attempt_num, FLAGS.logfile_name)
+    record_process = _record_android_tests(video_name)
+    _clear_android_logcat()
+    _run_instrumented_test()
+    _stop_recording(record_process)
+    log = _get_android_test_log(package_name)
+    _uninstall_android_app(package_name)
+
+    result = test_validation.validate_results(log, test_validation.CPP)
+    if result.complete and (FLAGS.test_type != "uitest" or result.fails == 0):
+      break
+
     _save_recorded_android_video(video_name, testapp_dir)
     _save_android_logcat(logcat_name, testapp_dir)
-    if retry > 1:
-      logging.info("Retry _run_android_test. Remaining retry: %s", retry-1)
-      return _run_android_test(testapp_dir, package_name, app_path, helper_project, retry=retry-1)
+    attempt_num += 1
 
   return log
 
@@ -726,9 +785,13 @@ def _install_android_helper_app(helper_project):
 
 
 def _record_android_tests(video_name):
-  command = "adb shell screenrecord /sdcard/%s" % video_name
-  logging.info("Recording test: %s", command)
-  return subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True, preexec_fn=os.setsid)
+  return _start_recording([
+    "adb",
+    "shell",
+    "screenrecord",
+    "--bugreport",
+    "/sdcard/%s" % video_name,
+  ])
 
 
 def _save_recorded_android_video(video_name, summary_dir):
@@ -774,12 +837,20 @@ def _get_android_test_log(test_package):
   return result.stdout
 
 
-def _run_with_retry(args, shell=False, check=True, timeout=_CMD_TIMEOUT, retry_time=_TEST_RETRY, device=_DEVICE_NONE, type=_RESET_TYPE_REBOOT):
-  logging.info("run_with_retry: %s; remaining retry: %s", args, retry_time)
-  if retry_time > 1:
+def _run_with_retry(args, shell=False, check=True, timeout=_CMD_TIMEOUT, max_attempts=_MAX_ATTEMPTS, device=_DEVICE_NONE, type=_RESET_TYPE_REBOOT):
+  attempt_num = 1
+  while attempt_num <= max_attempts:
+    logging.info("run_with_retry: %s (attempt %s of %s)", args, attempt_num, max_attempts)
     try:
       subprocess.run(args, shell=shell, check=check, timeout=timeout)
-    except:
+    except subprocess.SubprocessError:
+      logging.exception("run_with_retry: %s (attempt %s of %s) FAILED", args, attempt_num, max_attempts)
+
+      # If retries have been exhausted, just raise the exception
+      if attempt_num >= max_attempts:
+        raise
+
+      # Otherwise, reset the emulator/simulator and try again.
       if device == _DEVICE_NONE:
         pass
       elif device == _DEVICE_ANDROID:
@@ -788,9 +859,10 @@ def _run_with_retry(args, shell=False, check=True, timeout=_CMD_TIMEOUT, retry_t
       else:
         # Apple
         _reset_simulator_on_error(device, type)
-      _run_with_retry(args, shell, check, timeout, retry_time-1, device, type)
-  else:
-    subprocess.run(args, shell=shell, check=False, timeout=timeout)
+    else:
+      break
+
+    attempt_num += 1
 
 
 if __name__ == '__main__':