diff --git a/.github/workflows/run-maestro-tests.yaml b/.github/workflows/run-maestro-tests.yaml index 87d4d6b6e..e77db400e 100644 --- a/.github/workflows/run-maestro-tests.yaml +++ b/.github/workflows/run-maestro-tests.yaml @@ -199,14 +199,17 @@ jobs: mkdir maestro_outputs # Run every flow even if an earlier one fails, then fail the step at # the end if any of them failed. - status=0 - $HOME/.maestro/bin/maestro test --format=junit --output=report1.xml --test-output-dir=maestro_outputs --no-ansi maestro/Onboarding.yaml || status=1 - $HOME/.maestro/bin/maestro test --format=junit --output=report2.xml --test-output-dir=maestro_outputs --no-ansi maestro/HomePage.yaml || status=1 - $HOME/.maestro/bin/maestro test --format=junit --output=report3.xml --test-output-dir=maestro_outputs --no-ansi maestro/LocationDetails.yaml || status=1 - $HOME/.maestro/bin/maestro test --format=junit --output=report4.xml --test-output-dir=maestro_outputs --no-ansi maestro/PlacesNearby.yaml || status=1 - $HOME/.maestro/bin/maestro test --format=junit --output=report5.xml --test-output-dir=maestro_outputs --no-ansi maestro/MarkersAndRoutes.yaml || status=1 - $HOME/.maestro/bin/maestro test --format=junit --output=report6.xml --test-output-dir=maestro_outputs --no-ansi maestro/RouteCreation.yaml || status=1 - exit $status + # + # This MUST stay a single line. android-emulator-runner runs each + # script line in its own "sh -c" (see the comment above), so a + # "status" variable accumulated across separate lines, with a trailing + # "exit $status", does not work: each line gets a fresh shell, the + # "|| status=1" makes the failing line still exit 0, and the final + # "exit $status" sees an empty variable and exits 0 - which left the + # step (and the whole run) green even when every flow failed. Keeping + # the loop and the exit in one shell makes the non-zero status stick. + # To add a flow, add its name (without .yaml) to the list below. + status=0; for flow in Onboarding HomePage LocationDetails PlacesNearby MarkersAndRoutes RouteCreation; do $HOME/.maestro/bin/maestro test --format=junit --output="maestro_outputs/report-$flow.xml" --test-output-dir=maestro_outputs --no-ansi "maestro/$flow.yaml" || status=1; done; exit $status # Artifact names must be unique across the matrix, so suffix them with the # combination (e.g. maestro-reports-35-large-ja-JP). diff --git a/docs/developers/maestro-tests.md b/docs/developers/maestro-tests.md index 9ad29e13e..498d2e73a 100644 --- a/docs/developers/maestro-tests.md +++ b/docs/developers/maestro-tests.md @@ -151,6 +151,17 @@ A few small reusable flows take parameters via `env:` and are called with is tappable. Instead `SwipeAndTap` swipes the scrollable region in small steps until the element appears, then nudges it clear of the edge so it's not clipped when tapped. + + The nudge swipe can accidentally land on an **inner scrollable list** (rather + than the outer page) and scroll a fully-visible target back out of view — this + is what happened with the audio-beacon list in `Onboarding.yaml`: the nudge + scrolled "Current" out from under the tap, so no beacon was selected and the + continue button stayed disabled. To handle this generically, after nudging, + `SwipeAndTap` re-checks that the target is still visible and, if it is not, + reverses the nudge (the opposite swipe) to restore it. A normally + partially-clipped target stays visible after the nudge, so the reverse is + skipped for it and the behaviour is unchanged — no per-call configuration is + needed. - **`Wait.yaml`** — a fixed pause that never fails the flow. It waits for an element that intentionally never exists, with `optional: true`, so it simply burns the `timeout` (in ms) and carries on. Used to let map tiles or network @@ -385,19 +396,26 @@ For each matrix combination it then: status=1`), and the step fails at the end if any flow failed: ```bash - status=0 - maestro test --format=junit --output=report1.xml \ - --test-output-dir=maestro_outputs --no-ansi maestro/Onboarding.yaml || status=1 - maestro test --format=junit --output=report2.xml \ - --test-output-dir=maestro_outputs --no-ansi maestro/HomePage.yaml || status=1 - # ... and so on through FullScreenMap.yaml - exit $status + status=0; for flow in Onboarding HomePage LocationDetails PlacesNearby MarkersAndRoutes RouteCreation; do \ + maestro test --format=junit --output="maestro_outputs/report-$flow.xml" \ + --test-output-dir=maestro_outputs --no-ansi "maestro/$flow.yaml" || status=1; \ + done; exit $status ``` + This is deliberately a **single shell line**: `android-emulator-runner` runs + each line of the `script` in its own `sh -c`, so a `status` variable spread + across separate lines (with a trailing `exit $status`) is lost — every line + gets a fresh shell, the `|| status=1` makes the failing line exit 0 anyway, + and the final `exit` sees an empty variable and exits 0. That previously left + the step — and the whole run — **green even when every flow failed**. Keeping + the loop and the `exit` in one shell makes the non-zero status stick. + Running every flow regardless of earlier failures means one broken flow no longer hides the results of the flows after it — but note the suite is still [stateful and ordered](#the-suite-is-stateful-and-ordered), so a flow that - fails part-way can still leave later flows without the state they expect. + fails part-way can still leave later flows without the state they expect (as + happens when `Onboarding` fails: every later flow then fails too because the + app never reaches the onboarded home screen). 4. Uploads the `maestro_outputs` reports as an artifact, and on failure also uploads `app/emulator.log` (a full `logcat` capture) to help diagnose what @@ -406,5 +424,5 @@ For each matrix combination it then: The Maestro step uses `continue-on-error: true` so the report-upload steps always run; a final step re-raises the failure to mark the workflow red. -When you add a new flow, add a matching `maestro test ... || status=1` line in -the correct position so CI runs it. +When you add a new flow, add its name (without the `.yaml` extension) to the +`for flow in …` list in the correct position so CI runs it. diff --git a/maestro/SwipeAndTap.yaml b/maestro/SwipeAndTap.yaml index 2f84afbd8..49ce0ab78 100644 --- a/maestro/SwipeAndTap.yaml +++ b/maestro/SwipeAndTap.yaml @@ -16,6 +16,12 @@ appId: org.scottishtecharmy.soundscape # scrollable part of the screen in small steps until the element appears, nudge # it clear of the edge, then tap. # +# The nudge swipe can land on an inner scrollable list (e.g. the audio beacon +# list) rather than the outer page; that scrolls a fully-visible target back out +# of view. So after nudging we check the target is still visible and, if not, +# reverse the nudge to put it back - a partially-clipped target stays visible and +# is left as nudged. +# # Note: swipe start/end coordinates can't be supplied via ${} variables (Maestro # parses them as literals and throws a number-format error), so the per-direction # coordinates are hard-coded below and selected with `when` conditions. @@ -89,5 +95,28 @@ appId: org.scottishtecharmy.soundscape start: 50%, 60% end: 50%, 75% +# If the nudge landed on an inner scrollable list it can scroll a fully-visible +# target back out of view. If the target is no longer visible, reverse the nudge +# (the opposite swipe) to restore it. A normally partially-clipped target stays +# visible after the nudge, so this reverse is skipped for it. +- runFlow: + when: + true: "${output.swipeFound == 1 && output.swipeDir == 'up'}" + notVisible: + id: ${id} + commands: + - swipe: + start: 50%, 60% + end: 50%, 75% +- runFlow: + when: + true: "${output.swipeFound == 1 && output.swipeDir == 'down'}" + notVisible: + id: ${id} + commands: + - swipe: + start: 50%, 75% + end: 50%, 60% + - tapOn: id: ${id}