Skip to content

fix(e2e): fix applesimutils brew trust ordering and improve CI failure messages#9899

Closed
yasserfaraazkhan wants to merge 5 commits into
mainfrom
e2e-iOS-home-brew-fix
Closed

fix(e2e): fix applesimutils brew trust ordering and improve CI failure messages#9899
yasserfaraazkhan wants to merge 5 commits into
mainfrom
e2e-iOS-home-brew-fix

Conversation

@yasserfaraazkhan

Copy link
Copy Markdown
Contributor

Summary

  • brew trust wix/brew was running before brew tap wix/brew, causing "Refusing to load formula from untrusted tap" errors on fresh runners. Fixed by tapping before trusting, and removing || true suppression.

  • When all test machines fail before producing results, the PR check showed "? passed, ? failed" because the report-save step was blocking the failure-counting step. Fixed with continue-on-error: true.

  • When no S3 report exists (infra failure), clicking the check now falls back to the GitHub Actions run URL instead of going nowhere.

Fixes the E2E failure on #9887.

NONE

…e messages

- brew trust wix/brew was running before brew tap wix/brew, causing
  "Refusing to load formula from untrusted tap" errors on fresh runners.
  Fixed by tapping before trusting, and removing || true suppression.

- When all test machines fail before producing results, the PR check
  showed "? passed, ? failed" because the report-save step was blocking
  the failure-counting step. Fixed with continue-on-error: true.

- When no S3 report exists (infra failure), clicking the check now
  falls back to the GitHub Actions run URL instead of going nowhere.

Fixes the E2E failure on #9887.
@mattermost-build mattermost-build added the E2E/Run Triggers E2E tests on both iOS and Android via Matterwick label Jun 30, 2026
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

An error occurred during the review process. Please try again later.

📝 Walkthrough

Walkthrough

The PR migrates E2E status reporting to GitHub Checks, updates permissions, changes skipped and cancelled outcomes, and adjusts Maestro and Detox target URL handling. It also changes iOS setup behaviour and no-report parsing in the E2E workflows.

Changes

GitHub Checks migration and E2E status handling

Layer / File(s) Summary
Check-run status action
.github/actions/update-commit-status/action.yml
The reusable status action now validates allowed statuses, maps them to check-run conclusions, updates existing non-completed check runs, and creates new check runs with optional details URLs.
Maestro PR status wiring
.github/workflows/e2e-maestro-pr.yml
The Maestro PR workflow now uses the local status action for initial and final status jobs, updates permissions to checks write, and passes skipped outcomes and target URLs through the action.
Workflow permissions updates
.github/workflows/e2e-detox-pr.yml, .github/workflows/e2e-detox.yml, .github/workflows/e2e-label-manager.yml, .github/workflows/e2e-cancel-on-label-removal.yml
Detox PR, Detox release, label manager, and cancel-on-label-removal workflows replace statuses write with checks write across their status-related jobs.
Detox and iOS status handling
.github/workflows/e2e-ios-template.yml, .github/workflows/e2e-detox.yml
The Detox workflow final-status jobs now treat skipped test results as skipped and fall back to constructed run URLs, while the iOS template reorders Homebrew setup, removes setup suppression, and makes report-link saving and target URL generation conditional.
Maestro no-report handling
.github/workflows/e2e-maestro-template.yml
The Maestro template changes the no-report branch to emit an infrastructure-failure sentinel, adds a fallback output write, and blocks target URL generation when the sentinel value is present.
Cancelled E2E status updates
.github/actions/cancel-e2e-runs/action.yml
The cancellation helper now creates completed check runs with skipped conclusions and summary text derived from the cancellation reason.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main CI fixes, especially the brew trust ordering and reporting improvements.
Description check ✅ Passed The description directly describes the Homebrew ordering fix, error-handling change, and target URL fallback.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch e2e-iOS-home-brew-fix

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/e2e-ios-template.yml:
- Around line 719-721: The report-save step is allowed to fail in Save report
Detox Dependencies, but generate-report still sets TARGET_URL as if the S3
upload succeeded. Update the workflow logic around Save report Detox
Dependencies and the TARGET_URL assignment so the URL is only published when the
report save actually succeeds; if npm ci or npm run e2e:save-report fails, leave
TARGET_URL empty or unset so the fallback path can run instead.

In @.github/workflows/e2e-maestro-template.yml:
- Around line 363-366: The infra-failure sentinel is being set in the Maestro
report parsing path, but the URL-setting logic still leaves TARGET_URL populated
with the S3 report link. Update the steps that set the URL (including set-url
and the related fallback path in the Maestro workflow template) to detect the
NO_REPORT/FAILURES=-1 case and avoid publishing the S3 URL, so the downstream
fallback in e2e-maestro-pr.yml can redirect to the Actions run instead.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 28e8a007-3795-4ef2-aeff-99a3b141321c

📥 Commits

Reviewing files that changed from the base of the PR and between 84ea188 and 0edd369.

📒 Files selected for processing (4)
  • .github/workflows/e2e-detox.yml
  • .github/workflows/e2e-ios-template.yml
  • .github/workflows/e2e-maestro-pr.yml
  • .github/workflows/e2e-maestro-template.yml

Comment thread .github/workflows/e2e-ios-template.yml
Comment thread .github/workflows/e2e-maestro-template.yml
@yasserfaraazkhan yasserfaraazkhan removed the E2E/Run Triggers E2E tests on both iOS and Android via Matterwick label Jun 30, 2026
@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

Coverage Comparison Report

Generated on June 30, 2026 at 05:00:54 UTC

+-----------------+------------+------------+-----------+
| Metric          | Main       | This PR    | Diff      |
+-----------------+------------+------------+-----------+
| Lines           |     87.76% |     87.71% |    -0.05% |
| Statements      |     87.62% |     87.57% |    -0.05% |
| Branches        |     76.47% |     76.44% |    -0.03% |
| Functions       |     87.22% |     87.15% |    -0.07% |
+-----------------+------------+------------+-----------+
| Total           |     84.76% |     84.71% |    -0.05% |
+-----------------+------------+------------+-----------+

@mattermost-build mattermost-build added the E2E/Run Triggers E2E tests on both iOS and Android via Matterwick label Jun 30, 2026
@yasserfaraazkhan yasserfaraazkhan removed the E2E/Run Triggers E2E tests on both iOS and Android via Matterwick label Jun 30, 2026
@mattermost-build mattermost-build added the E2E/Run Triggers E2E tests on both iOS and Android via Matterwick label Jun 30, 2026
@yasserfaraazkhan yasserfaraazkhan removed the E2E/Run Triggers E2E tests on both iOS and Android via Matterwick label Jun 30, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/actions/cancel-e2e-runs/action.yml:
- Around line 100-111: The cancel-e2e-runs action is creating a new check run
instead of updating the existing one, which leaves the original E2E check
unresolved. In the GitHub API call inside
`.github/actions/cancel-e2e-runs/action.yml`, replace the current checks.create
flow with checks.update against the matching non-completed run, or delegate to
the shared status action so the original run is marked skipped. Use the existing
statusContext and sha values to locate and update the correct check run.

In @.github/actions/update-commit-status/action.yml:
- Around line 55-77: The check-run update logic in the action should correlate
each run with its originating workflow run instead of picking the latest
in-progress check for the same commit/context. Update the flow around the
existing github.rest.checks.listForRef and github.rest.checks.update/create
calls to use a stable external_id, and match against the returned runs by that
identifier before updating. If needed, thread the created check_run_id through
the workflow so the final completion update targets the exact run that was
originally created.

In @.github/workflows/e2e-maestro-pr.yml:
- Around line 52-55: The status-job checkout is still using the untrusted tested
ref before running update-commit-status, so adjust the checkout in the
e2e-maestro-pr workflow to load the action from a trusted ref while keeping
commit_sha pointed at the tested SHA. Update the actions/checkout step used
before .github/actions/update-commit-status to remove the direct dependency on
inputs.MOBILE_VERSION, and set persist-credentials: false so the job does not
retain write credentials on the checked-out repository.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8e8f2acf-051e-46e6-9359-e6698e2f95b2

📥 Commits

Reviewing files that changed from the base of the PR and between f81756f and 4a47515.

📒 Files selected for processing (7)
  • .github/actions/cancel-e2e-runs/action.yml
  • .github/actions/update-commit-status/action.yml
  • .github/workflows/e2e-cancel-on-label-removal.yml
  • .github/workflows/e2e-detox-pr.yml
  • .github/workflows/e2e-detox.yml
  • .github/workflows/e2e-label-manager.yml
  • .github/workflows/e2e-maestro-pr.yml
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/e2e-cancel-on-label-removal.yml

Comment thread .github/actions/cancel-e2e-runs/action.yml Outdated
Comment thread .github/actions/update-commit-status/action.yml
Comment thread .github/workflows/e2e-maestro-pr.yml
@yasserfaraazkhan yasserfaraazkhan added the E2E/Override Skip Running E2E tests label Jun 30, 2026
@yasserfaraazkhan
yasserfaraazkhan enabled auto-merge (squash) June 30, 2026 05:26
@yasserfaraazkhan

Copy link
Copy Markdown
Contributor Author

closing this in favor of #9925

auto-merge was automatically disabled July 22, 2026 12:42

Pull request was closed

@yasserfaraazkhan
yasserfaraazkhan deleted the e2e-iOS-home-brew-fix branch July 22, 2026 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

E2E/Override Skip Running E2E tests release-note-none

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants