Repository navigation
test(ios): tell a late tap from a lost one in ios_touch - #501
Merged
Merged
Conversation
siracozmen01
commented
Nov 8, 2026
siracozmen01
left a comment
Contributor
Author
There was a problem hiding this comment.
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 postsmwin_eventTouchDowntwice for oneUITouch, the regression the old== 1caught. 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]. ThendownIdis tap 2's andupIdtap 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 inCollect, 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->tapsmeans idb has finished the last tap asked for.TapresetsdoneNs.Watchstops 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 thequarantinelabel (APPEND, sointegrationstays), 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 thetouchscenario's expectedresult: 0 failuresand status 0 fail. The newtook [0-9]+ s, done at [0-9]+ spattern fails without the timing line. The runner writes the count through a.partfile andmv, 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
commented
Nov 8, 2026
siracozmen01
left a comment
Contributor
Author
There was a problem hiding this comment.
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, withTapTouchesmeaningdowns == 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.Collectnow 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_touchesruns on every platform. Each of its assertions fails under the loosened>= 1check 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ios_touchfailed on main at 107e37c (run 42473356947): the test asked for its tap at 1.64 s, the runner ranidb ui tap 201 437and 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.shprints 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 fileMWIN_TEST_RANnames, which it passes to the application as it passesMWIN_TEST_OUT.test/test_ios_touch.mgives 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 inMWIN_TEST_RAN, and the check wants the timing line.test/test_tap_touches.cchecks 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.txtquarantinesios_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.shpasses every scenario. With the runner's two writes ofMWIN_TEST_RANtaken out, it fails the touch scenario alone, its application waiting for a count that never comes.test_tap_touchespasses. 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.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 ranios_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.test/test_ios_touch.mas it is. The Objective-C calls no new library function.Closes #499