Skip to content

Commit a0ffc7b

Browse files
ci: make cancelling the native pipeline actually stop it
Clicking "Cancel workflow" appeared to do nothing for minutes. The reason is that `if: always()` is exempt from cancellation by design: a step or job with always() keeps running (and a job with it can even start) after the run is cancelled. The pipeline used always() in eleven places, including every upload inside archive-test-results — so on cancel each of the ~10 in-flight shards first uploaded five artifact sets nobody was waiting for. Replaced all of them with `!cancelled()`, which is equivalent for the cases we care about — it is true on success AND on failure, so failing shards still archive their screenshots/logs/videos/diffs, and jobs with skipped dependencies still run — but is false on cancellation. A shard that trips timeout-minutes is not "cancelled" in this sense, so timeout artifacts survive. Not fixed here: the maestro bash harness does not trap SIGINT/SIGTERM, so a shard mid-flow still runs to the runner's force-kill. Cancellation is much faster, not instant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent d04ff8d commit a0ffc7b

2 files changed

Lines changed: 25 additions & 20 deletions

File tree

.github/actions/archive-test-results/action.yaml

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,24 @@ inputs:
1212
required: false
1313
default: ${{ github.workspace }}
1414

15+
# Every step below is `if: ${{ !cancelled() }}` rather than `if: always()`. Both still run
16+
# when the test step failed — which is the case we need these artifacts for — but always()
17+
# is exempt from cancellation, so cancelling a run left each shard uploading five artifact
18+
# sets before it could die, which is why "Cancel workflow" appeared to do nothing.
1519
runs:
1620
using: composite
1721
steps:
1822
- name: Archive runtime logs
1923
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f #v7
20-
if: always()
24+
if: ${{ !cancelled() }}
2125
with:
2226
name: ${{ inputs.platform }}-runtime-logs-${{ inputs.test-type }}
2327
path: log/*.log
2428
if-no-files-found: ignore
2529

2630
- name: Archive test screenshots
2731
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f #v7
28-
if: always()
32+
if: ${{ !cancelled() }}
2933
with:
3034
name: ${{ inputs.platform }}-screenshots-${{ inputs.test-type }}
3135
path: ${{ inputs.workspace-path }}/maestro/images/actual/${{ inputs.platform }}/**/*.png
@@ -38,7 +42,7 @@ runs:
3842
# resolves the diff name against the baseline's own relative path. That nested location
3943
# was never matched by the screenshots glob above, so diffs were silently lost. Gather
4044
# any *_diff.png from wherever it landed into one flat dir for upload.
41-
if: always()
45+
if: ${{ !cancelled() }}
4246
shell: bash
4347
run: |
4448
dest="${{ inputs.workspace-path }}/maestro/images/diff/${{ inputs.platform }}"
@@ -49,15 +53,15 @@ runs:
4953
5054
- name: Archive screenshot diffs
5155
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f #v7
52-
if: always()
56+
if: ${{ !cancelled() }}
5357
with:
5458
name: ${{ inputs.platform }}-diffs-${{ inputs.test-type }}
5559
path: ${{ inputs.workspace-path }}/maestro/images/diff/${{ inputs.platform }}/*.png
5660
if-no-files-found: ignore
5761

5862
- name: Archive artifacts
5963
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f #v7
60-
if: always()
64+
if: ${{ !cancelled() }}
6165
with:
6266
name: ${{ inputs.platform }}-artifacts-${{ inputs.test-type }}
6367
path: packages/pluggableWidgets/**/artifacts/
@@ -68,7 +72,7 @@ runs:
6872
# empty and the upload is a no-op (if-no-files-found: ignore).
6973
- name: Archive failure videos
7074
uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f #v7
71-
if: always()
75+
if: ${{ !cancelled() }}
7276
with:
7377
name: ${{ inputs.platform }}-videos-${{ inputs.test-type }}
7478
path: maestro/videos/*.mp4

.github/workflows/NativePipeline.yml

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ jobs:
418418
runs-on: ubuntu-26.04
419419
# Skip this job if we're using artifacts from a specific run; the native bundles now come
420420
# from the project job (deploy + native-packager), so gate on project success.
421-
if: ${{ github.event.inputs.LOCAL_TEST_ARTIFACT_RUN_ID == '' && always() && (needs.project.result == 'success') }}
421+
if: ${{ github.event.inputs.LOCAL_TEST_ARTIFACT_RUN_ID == '' && !cancelled() && (needs.project.result == 'success') }}
422422
steps:
423423
- name: Debug branch value
424424
run: echo "Using branch ${{ needs.determine-nt-version.outputs.nt_branch }}"
@@ -535,7 +535,7 @@ jobs:
535535
runs-on: macos-26
536536
# Skip this job if we're using artifacts from a specific run; the native bundles now come
537537
# from the project job (deploy + native-packager), so gate on project success.
538-
if: ${{ github.event.inputs.LOCAL_TEST_ARTIFACT_RUN_ID == '' && always() && (needs.project.result == 'success') }}
538+
if: ${{ github.event.inputs.LOCAL_TEST_ARTIFACT_RUN_ID == '' && !cancelled() && (needs.project.result == 'success') }}
539539
steps:
540540
- name: "Check out Native Template for Native Components Test Project"
541541
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@@ -647,7 +647,7 @@ jobs:
647647
android-widget-tests:
648648
needs: [scope, mendix-version, project, android-app, android-avd-cache]
649649
# Run if widgets need testing (widgets_to_test is not empty) and project succeeds
650-
if: ${{ needs.scope.outputs.widgets_to_test != '[]' && always() && needs.project.result == 'success' && (needs.android-app.result == 'success' || needs.android-app.result == 'skipped') }}
650+
if: ${{ needs.scope.outputs.widgets_to_test != '[]' && !cancelled() && needs.project.result == 'success' && (needs.android-app.result == 'success' || needs.android-app.result == 'skipped') }}
651651
runs-on: ubuntu-26.04
652652
# 30 min hard cap per widget shard. A healthy widget's flows run in ~30s each; the cap is a
653653
# backstop for genuinely wedged shards. Bumped 20→30 after a run where shards were cancelled
@@ -731,12 +731,11 @@ jobs:
731731
bash maestro/run_maestro_widget_tests.sh android "${{ matrix.widget }}" || [ "$UPDATE_BASELINES" = "true" ]
732732
733733
- name: Archive test results
734-
# always(): the test step exits non-zero on a failing shard, which would otherwise
734+
# !cancelled(): the test step exits non-zero on a failing shard, which would otherwise
735735
# SKIP this step — i.e. we'd capture screenshots/logs/videos for green shards but NOT
736-
# for the failing ones we actually need to debug. Run regardless of test outcome.
737-
# !cancelled() rather than always(): always() is exempt from cancellation, so a
738-
# cancelled run still uploaded five artifact sets per shard before it would die.
739-
# This still covers pass AND fail (the cases whose artifacts we want).
736+
# for the failing ones we actually need to debug. This covers pass AND fail, but
737+
# unlike always() it is NOT exempt from cancellation, so a cancelled run stops here
738+
# instead of first uploading five artifact sets per shard.
740739
if: ${{ !cancelled() }}
741740
uses: ./.github/actions/archive-test-results
742741
with:
@@ -746,7 +745,7 @@ jobs:
746745
ios-widget-tests:
747746
needs: [scope, mendix-version, project, ios-app]
748747
# Run if widgets need testing (widgets_to_test is not empty) and project succeeds
749-
if: ${{ needs.scope.outputs.widgets_to_test != '[]' && always() && needs.project.result == 'success' && (needs.ios-app.result == 'success' || needs.ios-app.result == 'skipped') }}
748+
if: ${{ needs.scope.outputs.widgets_to_test != '[]' && !cancelled() && needs.project.result == 'success' && (needs.ios-app.result == 'success' || needs.ios-app.result == 'skipped') }}
750749
runs-on: macos-26
751750
# 30 min per widget shard, matching android-widget-tests. iOS needs the budget more than
752751
# Android: sim cold-boot + Maestro driver startup + runtime-ready wait eat several minutes
@@ -818,7 +817,7 @@ jobs:
818817
android-js-tests:
819818
needs: [scope, mendix-version, project, android-app, android-avd-cache]
820819
# Run if JS actions changed and project succeeds and either android-app succeeds OR we're using custom artifacts (android-app was skipped)
821-
if: ${{ needs.scope.outputs.js_actions_changed == 'true' && always() && needs.project.result == 'success' && (needs.android-app.result == 'success' || needs.android-app.result == 'skipped') }}
820+
if: ${{ needs.scope.outputs.js_actions_changed == 'true' && !cancelled() && needs.project.result == 'success' && (needs.android-app.result == 'success' || needs.android-app.result == 'skipped') }}
822821
runs-on: ubuntu-26.04
823822
timeout-minutes: 90
824823
steps:
@@ -900,7 +899,7 @@ jobs:
900899
ios-js-tests:
901900
needs: [scope, mendix-version, project, ios-app]
902901
# Run if JS actions changed and project succeeds and either ios-app succeeds OR we're using custom artifacts (ios-app was skipped)
903-
if: ${{ needs.scope.outputs.js_actions_changed == 'true' && always() && needs.project.result == 'success' && (needs.ios-app.result == 'success' || needs.ios-app.result == 'skipped') }}
902+
if: ${{ needs.scope.outputs.js_actions_changed == 'true' && !cancelled() && needs.project.result == 'success' && (needs.ios-app.result == 'success' || needs.ios-app.result == 'skipped') }}
904903
runs-on: macos-26
905904
timeout-minutes: 90
906905
steps:
@@ -963,9 +962,11 @@ jobs:
963962
# sources after a successful merge, leaving just the combined artifacts.
964963
aggregate-test-results:
965964
needs: [android-widget-tests, ios-widget-tests, android-js-tests, ios-js-tests]
966-
# Run even when shards fail/cancel — the failing shards are exactly the ones whose
967-
# screenshots/logs we want to inspect.
968-
if: ${{ always() }}
965+
# Run even when shards fail or time out — those are exactly the ones whose
966+
# screenshots/logs we want to inspect. !cancelled() rather than always() so an
967+
# explicitly cancelled run isn't held open merging artifacts nobody asked for
968+
# (a shard that hits timeout-minutes does not make cancelled() true).
969+
if: ${{ !cancelled() }}
969970
runs-on: ubuntu-26.04
970971
# The workflow-level block grants only packages:write, so every other scope defaults to
971972
# none. This job needs contents:read (to check out the triage script) and actions:read

0 commit comments

Comments
 (0)