Use workflow_run to trigger emulator.wtf tests#6864
Conversation
| @@ -340,42 +337,27 @@ jobs: | |||
| run: ./gradlew :app:assembleDebug :app:assembleAndroidTest -Pabis=x86,x86_64 | |||
|
|
|||
| - name: Build device list | |||
There was a problem hiding this comment.
Why not move this step to the new workflow? There is no actual building happening in this step, and then we have less upload/download and untrusted input to deal with.
There was a problem hiding this comment.
It allow us to build the list of devices in a PR, otherwise we need to make a first PR to update the list since we execute the code from main not from the PR.
There was a problem hiding this comment.
You could use actions/checkout with a reference to the PR branch, no? That also works for forks based on what I can find. We don't actually use the files in the new workflow so no conflict.
Snippet I found, ref/repository should be changed to use github.event.workflow_run:
- name: Clone repository
uses: actions/checkout
with:
fetch-depth: 0
ref: ${{github.event.pull_request.head.ref}}
repository: ${{github.event.pull_request.head.repo.full_name}}There was a problem hiding this comment.
I still think this could work if moved and is a cleaner approach. You've already added the number validation in the other workflow, not too different from validating what is in the version file...
There was a problem hiding this comment.
What do you mean by not too different from validating what is in the version file. I don't see the benefits of checking out in the new workflow, it add one checkout compare to pulling one file. I might change my mind once we introduce e2e test.
There was a problem hiding this comment.
Pull request overview
This PR moves Emulator.wtf instrumentation testing out of the main Pull Request workflow into a new workflow_run-triggered workflow so Emulator.wtf OIDC auth can run safely for PRs coming from forks, while still publishing aggregated test results back to the PR.
Changes:
- Replaced the in-workflow Emulator.wtf execution with a build-only job that uploads APKs + a device list as an artifact.
- Added a new
PR Emulator.wtfworkflow triggered byworkflow_runto download those inputs, run Emulator.wtf tests, and upload results. - Moved test-result publication (EnricoMi) from
pr.ymlinto the new workflow to aggregate both PR-workflow and Emulator.wtf results.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| .github/workflows/pr.yml | Removes direct Emulator.wtf execution and instead builds/uploads Emulator.wtf inputs (APKs + devices.txt); removes in-workflow results publication. |
| .github/workflows/pr-emulator-wtf.yml | New workflow that runs Emulator.wtf after Pull Request completes and publishes aggregated test results back to the PR. |
Comments suppressed due to low confidence (1)
.github/workflows/pr-emulator-wtf.yml:127
- This step unconditionally downloads the
Instrumentation Test app resultsartifact from the current run. If theemulator_wtfjob was skipped (or exited early before uploading artifacts), this download will fail and prevent publishing the results produced bypr.yml.
Make this download conditional (for example only when needs.emulator_wtf.result == 'success') or otherwise handle the missing-artifact case gracefully.
- name: Download emulator.wtf results from this run
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
name: Instrumentation Test app results
path: pr-artifacts/Instrumentation Test app results
| if [[ ! "$line" =~ ^model=Pixel7,version=(2[3-9]|3[0-9]|40)$ ]]; then | ||
| echo "::error::Invalid line in devices.txt" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
@jpelgrom I didn't know what to do there that's why I kept an acceptable range [23-40]
There was a problem hiding this comment.
I think it's fine for now, we have some time until reached and it's better than nothing
Summary
Emulator.wtf currently can't run on PRs from forks. The authentication uses OIDC, which would require allowing anyone with a fork to authenticate against our Emulator.wtf account and we only want our own repo to be able to do that.
This PR adds a new
PR Emulator.wtfworkflow that is triggered byworkflow_runonce the mainPull Requestworkflow completes. Because workflow_run-triggered workflows always execute in the base-repo context (regardless of whether the PR comes from a fork), it can use OIDC credentials safely. The new workflow runs the Emulator.wtf jobs and takes over the test-results reporting that the original publish_test_results job used to handle.