Skip to content

Commit 2ea757e

Browse files
fix(maestro): use a portable mktemp template in run_maestro
`mktemp -t maestro-run` works on BSD/macOS, which appends the random suffix itself, but GNU coreutils requires at least 6 X's in the template. So on the Linux Android runners every flow printed mktemp: too few X's in template 'maestro-run' '': No such file or directory (os error 2) — log_file was empty, so the tee redirection failed and maestro never ran. iOS was unaffected, which is why this passed local and macOS testing. Use an explicit XXXXXX template, which both implementations accept, and fall back to a PID-based path if mktemp fails for any other reason: artifact plumbing must never be what decides whether the tests run. Audited the rest of the commands added in this series (tr -c, grep -qE) — both POSIX, and already used by the pre-existing recording code that runs on Android. Verified by stubbing a GNU-strict mktemp on PATH: reproduces the CI error with the old form and run_maestro completes with the new one. fix(intro-screen-native): catch the trailing drag whatever order it arrives in The Android e2e flow failed deterministically (both the first attempt and the retry) at `Assert that "Active slide: 3" is visible`. The CI video shows the widget reach slide 3, then snap back to slide 1 and increment the change counter, so the trailing drag the previous guard was written for still got through. It got through because the guard required evidence that a programmatic scroll had landed (settledScrollTarget), and that evidence is not always recorded: the settling list can emit its trailing scrollBeginDrag BEFORE the scroll's own momentum end, and onScrollBeginDrag clears the pending target, so the branch that would have set settledScrollTarget never runs. The guard was then skipped and the stale offset was treated as a swipe. Drop the bookkeeping and rely on the geometry instead. The list is pagingEnabled, so a drag settles at most one page from where it began; a momentum end further away than that cannot be a finger regardless of what came before it. Covered by a test reproducing the CI ordering, which fails without the change.
1 parent 413da76 commit 2ea757e

3 files changed

Lines changed: 53 additions & 22 deletions

File tree

maestro/helpers/helpers.sh

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,17 @@ run_maestro() {
159159
local flow="$1"
160160
shift
161161
local log_file
162-
log_file="$(mktemp -t maestro-run)"
162+
# Explicit XXXXXX template, not `mktemp -t maestro-run`: BSD/macOS mktemp appends the random
163+
# suffix itself, but GNU coreutils (the Linux Android runners) requires at least 6 X's in the
164+
# template and fails with "too few X's in template", leaving log_file empty and every
165+
# redirection to it failing. Works on both.
166+
log_file="$(mktemp "${TMPDIR:-/tmp}/maestro-run.XXXXXX")" || log_file=""
167+
if [ -z "$log_file" ]; then
168+
# Never let artifact plumbing decide whether tests run: fall back to a fixed path so the
169+
# flow still executes (classification just loses its input if that also fails).
170+
log_file="${TMPDIR:-/tmp}/maestro-run.$$"
171+
: > "$log_file" 2>/dev/null || true
172+
fi
163173

164174
DEBUG_RUN_DIR="$DEBUG_OUTPUT_DIR/${PLATFORM}-$(basename "${flow%.yaml}" | tr -c 'A-Za-z0-9._-' '_')"
165175
rm -rf "$DEBUG_RUN_DIR"

packages/pluggableWidgets/intro-screen-native/src/SwipeableContainer.tsx

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,6 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement
8080
// wherever the user left it. Used to recognise momentum events that contradict the
8181
// scroll we just asked for; see onMomentumScrollEnd.
8282
const pendingScrollTarget = useRef<number | null>(null);
83-
// Slide the last programmatic scroll settled on. Android can follow that scroll with a
84-
// second drag+momentum pair reporting the offset the list LEFT, which looks exactly like
85-
// a user swipe back; see onMomentumScrollEnd.
86-
const settledScrollTarget = useRef<number | null>(null);
8783

8884
const rtlSafeIndex = useCallback(
8985
(i: number): number => (isAndroidRTL ? props.slides.length - 1 - i : i),
@@ -95,7 +91,6 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement
9591
setActiveIndex(pageNum);
9692
if (flashList && flashList.current) {
9793
pendingScrollTarget.current = pageNum;
98-
settledScrollTarget.current = null;
9994
flashList.current.scrollToOffset({
10095
offset: rtlSafeIndex(pageNum) * width,
10196
animated
@@ -360,10 +355,6 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement
360355
if (pendingTarget !== null) {
361356
if (newIndex === pendingTarget) {
362357
pendingScrollTarget.current = null;
363-
// Remember where the scroll landed: on Android the settling list can emit a
364-
// SECOND drag + momentum pair afterwards, and that one carries the offset it
365-
// moved away from.
366-
settledScrollTarget.current = pendingTarget;
367358
}
368359
return;
369360
}
@@ -372,14 +363,18 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement
372363
return;
373364
}
374365

375-
// The list is pagingEnabled, so one drag moves exactly one page. A momentum end that
376-
// jumps more than a page right after a programmatic scroll settled is therefore the
377-
// tail of that scroll, reporting the offset it moved AWAY from — not a swipe. Android
378-
// emits it with the drag flag set (the settling list, not a finger, starts the drag),
379-
// so wasUserDragging cannot tell the two apart, and trusting it rewrote the attribute
380-
// to the slide being left: pressing NEXT on slide 2 wrote 3, then 1. A one-page swipe
381-
// still reports normally.
382-
if (settledScrollTarget.current !== null && Math.abs(newIndex - activeIndex) > 1) {
366+
// The list is pagingEnabled, so a drag settles at most one page from where it began.
367+
// Any momentum end further away than that is the tail of a programmatic scroll,
368+
// reporting an offset the list has already left — not a swipe. Android emits it with
369+
// the drag flag set (the settling list, not a finger, begins the drag), so
370+
// wasUserDragging cannot tell the two apart, and trusting it rewrote the attribute to
371+
// a slide that is not on screen: pressing NEXT on slide 2 wrote 3, then 1.
372+
//
373+
// This deliberately does not depend on having seen the programmatic scroll land. That
374+
// trailing drag can arrive BEFORE the scroll's own momentum end, and onScrollBeginDrag
375+
// clears pendingScrollTarget, so there is no record left to gate on — the earlier
376+
// version of this guard checked for one and was skipped in exactly that ordering.
377+
if (Math.abs(newIndex - activeIndex) > 1) {
383378
return;
384379
}
385380

packages/pluggableWidgets/intro-screen-native/src/__tests__/SwipeableContainer.remount.spec.tsx

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -221,17 +221,43 @@ describe("SwipeableContainer remount behaviour", () => {
221221
fireEvent.press(getByTestId("intro$buttonNext"));
222222
onSlideChange.mockClear();
223223

224-
// The user grabs the list before the scroll to slide 3 settles and swipes back to
225-
// slide 1. Taking over invalidates the pending target, so this must be reported.
224+
// The user grabs the list before the scroll to slide 3 settles and swipes back one
225+
// page, landing on slide 2. Taking over invalidates the pending target, so this must
226+
// be reported even though the scroll it interrupted never reported a momentum end.
227+
act(() => {
228+
fireEvent(getByTestId("intro"), "scrollBeginDrag", {});
229+
fireEvent(getByTestId("intro"), "momentumScrollEnd", {
230+
nativeEvent: { contentOffset: { x: WIDTH } }
231+
});
232+
});
233+
234+
expect(onSlideChange).toHaveBeenCalledWith(1, 2);
235+
expect(activeSlide.setValue).toHaveBeenLastCalledWith(new Big(2));
236+
});
237+
238+
it("ignores a trailing drag that arrives before the programmatic scroll reports landing", () => {
239+
// The ordering that defeated the previous guard, taken from the Android CI video: on the
240+
// re-entered page NEXT moves slide 2 -> 3, and the settling list emits its trailing
241+
// scrollBeginDrag BEFORE the scroll's own momentum end. That drag clears the pending
242+
// target, so nothing records that a programmatic scroll ever landed — the old guard
243+
// required such a record and was skipped, sending the attribute 3 -> 1 and reporting a
244+
// change, which is what the flow's "Active slide: 3" assertion saw as "Active slide: 1".
245+
const { getByTestId, activeSlide, onSlideChange } = setup(2);
246+
247+
fireEvent.press(getByTestId("intro$buttonNext"));
248+
expect(activeSlide.setValue).toHaveBeenLastCalledWith(new Big(3));
249+
226250
act(() => {
227251
fireEvent(getByTestId("intro"), "scrollBeginDrag", {});
228252
fireEvent(getByTestId("intro"), "momentumScrollEnd", {
229253
nativeEvent: { contentOffset: { x: 0 } }
230254
});
231255
});
232256

233-
expect(onSlideChange).toHaveBeenCalledWith(0, 2);
234-
expect(activeSlide.setValue).toHaveBeenLastCalledWith(new Big(1));
257+
expect(activeSlide.setValue).toHaveBeenLastCalledWith(new Big(3));
258+
expect(onSlideChange).toHaveBeenCalledTimes(1);
259+
expect(onSlideChange).toHaveBeenCalledWith(2, 1);
260+
expect(getByTestId("intro$buttonDone")).toBeTruthy();
235261
});
236262

237263
it("ignores a stale programmatic scroll that reports the pre-scroll offset", () => {

0 commit comments

Comments
 (0)