Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Maestro run for each PR push #4121

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

jmartinesp
Copy link
Member

@jmartinesp jmartinesp commented Jan 7, 2025

Content

Follow up of #4092, since it wasn't working as expected.

  • Remove the workflow_run configuration for the Maestro job.
  • Make the 'build apk' job always run again too.
  • Make sure the test results are uploaded when the Maestro job fails too (this broke somehow).

Motivation and context

Ideally we wanted to run Maestro once the 'Build APK' flow was finished and reuse its output so we don't have to build the APK twice, but this has proven difficult to achieve, since the 2 options GH actions gives us for that are:

  • workflow_run, which can set up dependencies in a 'when X flow finishes, run flow Y' way, and it's technically what we want, but has the downside that it'll always use the .yml contents of the develop branch, which makes it difficult to maintain or change (as proven by Test using Maestro CLI + emulator instead of Cloud #4092 not really working after being merged).
  • workflow_call, which allows a flow to explicitly start another: it worked as seen in this commit, but ended up in either the Maestro flow being run and displayed as part of the Build APK one or viceversa, and in both cases it was quite awkward since you couldn't see the results directly in the 'checks' UI.

So in the end we're just re-building the APK until a better option appears.

Tests

There should be a working 'Maestro (local)' flow in the list of actions run.

Checklist

  • Changes have been tested on an Android device or Android emulator with API 24
  • UI change has been tested on both light and dark themes
  • Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
  • Pull request is based on the develop branch
  • Pull request title will be used in the release note, it clearly define what will change for the user
  • Pull request includes screenshots or videos if containing UI changes
  • You've made a self review of your PR

Copy link
Contributor

github-actions bot commented Jan 7, 2025

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/xJ5t3f

@jmartinesp jmartinesp force-pushed the test/maestro-local-ci-2 branch 2 times, most recently from 9cc8075 to 18489b4 Compare January 7, 2025 13:30
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.28%. Comparing base (6d1dac6) to head (42a276b).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4121   +/-   ##
========================================
  Coverage    83.28%   83.28%           
========================================
  Files         1885     1885           
  Lines        49096    49096           
  Branches      5764     5764           
========================================
  Hits         40888    40888           
  Misses        6139     6139           
  Partials      2069     2069           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmartinesp jmartinesp force-pushed the test/maestro-local-ci-2 branch 3 times, most recently from 65a349c to 7ac00ec Compare January 7, 2025 15:09
@jmartinesp jmartinesp changed the base branch from develop to test/maestro-local-ci-3 January 7, 2025 15:17
@jmartinesp jmartinesp force-pushed the test/maestro-local-ci-2 branch 7 times, most recently from 0f45363 to a99a0e3 Compare January 7, 2025 16:39
@jmartinesp jmartinesp changed the title Test Maestro runs in PR Make Maestro run for each PR push Jan 8, 2025
@jmartinesp jmartinesp added the PR-Build For changes related to build, tools, CI/CD label Jan 8, 2025
@jmartinesp jmartinesp force-pushed the test/maestro-local-ci-2 branch from 36bd26b to 46dcca1 Compare January 8, 2025 09:45
@jmartinesp jmartinesp changed the base branch from test/maestro-local-ci-3 to develop January 8, 2025 09:51
@jmartinesp jmartinesp marked this pull request as ready for review January 8, 2025 09:52
@jmartinesp jmartinesp requested a review from a team as a code owner January 8, 2025 09:52
@jmartinesp jmartinesp requested review from bmarty and removed request for a team January 8, 2025 09:52
- Remove the `workflow_run` configuration for the Maestro job.
- Make the 'build apk' job always run again too.
- Make sure the test results are uploaded when the Maestro job fails too (this broke somehow).
@jmartinesp jmartinesp force-pushed the test/maestro-local-ci-2 branch from 46dcca1 to 42a276b Compare January 8, 2025 10:03
Copy link

sonarqubecloud bot commented Jan 8, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Build For changes related to build, tools, CI/CD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant