.github/scripts/run-maestro.sh (3)

92-107: Improve error visibility for XML parsing failures.

The Python inline script (lines 92-107) silently returns an empty string if XML parsing fails (except: sys.exit(0)). This can mask real issues like corrupted XML reports or Maestro crashes that produce invalid output.

Log a warning and handle XML errors explicitly:

 FAILED_NAMES="$(python3 - <<PY
 import sys,xml.etree.ElementTree as ET
 try:
   tree = ET.parse("$MAIN_REPORT")
-except:
-  sys.exit(0)
+except Exception as e:
+  print(f"Warning: Failed to parse XML report: {e}", file=sys.stderr)
+  sys.exit(0)
 root = tree.getroot()
 failed=[]
 for tc in root.findall(".//testcase"):
   if tc.find("failure") is not None or tc.find("error") is not None:
     if tc.get("name"):
       failed.append(tc.get("name").strip())
 for n in sorted(set(failed)):
   print(n)
 PY
 )"

This provides visibility into XML parsing issues while maintaining the same exit behavior.

---

32-43: Verify YAML name extraction handles edge cases robustly.

The flow name extraction (lines 36-37) uses chained sed operations that may be fragile:

raw_name="$(grep -m1 -E '^[[:space:]]*name:' "$file" || true)"
name_val="$(echo "$raw_name" | sed -E 's/^[[:space:]]*name:[[:space:]]*//; s/^["'\'']//; s/["'\'']$//; s/[[:space:]]*$//')"
name_val="$(echo "$name_val" | sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//')"

Edge cases that may cause issues:

• YAML names with multiline strings or YAML anchors/aliases
• Names containing colons (e.g., name: "test: scenario")
• Names with mixed quotes (e.g., name: 'test"name')
• Empty or missing names after sed processing

Consider using a YAML parser library instead of regex/sed:

 while IFS= read -r -d '' file; do
   if grep -qE "test-${SHARD}" "$file"; then
-    raw_name="$(grep -m1 -E '^[[:space:]]*name:' "$file" || true)"
-    if [ -n "$raw_name" ]; then
-      name_val="$(echo "$raw_name" | sed -E 's/^[[:space:]]*name:[[:space:]]*//; s/^["'\'']//; s/["'\'']$//; s/[[:space:]]*$//')"
-      name_val="$(echo "$name_val" | sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//')"
-      if [ -n "$name_val" ]; then
-        printf '%s\t%s\n' "$name_val" "$file" >> "$MAPFILE"
-      fi
-    fi
+    name_val="$(python3 -c "import yaml; data=yaml.safe_load(open('$file')); print(data.get('name', ''))" 2>/dev/null || true)"
+    if [ -n "$name_val" ]; then
+      printf '%s\t%s\n' "$name_val" "$file" >> "$MAPFILE"
+    fi

This ensures correct YAML parsing and reduces fragility. However, this requires PyYAML to be available. If that's not feasible, add validation to skip malformed flows and log warnings.

---

1-194: Add instrumentation and error context to aid debugging.

The script lacks structured logging that makes it hard to diagnose failures in CI. Consider adding:

1. Timestamp logging for each stage (discovery, main run, retries)
2. Exit codes logged explicitly when commands fail
3. Summary output showing shard, platform, flows executed, pass/fail counts

Example instrumentation additions:

 #!/usr/bin/env bash
 set -euo pipefail
+
+log_info() { echo "[$(date +'%Y-%m-%d %H:%M:%S')] [INFO] $*"; }
+log_error() { echo "[$(date +'%Y-%m-%d %H:%M:%S')] [ERROR] $*" >&2; }
 
 PLATFORM="${1:-${PLATFORM:-android}}"
 SHARD="${2:-${SHARD:-default}}"
+log_info "Starting Maestro orchestration: platform=$PLATFORM shard=$SHARD"
 
 FLOWS_DIR=".maestro/tests"

Then replace echo and other output with log_info / log_error calls. This improves log searchability and debugging in CI dashboards.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

.github/workflows/maestro-ios.yml (1)

90-93: Quote the $UDID variable to prevent word splitting.

The variable $UDID should be quoted to prevent potential word splitting, even though UDID values are typically alphanumeric.

Apply this diff:

       - name: Install App
         run: |
-          xcrun simctl install $UDID "ios-simulator-app"
+          xcrun simctl install "$UDID" "ios-simulator-app"

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

.github/scripts/run-maestro.sh (1)

139-152: Android rerun is missing adb setup—tests may run with stale device state.

The rerun block (lines 139–144) omits the Android device setup that precedes the main run (lines 66–70): adb shell settings put system show_touches, adb install, app launch, and force-stop. Skipping this setup in reruns can leave the device in an incorrect state and cause flaky test results.

Additionally, the maestro test commands lack timeout wrappers despite the workflow's 120-minute timeout, risking indefinite hangs on stuck tests.

Apply this diff to restore adb setup in reruns and add timeouts:

  if [ "$PLATFORM" = "android" ]; then
+   adb shell settings put system show_touches 1 || true
+   adb install -r "app-experimental-release.apk" || true
+   adb shell monkey -p "chat.rocket.reactnative" -c android.intent.category.LAUNCHER 1 || true
+   sleep 6
+   adb shell am force-stop "chat.rocket.reactnative" || true
+
-   maestro test "${CURRENT_FAILS[@]}" \
+   timeout 3600 maestro test "${CURRENT_FAILS[@]}" \
      --exclude-tags=util \
      --include-tags="test-${SHARD}" \
      --format junit \
      --output "$RPT" || true
  else
-   maestro test "${CURRENT_FAILS[@]}" \
+   timeout 3600 maestro test "${CURRENT_FAILS[@]}" \
      --exclude-tags=util \
      --include-tags="test-${SHARD}" \
      --exclude-tags=android-only \
      --format junit \
      --output "$RPT" || true
  fi

.github/scripts/run-maestro.sh (2)

92-107: Python XML parsing has bare exception handling—consider specificity.

The embedded Python snippets (lines 92–107 and 159–174) catch all exceptions silently:

try:
  tree = ET.parse("$MAIN_REPORT")
except:
  sys.exit(0)

Bare except: is overly broad and can hide unexpected errors (e.g., file permission issues, syntax errors). Use except (FileNotFoundError, ET.ParseError): instead to explicitly handle expected failures.

Consider refactoring to extract the XML parsing logic into a helper function to reduce duplication and improve maintainability:

parse_junit_failures() {
  local report_file="$1"
  python3 - <<PY
import sys, xml.etree.ElementTree as ET
try:
  tree = ET.parse("$report_file")
except (FileNotFoundError, ET.ParseError):
  sys.exit(0)
root = tree.getroot()
failed = []
for tc in root.findall(".//testcase"):
  if tc.find("failure") is not None or tc.find("error") is not None:
    if tc.get("name"):
      failed.append(tc.get("name").strip())
for n in sorted(set(failed)):
  print(n)
PY
}

# Then reuse:
FAILED_NAMES="$(parse_junit_failures "$MAIN_REPORT")"
NEXT_FAILED="$(parse_junit_failures "$RPT")"

Also applies to: 159-174

---

116-124: Flow deduplication could use associative arrays for clarity.

The deduplication pattern (lines 116–124 and 183–191) uses string-based SEEN variables:

SEEN2=""
for NAME in "${FAILED_ARRAY[@]}"; do
  if [ -n "$FILE" ] && ! printf '%s\n' "$SEEN2" | grep -Fq "$FILE"; then
    SEEN2="${SEEN2}"$'\n'"${FILE}"
  fi
done

This is functional but harder to read. Since bash 4.0+, an associative array is more idiomatic:

declare -A SEEN2
for NAME in "${FAILED_ARRAY[@]}"; do
  if [ -n "$FILE" ] && [ -z "${SEEN2[$FILE]:-}" ]; then
    SEEN2[$FILE]=1
    CANDIDATE_FILES+=("$FILE")
  fi
done

This pattern already appears in lines 54–60, so consistency would help.

Also applies to: 183-191

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.