ReviewOS

also looking at this

craft-native/craft

test(ios): require a deadline per parked call, or a written reason for none

#249
Merged glennmichael123 wants to merge test/ios-deadline-per-claim into main
2 files +71 -24
9 days ago

Refs #223. Closes a hole in the ratchet #247 added, before the health module needs it closed.

The hole, demonstrated

#247's check asked that a module parking calls on ios_pending call scheduleDeadline somewhere in the file. Once one action in a file had a deadline, a second could be added without one and the check stayed green.

That was a prediction until #248, where it happened: deleting scheduleNotification's deadline passed, because getPendingNotifications in the same file still had its own.

What it checks now

The unit is the function that claims a block. Each one has to either arm a deadline, or carry a line saying why it doesn't:

// no-deadline: stage one is the authorization prompt, and there is a
// person reading it.

Some parked calls rightly have no deadline — a prompt with a person in front of it, or a later stage an earlier deadline already covers. The point is not to forbid those. It is that leaving a deadline out becomes a statement someone wrote, rather than an omission nobody saw. A reviewer reading // no-deadline: can disagree with the reason; nobody can disagree with a line that isn't there.

The first such statement is scheduleNotification's stage one. It was already true and already commented. It is now something the build checks, and it failed the new ratchet until the comment became a declaration — which is the ratchet working.

Why now, and not with the health module

bridge_mobile_health.zig has three actions on one table, not two:

  • getHealthData and saveHealthWorkout are unattended and need deadlines (#223).
  • requestHealthAuthorization puts the HealthKit sheet in front of a person, and must have none.

That is precisely the case the per-module check could not tell apart from a forgotten deadline: the file would contain scheduleDeadline and pass regardless of which action was missing one. Landing this first means the health module is written under the stricter rule rather than checked against it afterwards.

Mutations

mutationfails, naming
scheduleNotification's deadline deleted, getPendingNotifications' kept — the #248 casefn authorizationFired
Siri's deadline deletedfn removeShortcut
stage one's no-deadline: declaration removedfn scheduleNotification

The first is the one the previous version passed.

Each failure names the function, found as the span between the nearest function start above the claim and the next one below it. Claims inside tests are skipped — tests claim blocks to drive the table, and are not dispatch.

No self-match risk of the kind #248 hit: the scan reads the embedded bridge_mobile_* sources, not the conformance test's own text, so the literals in its error message cannot satisfy it.

Verification

zig build test:ios and zig build test:android, Pickier (0 errors), and the mutations above.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.