chore: rerun only failed test on action #6831
Conversation
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
WalkthroughAdds a centralized Bash script (.github/scripts/run-maestro.sh) to discover and run Maestro test flows by shard, parse XML results to identify failures, and perform multi-round reruns until success or max attempts; workflows were updated to invoke this script for Android and iOS. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor GitHubActions as "GitHub Actions"
participant Script as "run-maestro.sh"
participant Repo as "Repository (.maestro/tests, reports)"
participant Maestro as "Maestro CLI"
participant Device as "Device/Simulator (adb/xcrun)"
GitHubActions->>Script: invoke (platform, shard)
Script->>Repo: scan .maestro/tests for flows matching test-<SHARD>
Script->>Repo: build deduplicated run plan
Script->>Maestro: run flows (platform-specific flags & tags)
Maestro->>Device: install/launch (Android) and execute tests
Device-->>Maestro: test execution results
Maestro->>Repo: write maestro-report.xml
Script->>Repo: parse maestro-report.xml (embedded Python) -> failed tests
alt failed tests found
Script->>Repo: map failed tests -> flow files
loop rerun rounds until success or max
Script->>Maestro: rerun failing flows -> maestro-rerun-round-<n>.xml
Maestro->>Device: execute rerun flows
Device-->>Maestro: per-round results
Maestro->>Repo: write per-round report
Script->>Repo: parse per-round report -> updated failures
end
end
Script->>GitHubActions: exit status & summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🪛 actionlint (1.7.9).github/workflows/maestro-ios.yml91-91: shellcheck reported issue in this script: SC2086:info:1:22: Double quote to prevent globbing and word splitting (shellcheck) 🪛 Shellcheck (0.11.0).github/scripts/run-maestro.sh[warning] 29-29: Variable was used as an array but is now assigned a string. (SC2178) [warning] 39-39: Expanding an array without an index only gives the first element. (SC2128) [warning] 45-45: Expanding an array without an index only gives the first element. (SC2128) [warning] 51-51: Expanding an array without an index only gives the first element. (SC2128) [warning] 55-55: name appears unused. Verify use (or export if used externally). (SC2034) [warning] 60-60: Expanding an array without an index only gives the first element. (SC2128) [warning] 122-122: Expanding an array without an index only gives the first element. (SC2128) [warning] 189-189: Expanding an array without an index only gives the first element. (SC2128) 🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.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" + fiThis ensures correct YAML parsing and reduces fragility. However, this requires
PyYAMLto 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:
- Timestamp logging for each stage (discovery, main run, retries)
- Exit codes logged explicitly when commands fail
- 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
echoand other output withlog_info/log_errorcalls. This improves log searchability and debugging in CI dashboards.
📜 Review details
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.
📒 Files selected for processing (3)
.github/scripts/run-maestro.sh(1 hunks).github/workflows/maestro-android.yml(1 hunks).github/workflows/maestro-ios.yml(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
.github/scripts/run-maestro.sh
[warning] 29-29: Variable was used as an array but is now assigned a string.
(SC2178)
[warning] 39-39: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 45-45: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 51-51: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 55-55: name appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 60-60: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 122-122: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 178-178: Expanding an array without an index only gives the first element.
(SC2128)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (2)
.github/workflows/maestro-android.yml (1)
60-60: Clean delegation to centralized orchestration script.The change cleanly delegates test execution to the external script, improving maintainability and avoiding duplication with the iOS workflow.
.github/workflows/maestro-ios.yml (1)
92-92: Clean delegation to centralized orchestration script.The change removes inline retry logic and delegates to the external script, enabling unified orchestration across platforms and reducing duplication.
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/maestro-ios.yml (1)
90-93: Quote the$UDIDvariable to prevent word splitting.The variable
$UDIDshould 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"
📜 Review details
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.
📒 Files selected for processing (3)
.github/scripts/run-maestro.sh(1 hunks).github/workflows/maestro-android.yml(2 hunks).github/workflows/maestro-ios.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/maestro-ios.yml
91-91: shellcheck reported issue in this script: SC2086:info:1:22: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 Shellcheck (0.11.0)
.github/scripts/run-maestro.sh
[warning] 29-29: Variable was used as an array but is now assigned a string.
(SC2178)
[warning] 39-39: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 45-45: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 51-51: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 55-55: name appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 60-60: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 122-122: Expanding an array without an index only gives the first element.
(SC2128)
[warning] 195-195: Expanding an array without an index only gives the first element.
(SC2128)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: E2E Build Android / android-build
- GitHub Check: E2E Build iOS / ios-build
🔇 Additional comments (5)
.github/scripts/run-maestro.sh (4)
1-27: ✓ Solid initialization and validation.Proper parameter handling with fallbacks, tool presence checks, and error exit codes prevent downstream failures.
29-63: ✓ Flow discovery and deduplication logic is robust.The use of
-print0withfind, safe sed extraction, and deduplicate-by-path approach prevents duplicate test runs and handles edge cases well.
65-90: ✓ Main test execution properly handles platform differences.Android setup, shard tag filtering, and platform-specific exclusions are all in place. Report validation prevents parsing errors.
137-161: ✓ Rerun loop now properly includes shard filtering and platform-specific setup.The changes here address the previous critical issue—both Android setup and shard tag filtering (
--include-tags="test-${SHARD}") are now present in reruns, and iOS correctly excludesandroid-onlytests. Deduplication logic is sound.However, verify timeout coverage:
- Android: workflow has 60-min timeout at step level (maestro-android.yml:51), so rerun maestro calls are implicitly bounded. ✓
- iOS: workflow step "Run Tests" (maestro-ios.yml:94-95) has no explicit timeout—if maestro hangs, nothing stops it until the GitHub Actions job timeout (6 hours). Reruns could block indefinitely.
Please verify that the iOS workflow's "Run Tests" step has adequate timeout protection, or confirm that a timeout is desired at the script level (e.g., wrapping maestro calls with
timeout 3600)..github/workflows/maestro-android.yml (1)
46-63: ✓ Workflow properly delegates to centralized script with correct argument passing.The chmod step and script invocation (
android ${{ inputs.shard }}) correctly align with the script's parameter handling. The 60-minute timeout adequately covers both main run and reruns for Android.
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_android_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
official_ios_build
GitHub Actions
Error
Rohit3523
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
Rohit3523
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
Rohit3523
requested a deployment
to
official_ios_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
official_android_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
experimental_android_build
GitHub Actions
Waiting
Rohit3523
requested a deployment
to
experimental_ios_build
GitHub Actions
Waiting
Proposed changes
Issue(s)
https://rocketchat.atlassian.net/browse/COMM-83
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.