From 43069d1d56a95f433cc450dc0cfaee47fcd7f02d Mon Sep 17 00:00:00 2001
From: Riccardo Cipolleschi <cipolleschi@meta.com>
Date: Fri, 3 Jan 2025 09:55:15 -0800
Subject: [PATCH] Fix code to record and upload videos (#48444)

Summary:

While debugging why the Debug variant was failing, I realised that the code to store video artifacts and the code to record the videos were not working properly.

This diff fixes that

## Context

While looking at the recent failures of the E2E tests, I realized that the Hermes, NewArch, Debug variant often fails to build, not to test, for some misconfiguration.

I also realized that we are already building that varaint successfully once, so why not reuse it? To reuse prebuilds, we need a few steps:

1. make sure we build all the variants we need
2. store the .app file as an artifact
3. download the artifact and use it in the E2E tests

## Changelog:
[Internal] - Build release variant for RNTester

Differential Revision: D67760436
---
 .github/actions/maestro-ios/action.yml  | 10 +++----
 .github/workflow-scripts/maestro-ios.js | 37 +++++++++++++++----------
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/.github/actions/maestro-ios/action.yml b/.github/actions/maestro-ios/action.yml
index fe059326119836..2a476fceeb3fcd 100644
--- a/.github/actions/maestro-ios/action.yml
+++ b/.github/actions/maestro-ios/action.yml
@@ -70,11 +70,11 @@ runs:
       with:
         name: e2e_ios_${{ inputs.app-id }}_report_${{ inputs.jsengine }}_${{ inputs.flavor }}_${{ inputs.architecture }}
         path: |
-          video_record_1.mov
-          video_record_2.mov
-          video_record_3.mov
-          video_record_4.mov
-          video_record_5.mov
+          video_record_${{ inputs.jsengine }}_1.mov
+          video_record_${{ inputs.jsengine }}_2.mov
+          video_record_${{ inputs.jsengine }}_3.mov
+          video_record_${{ inputs.jsengine }}_4.mov
+          video_record_${{ inputs.jsengine }}_5.mov
           report.xml
     - name: Store Logs
       if: failure() && steps.run-tests.outcome == 'failure'
diff --git a/.github/workflow-scripts/maestro-ios.js b/.github/workflow-scripts/maestro-ios.js
index cf0a578f5b352e..c2b4b80b61178a 100644
--- a/.github/workflow-scripts/maestro-ios.js
+++ b/.github/workflow-scripts/maestro-ios.js
@@ -89,18 +89,28 @@ function startVideoRecording(jsengine, currentAttempt) {
   console.log(
     `Start video record using pid: video_record_${jsengine}_${currentAttempt}.pid`,
   );
-  childProcess.exec(
-    `xcrun simctl io booted recordVideo video_record_${jsengine}_${currentAttempt}.mov & echo $! > video_record_${jsengine}_${currentAttempt}.pid`,
-  );
+
+  const recordingArgs =
+    `simctl io booted recordVideo video_record_${jsengine}_${currentAttempt}.mov`.split(
+      ' ',
+    );
+  const recordingProcess = childProcess.spawn('xcrun', recordingArgs, {
+    detached: true,
+    stdio: 'ignore',
+  });
+
+  return recordingProcess;
 }
 
-function stopVideoRecording(jsengine, currentAttempt) {
-  console.log(
-    `Stop video record using pid: video_record_${jsengine}_${currentAttempt}.pid`,
-  );
-  childProcess.exec(
-    `kill -SIGINT $(cat video_record_${jsengine}_${currentAttempt}.pid)`,
-  );
+function stopVideoRecording(recordingProcess) {
+  if (!recordingProcess) {
+    console.log("Passed a null recording process. Can't kill it");
+    return;
+  }
+
+  console.log(`Stop video record using pid: ${recordingProcess.pid}`);
+
+  recordingProcess.kill('SIGINT');
 }
 
 function executeTestsWithRetries(
@@ -110,9 +120,8 @@ function executeTestsWithRetries(
   jsengine,
   currentAttempt,
 ) {
+  const recProcess = startVideoRecording(jsengine, currentAttempt);
   try {
-    startVideoRecording(jsengine, currentAttempt);
-
     const timeout = 1000 * 60 * 10; // 10 minutes
     const command = `$HOME/.maestro/bin/maestro --udid="${udid}" test "${maestroFlow}" --format junit -e APP_ID="${appId}"`;
     console.log(command);
@@ -121,11 +130,11 @@ function executeTestsWithRetries(
       timeout,
     });
 
-    stopVideoRecording(jsengine, currentAttempt);
+    stopVideoRecording(recProcess);
   } catch (error) {
     // Can't put this in the finally block because it will be executed after the
     // recursive call of executeTestsWithRetries
-    stopVideoRecording(jsengine, currentAttempt);
+    stopVideoRecording(recProcess);
 
     if (currentAttempt < MAX_ATTEMPTS) {
       executeTestsWithRetries(