ReviewOS

also looking at this

craft-native/craft

test(window): make "every window can be reopened" a property the build checks

#76
Merged glennmichael123 wants to merge feat/window-lifecycle-conformance into main
4 files +432 -23
11 days ago

Closes the gap that let the isCraftWindow bug ship.

The bug this exists for

isCraftWindow decided whether a window was craft's by asking whether its content view was a WKWebView. True of exactly one of craft's four window styles:

constructorcontent viewrecognised?
plain windowthe WKWebView✅
--web-sidebar-material (macos.zig:959)backdrop container❌
createWindowWithSidebar (:2214)split view controller❌
createWindowWithSidebarURL (:3363)its own container❌

All three keep their closed window alive, so reopen found them and skipped them — a Dock click on a sidebar app activated the process with nothing on screen, the exact dead end #63 was added to remove.

It compiled. Every test passed. It was caught by reading, not by running — and it could not have been caught by running, because no test in this repository constructs a window at all. Three constructors, six content-view installation sites, zero coverage.

The half of the fix worth more than the fix

Recording the answer instead of inferring it turns an untestable property into a checkable one.

"Does this window look like ours?" can only be answered by holding a window — a live NSApplication, a WKWebView, and the WebContent process behind it. Heavy, and flaky on a runner with no display.

"Did every constructor register?" is a property of the source.

That is why this lands as a source conformance test rather than an integration test, and it is the same idiom test/capabilities_test.zig already uses for bridges.

What is pinned

  • every window constructor registers its window — with a floor of 3, so renaming the initialiser cannot make the scan pass having checked nothing
  • nothing infers craft-ness from a window again — contentView, isKindOfClass:, contentViewController, subviews are all rejected inside isCraftWindow, and it must actually consult the registry
  • the reopen handler still filters by that answer — registration is worthless if the loop stops asking
  • a closed window still survives to be reopened — releasedWhenClosed = NO is set in the same helper that registers, so a constructor cannot get one without the other
  • the scan can find what it claims to check — guards the guard, in case formatting changes make enclosingFnBody return the whole file and every check above start passing vacuously

The registry itself moves to window_registry.zig, free of Objective-C, with 8 unit tests covering capacity, idempotence, null handling, overflow reporting and slot reuse.

Controls

Every check was verified to fail when the property is broken:

controlresult
restore the original contentView inferencefails ✓
comment out one constructor's registrationfails ✓ — names createWindowWithSidebarURL
delete one constructor's registrationfails ✓
stub isCraftWindow to always-truefails ✓
rename the initialiser (vacuous scan)fails via the floor ✓

The first version of the constructor check was toothless, and the controls are what found it. Commenting a registration out left the text in place and satisfied a substring match — the check passed on broken code. Calls are now distinguished from mentions, and that helper has its own test.

Verified

zig build · zig build test · zig build test-js · x86_64-windows cross-build · zig fmt --check · 511 bun tests · pickier 38 warnings, unchanged from main.

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

Sign in to comment on this pull request.