ReviewOS

also looking at this

craft-native/craft

test(ios): every field the spec's replies carry, not just getDeviceInfo's

#129
Merged glennmichael123 wants to merge test/ios-reply-shape-conformance into main
1 file +374 -1
10 days ago

One action's check, applied to all of them

getDeviceInfo answers every field the spec answers exists because Zig's getDeviceInfo once answered four of the spec's fourteen fields. The action was .live, so the seam preferred it over the shim; it reported success; a page reading screenWidth or locale got undefined. Nothing else in the repo could catch that — from every other angle the action works. It just works less.

That test was written for one action, by hand. The hazard applies to every action whose reply carries more than one field.

This reads the spec's reply dictionaries out of CraftApp.swift, matches each to the action whose case arm produces it, and requires every key of the Zig module that declares that action. 64 keys attributed, 52 checked — the gap is actions Zig does not serve yet, which is the ratchet's business rather than this test's.

Why attribution, and not a simpler check

The obvious shortcut is to check each key against the union of all Zig sources. It would be worthless. Half these keys are words like value, key, id and success that appear in a dozen modules, so a union check passes for every one of them no matter which module dropped which field — green because it is vacuous.

So the scan does the work: bracket-balance the dictionary, find the enclosing Swift function, find the dispatcher arm that calls it. Two shapes are covered, because the spec uses both — a reply built in a private helper (getDeviceInfo), and one built inline in the case arm (getNetworkStatus). Both are asserted directly, so an attribution that silently stopped resolving fails rather than passing on an empty set.

Two things found while building it

A module's own tests were satisfying the check. The needle is the emitted JSON key — \"residentSize\": — and a module's test literals are full of exactly that. Searching the whole file lets a module pass on the strength of a test asserting a field its implementation has stopped emitting: the assertion and the bug in one file, agreeing with each other. Deleting residentSize from getMemoryUsage's format string passes a whole-file search and fails an implementation-only one. The pre-existing getDeviceInfo check had the same hole and is narrowed here too.

Modules spell keys two ways. Most build the reply as a format string, so the key appears escaped beside a colon. bridge_mobile_bgtasks.zig instead names it — const scheduled_key = "scheduled" — and passes it to a shared taskReply helper, so the literal never appears that way. Demanding the first spelling would fail a module that is correct, which is the failure that gets a check deleted rather than fixed.

Two deliberate divergences, recorded

Both carry the anti-rot guards the deferral table uses — a row Zig starts answering fails the build, and so does a row naming a key the spec stopped answering:

action.keywhy Zig is right
getMemoryUsage.errorSwift's failure arm resolves with ["usedMB": 0, "error": ...], settling the promise with a fabricated zero — a caller cannot tell "this app uses no memory" from "the reading could not be taken". Zig returns NativeCallFailed, so there is no success reply for the key to live in.
getNetworkStatus.isConnected, .typeDeclared .status = .unavailable; the shim answers, with isConnected hardcoded true. That is why Zig refuses rather than reproducing it.

A row here has to argue that Zig is right and the spec is wrong, not merely that they differ.

What was checked

Nine mutations, each expected to bite one guard, each verified to:

mutationresult
Zig drops residentSize (key survives only in the module's own test)ReplyDropsAFieldTheSpecAnswers
the spec grows a field Zig does not answerReplyDropsAFieldTheSpecAnswers
a divergence row deletedReplyDropsAFieldTheSpecAnswers
a divergence row's key starts being answeredReplyDivergenceIsSpent
a divergence row names a key the spec never answersReplyDivergenceNamesNoSuchKey
the resolveCallback(callbackId, result: [ anchor brokenboth scan floors
the enclosing-function needle brokenattribution assertions + floors

zig build test:ios: 18/18. No production behaviour changes.

1 changed file on the files tab, with 0 review threads.

Sign in to comment on this pull request.