Skip to content

test(ios): tell a late tap from a lost one in ios_touch - #501

Merged
siracozmen01 merged 2 commits into
mainfrom
ios-touch-ready
Nov 8, 2026
Merged

siracozmen01 merged 2 commits into
mainfrom
ios-touch-ready

Conversation

@siracozmen01

@siracozmen01 siracozmen01 commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Problem

ios_touch failed on main at 107e37c (run 42473356947): the test asked for its tap at 1.64 s, the runner ran idb ui tap 201 437 and idb returned success, and no touch reached the window in the 30 seconds phase 1 gave it (#499).

In the 34 passing runs of the iOS job with times, the tap is asked for at 1.54 to 2.04 s, and the touch comes 8.2 to 14 s after the ask (median 9.8 s), nearly all of it the runner and idb. So the deadline was about twice the slowest pass. The runner never said how long idb took, so the failing run could not tell a touch that came after the deadline from one the simulator lost.

Change

  • tools/run_ios_app.sh prints how long each idb and simctl command took and when it was done. After each one it writes how many of the test's lines it has run to the file MWIN_TEST_RAN names, which it passes to the application as it passes MWIN_TEST_OUT.
  • test/test_ios_touch.m gives the tap's phase 150 seconds; the runner's limit is 180. Once idb has done a tap and no touch comes for 20 seconds, the tap was lost on the way, and the test asks for one more, three at most. Each tap is told with when idb did it, and the result line with how many taps it took, so a run shows whether a touch came late or never. The check wants as many ups as downs, at least one, and no more than the taps sent (test/tap_touches.h): a single tap still demands exactly one down and one up, and a late touch of one tap beside the next tap's is allowed only once a tap was asked for again. The first finger's down and up are kept, and must be the same finger where the tap was made. On a timeout the test tells UIKit's state of the application and whether the window is key.
  • tools/test_run_ios_app.sh: the touch scenario's application now waits for the runner's count in MWIN_TEST_RAN, and the check wants the timing line.
  • test/test_tap_touches.c checks that rule on every platform, since the simulator's taps run only in CI: two fingers from one tap, a finger never lifted, and more fingers than taps all fail.
  • CMakeLists.txt quarantines ios_touch, its comment naming Bring ios_touch back from quarantine after 14 green nights #500, which brings it back after 14 green nights. The nightly quarantine step runs it on the iPhone and reports a failure as a warning.

Verification

  • tools/test_run_ios_app.sh passes every scenario. With the runner's two writes of MWIN_TEST_RAN taken out, it fails the touch scenario alone, its application waiting for a count that never comes.
  • test_tap_touches passes. With the rule loosened back to at least one down and one up, its checks of two fingers from one tap and of more fingers than taps fail.
  • CI dispatched on this branch at the head, 84fc5aa, run 42483297384 (https://github.com/Rawframe-Project/maul-window/actions/runs/42483297384): the iOS job passed, its required step running all 11 iPhone tests through the changed runner, and its quarantine step ios_touch, which passed in 16.03 s. The run before the review fix, at 4716900, run 42481557692 (https://github.com/Rawframe-Project/maul-window/actions/runs/42481557692), passed the same way: Its required step ran all 11 iPhone tests through the changed runner, and its quarantine step ran ios_touch, which passed in 11.95 s. ctest prints a passing test's output only in the required step's grep of phase lines, which leaves out the quarantined test, so this run does not show the new tap lines; a failing run, or the quarantine step's log of a failure, will.
  • The source, length, module and test hygiene rules pass, and clang-format leaves test/test_ios_touch.m as it is. The Objective-C calls no new library function.

Closes #499

@siracozmen01 siracozmen01 added the test A missing or weak test label Nov 8, 2026

@siracozmen01 siracozmen01 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 4716900: changes needed (1). I read the code and did not run it.

1. The touch check is weaker than before, with no stated reason, and it does not do what the body says. Phase 1's check went from downs == 1 && ups == 1 to downs >= 1 && ups >= 1. The body says "the check takes the first finger's down and up", but Collect still overwrites downId, upId and down with each event, so it keeps the last ones.

  • A doubled touch now passes. Take one tap (taps == 1) after which the library posts mwin_eventTouchDown twice for one UITouch, the regression the old == 1 caught. The new check passes: both downs carry the same id, and the place is right.
  • A touch can now be counted wrongly. A late touch from tap 1 and the touch from tap 2 can be drained in the same frame as [down1, up1, down2]. Then downId is tap 2's and upId tap 1's, so the check fails although tap 1 came through.
  • Phase 1 advances on the first frame that sees an up, so the retry does not need the looser count. The second tap's touch lands after the check, and the test has stopped by then.
  • Either restore downs == 1 && ups == 1, or bound the counts by the taps asked for (downs <= taps && ups <= taps, so exactly one with one tap). If more than one touch may be counted, record the first down and up in Collect, as the body says.

What you asked about.

  • A late tap does not count as a new one. The test's only runner lines are its taps, so RunnerDone() >= program->taps means idb has finished the last tap asked for. Tap resets doneNs. Watch stops once any down or up is seen, so no tap is asked for after a touch. A touch from tap 1 that arrives while tap 2 runs ends the phase at once, and phase 2 stops the test. Finding 1 is the remaining gap in how the touches are counted.
  • Quarantine. maul_quarantine(ios_touch) appends the quarantine label (APPEND, so integration stays), and the iOS job's quarantine step runs it, as run 42481557692 shows. The CMake comment names #499, which this pull request closes. The open issue that brings the test back is #500. Naming #500 there would lead a reader to the issue that is still open.
  • The stand-in fails without the done-file writes. The fake application now waits until [ "$(cat "$MWIN_TEST_RAN")" = 1 ]. Without the runner's writes it never writes its result, and the touch scenario's expected result: 0 failures and status 0 fail. The new took [0-9]+ s, done at [0-9]+ s pattern fails without the timing line. The runner writes the count through a .part file and mv, so the application never reads a half-written number.
  • Timing. Three taps at about 14 s of idb each plus 20 s of waiting fit in phase 1's 150 s, which is under the runner's 180 s and the test's 240 s timeout.
  • The body has Problem, Change and Verification, and no column-0 name( line.

@siracozmen01 siracozmen01 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at 84fc5aa: ready, once CI and the dispatched iOS run 42483297384 are green.

  • My point is fixed. Phase 1's check is now TapTouches(taps, downs, ups) && downId == upId, with TapTouches meaning downs == ups && 1 <= downs <= taps. With one tap it demands exactly one down and one up, as main did, so a doubled touch from one tap fails again. A retry allows one finger per tap sent. Collect now keeps the first down's id and place and the first up's id, so a late second finger in the same drain no longer swaps the ids.
  • test_tap_touches runs on every platform. Each of its assertions fails under the loosened >= 1 check or under a wrong bound: two fingers from one tap, none, a down with no up, more fingers than taps. It checks the same helper the iOS test calls, so it covers the condition, not a copy of it.
  • Quarantine. The CMake comment names #500, the open issue.
  • The runner, the stand-in and the rest are as I reviewed them at 4716900.

No findings.

@siracozmen01
siracozmen01 merged commit eb76081 into main Nov 8, 2026
86 checks passed
@siracozmen01
siracozmen01 deleted the ios-touch-ready branch November 8, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test A missing or weak test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ios_touch: a tap through idb can come later than the test waits, or not at all

1 participant