ReviewOS

also looking at this

craft-native/craft

test(mobile): run the generated apps on a simulator and an emulator

#199
Merged glennmichael123 wants to merge fix/mobile-e2e-real-assertions into main
26 files +3009 -293

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/android/templates/CraftBridge.kt.templatemodified+40-1
Changes to packages/android/templates/CraftBridge.kt.template
@@ -2366,10 +2366,49 @@ class CraftBridge(
23662366 activity.packageManager.getPackageInfo(activity.packageName, 0).versionCode
23672367 }
23682368 } catch (e: Exception) { 0 })
2369 put("isEmulator", Build.FINGERPRINT.contains("generic") || Build.FINGERPRINT.contains("emulator"))
2369 put("isEmulator", isEmulator())
23702370 }.toString()
23712371 }
23722372
2373 /**
2374 * Whether this is an emulator rather than a phone.
2375 *
2376 * This used to be `FINGERPRINT.contains("generic") ||
2377 * FINGERPRINT.contains("emulator")`, and it answered false on the AVDs
2378 * anyone actually runs. A current Google APIs image fingerprints as
2379 *
2380 * google/sdk_gphone64_x86_64/emu64xa:14/UE1A.230829.036/...:user/release-keys
2381 *
2382 * which contains neither word. The mobile E2E suite caught it on an API 34
2383 * x86_64 emulator, reporting `isEmulator is false` from inside one.
2384 *
2385 * `HARDWARE` is the reliable signal and leads here: the AVD's virtual board
2386 * is `goldfish` on the old QEMU pipeline and `ranchu` on the current one,
2387 * and has been one of those two for over a decade. The rest are kept as
2388 * secondary evidence for images that report something else - Genymotion,
2389 * and the older `generic`/`sdk` products.
2390 *
2391 * Note this is a hint, not a security boundary. Anything that can rewrite
2392 * these properties can defeat it, and nothing here should be treated as
2393 * proof of where the code is running.
2394 */
2395 private fun isEmulator(): Boolean {
2396 val hardware = Build.HARDWARE.lowercase()
2397 if (hardware.startsWith("goldfish") || hardware.startsWith("ranchu")) return true
2398
2399 val fingerprint = Build.FINGERPRINT.lowercase()
2400 if (fingerprint.startsWith("generic") || fingerprint.startsWith("unknown")) return true
2401 if (fingerprint.contains("emulator") || fingerprint.contains("sdk_gphone")) return true
2402
2403 val product = Build.PRODUCT.lowercase()
2404 if (product.startsWith("sdk") || product.contains("_sdk") || product.contains("sdk_")) return true
2405
2406 val model = Build.MODEL.lowercase()
2407 if (model.contains("google_sdk") || model.contains("emulator") || model.contains("android sdk built for")) return true
2408
2409 return Build.MANUFACTURER.contains("Genymotion") || Build.BRAND.startsWith("generic")
2410 }
2411
23732412 // ==================== Network Status ====================
23742413
23752414 @JavascriptInterface
packages/ios/fixtures/zig-slice/build-and-run.shmodified+39-8
Changes to packages/ios/fixtures/zig-slice/build-and-run.sh
@@ -123,14 +123,45 @@ LAUNCH_PID=$!
123123
124124# The round trip is fast, but a cold simulator is not. Poll rather than sleep a
125125# fixed amount, so a slow boot does not read as a failure.
126for _ in $(seq 1 60); do
127 if grep -q 'i=26' "$LOG" 2>/dev/null && grep -q 'i=28' "$LOG" 2>/dev/null \
128 && grep -q 'i=32' "$LOG" 2>/dev/null && grep -q 'i=22' "$LOG" 2>/dev/null \
129 && grep -q 'i=34' "$LOG" 2>/dev/null && grep -q 'i=38' "$LOG" 2>/dev/null \
130 && grep -q 'i=46' "$LOG" 2>/dev/null && grep -q 'i=44' "$LOG" 2>/dev/null \
131 && grep -q 'i=24' "$LOG" 2>/dev/null && grep -q 'i=20' "$LOG" 2>/dev/null \
132 && grep -q 'i=72' "$LOG" 2>/dev/null && grep -q 'i=76' "$LOG" 2>/dev/null \
133 && grep -q 'i=78' "$LOG" 2>/dev/null && grep -q 'i=82' "$LOG" 2>/dev/null; then sleep 1; break; fi
126#
127# The budget is wall-clock, not iterations. It used to be `seq 1 60` around a
128# body that runs fourteen greps, which on this laptop is about a minute and on
129# a loaded CI runner is nearer three — and when it ran out the loop simply fell
130# through to the assertions below, which then failed on whichever late marker
131# had not arrived yet. That is a timeout wearing the costume of a wrong result:
132# the first CI run of this fixture reported "the deletion completion never
133# fired, or its reply was lost" when the truth was that the app was still
134# working through the ids before it.
135#
136# So: a deadline, and an explicit failure that names what is missing. The two
137# situations look nothing alike now.
138SLICE_TIMEOUT="${CRAFT_SLICE_TIMEOUT:-600}"
139AWAITED_IDS="20 22 24 26 28 32 34 38 44 46 72 76 78 82"
140DEADLINE=$(( $(date +%s) + SLICE_TIMEOUT ))
141
142missing_ids() {
143 local missing=""
144 for id in $AWAITED_IDS; do
145 # ERE with an explicit boundary rather than \b, which is a GNU
146 # extension this script cannot count on, and rather than a bare
147 # "i=$id", which matches i=260 when it is looking for i=26.
148 grep -qE "i=$id(\$|[^0-9])" "$LOG" 2>/dev/null || missing="$missing i=$id"
149 done
150 echo "$missing"
151}
152
153while :; do
154 MISSING="$(missing_ids)"
155 [ -z "$MISSING" ] && { sleep 1; break; }
156 if [ "$(date +%s)" -ge "$DEADLINE" ]; then
157 echo "==> console"
158 cat "$LOG" || true
159 echo "FAIL: timed out after ${SLICE_TIMEOUT}s waiting for:$MISSING"
160 echo " The app never got that far — this is a timeout, not a wrong answer."
161 echo " Raise CRAFT_SLICE_TIMEOUT if the machine is simply slow."
162 kill "$LAUNCH_PID" 2>/dev/null || true
163 exit 1
164 fi
134165 sleep 1
135166done
136167kill "$LAUNCH_PID" 2>/dev/null || true
packages/ios/src/index.test.tsmodified+17-0
Changes to packages/ios/src/index.test.ts
@@ -350,6 +350,23 @@ describe('choosing a simulator', () => {
350350 expect(chosen?.name).toBe('iPhone 17 Pro')
351351 })
352352
353 it('ignores simulators that cannot run an iOS app', () => {
354 // Booted sorts ahead of everything, so an open Apple Watch or Apple TV
355 // simulator would otherwise be handed back as the place to install an
356 // iphonesimulator build.
357 const chosen = orderSimulators([
358 device('Apple Watch Series 10', 'watchOS-11-0', 'Booted'),
359 device('Apple TV 4K', 'tvOS-18-0', 'Booted'),
360 device('iPhone 17', 'iOS-27-0'),
361 ])[0]
362
363 expect(chosen?.name).toBe('iPhone 17')
364 })
365
366 it('returns nothing when only non-iOS simulators exist', () => {
367 expect(orderSimulators([device('Apple Watch Series 10', 'watchOS-11-0', 'Booted')])[0]).toBeUndefined()
368 })
369
353370 it('and never invents one that is not installed', () => {
354371 // The regression in one line: no devices means no device, rather than a
355372 // hard-coded name xcodebuild will refuse.
packages/ios/src/index.tsmodified+19-12
Changes to packages/ios/src/index.ts
@@ -819,23 +819,30 @@ export interface SimulatorDevice {
819819}
820820
821821/**
822 * Every available simulator, newest runtime first, iPhones before iPads.
822 * Every available *iOS* simulator, newest runtime first, iPhones before iPads.
823823 *
824824 * Exported because the choice is worth testing without a simulator present -
825825 * `pickSimulator` is the policy and this is the data it reads.
826826 */
827827export function orderSimulators(devices: SimulatorDevice[]): SimulatorDevice[] {
828 return [...devices].sort((left, right) => {
829 const booted = Number(right.state === 'Booted') - Number(left.state === 'Booted')
830 if (booted !== 0)
831 return booted
832
833 const phone = Number(right.name.startsWith('iPhone')) - Number(left.name.startsWith('iPhone'))
834 if (phone !== 0)
835 return phone
836
837 return right.runtime.localeCompare(left.runtime)
838 })
828 // iOS runtimes only, before anything else is considered. A booted device
829 // sorts ahead of every shut-down one, and a developer with an Apple Watch or
830 // Apple TV simulator open would otherwise have it handed back as the place
831 // to install an iphonesimulator build - which fails at `simctl install`,
832 // naming a device nobody asked for.
833 return devices
834 .filter(device => device.runtime.startsWith('iOS'))
835 .sort((left, right) => {
836 const booted = Number(right.state === 'Booted') - Number(left.state === 'Booted')
837 if (booted !== 0)
838 return booted
839
840 const phone = Number(right.name.startsWith('iPhone')) - Number(left.name.startsWith('iPhone'))
841 if (phone !== 0)
842 return phone
843
844 return right.runtime.localeCompare(left.runtime)
845 })
839846}
840847
841848/**
packages/ios/templates/CraftApp.swiftmodified+19-1
Changes to packages/ios/templates/CraftApp.swift
@@ -1445,11 +1445,29 @@ struct CraftWebView: UIViewRepresentable {
14451445
14461446 private func isTrustedURL(_ url: URL?) -> Bool {
14471447 guard let url = url else { return false }
1448 if url.scheme == "craft" && url.host == "app" { return true }
14491448 return isTrustedOrigin(scheme: url.scheme ?? "", host: url.host ?? "", port: url.port ?? 0)
14501449 }
14511450
1451 /// The single answer to "may this origin reach native?".
1452 ///
1453 /// The bundled app is served from craft://app by
1454 /// `BundledAssetSchemeHandler`, and it is the only content a generated
1455 /// app loads when no dev server is configured. That origin used to be
1456 /// trusted for *navigation* only: `isTrustedURL` carried the clause,
1457 /// while the `userContentController` guard called this function
1458 /// directly and fell through to the https/localhost check. So every
1459 /// bridge call from the app's own page was answered with
1460 ///
1461 /// Blocked Craft bridge message from untrusted origin: craft://app
1462 ///
1463 /// and its promise never settled — the whole native surface was dead
1464 /// in the default configuration, which is the one every generated app
1465 /// ships with. Two guards that had to agree, and did not. There is one
1466 /// now, and `scripts/mobile-e2e.ts` runs a real round trip on a booted
1467 /// simulator so a future hardening pass cannot quietly take the bridge
1468 /// away again.
14521469 private func isTrustedOrigin(scheme: String, host: String, port: Int) -> Bool {
1470 if scheme == "craft" && host == "app" { return true }
14531471 if scheme == "file" { return true }
14541472 guard scheme == "https" || (scheme == "http" && ["localhost", "127.0.0.1", "::1"].contains(host)) else { return false }
14551473 let defaultPort = scheme == "https" ? 443 : 80
packages/zig/src/bridge_android_device.zigmodified+125-18
Changes to packages/zig/src/bridge_android_device.zig
@@ -146,15 +146,53 @@ fn appendString(
146146// The JNI half
147147// =============================================================================
148148
149/// `Build.FINGERPRINT.contains("generic") || contains("emulator")`.
149/// The `Build` fields the emulator test reads.
150pub const BuildIdentity = struct {
151 hardware: []const u8,
152 fingerprint: []const u8,
153 product: []const u8,
154 model: []const u8,
155 manufacturer: []const u8,
156 brand: []const u8,
157};
158
159/// Whether this is an emulator, asking exactly what `CraftBridge.isEmulator()`
160/// asks.
161///
162/// Kept as a function so it can be asserted without a device, and kept in step
163/// with the Kotlin deliberately: the whole point of the hand-off is that a page
164/// cannot tell which side answered, and two emulator tests that disagree would
165/// make `isEmulator` depend on whether libcraft.so happened to load.
150166///
151/// Kotlin's own test, kept as a function so it can be asserted without a
152/// device. Deliberately not widened: the Kotlin does not check `"sdk"`,
153/// `"vbox"` or the other markers a more thorough emulator test would, and this
154/// answers the same question the shim answers.
155pub fn fingerprintLooksEmulated(fingerprint: []const u8) bool {
156 return std.mem.indexOf(u8, fingerprint, "generic") != null or
157 std.mem.indexOf(u8, fingerprint, "emulator") != null;
167/// Both sides used to be `FINGERPRINT.contains("generic") ||
168/// contains("emulator")`, which answers false on the AVDs anyone actually runs
169/// - a current Google APIs image fingerprints as
170/// `google/sdk_gphone64_x86_64/emu64xa:14/...` and contains neither word. The
171/// mobile E2E suite caught it on an API 34 emulator.
172///
173/// Note the last line is case-sensitive on both sides. That is the Kotlin's
174/// behaviour, not an oversight here.
175pub fn looksEmulated(build: BuildIdentity) bool {
176 const ascii = std.ascii;
177
178 if (ascii.startsWithIgnoreCase(build.hardware, "goldfish") or
179 ascii.startsWithIgnoreCase(build.hardware, "ranchu")) return true;
180
181 if (ascii.startsWithIgnoreCase(build.fingerprint, "generic") or
182 ascii.startsWithIgnoreCase(build.fingerprint, "unknown")) return true;
183 if (ascii.findIgnoreCase(build.fingerprint, "emulator") != null or
184 ascii.findIgnoreCase(build.fingerprint, "sdk_gphone") != null) return true;
185
186 if (ascii.startsWithIgnoreCase(build.product, "sdk") or
187 ascii.findIgnoreCase(build.product, "_sdk") != null or
188 ascii.findIgnoreCase(build.product, "sdk_") != null) return true;
189
190 if (ascii.findIgnoreCase(build.model, "google_sdk") != null or
191 ascii.findIgnoreCase(build.model, "emulator") != null or
192 ascii.findIgnoreCase(build.model, "android sdk built for") != null) return true;
193
194 return std.mem.indexOf(u8, build.manufacturer, "Genymotion") != null or
195 std.mem.startsWith(u8, build.brand, "generic");
158196}
159197
160198/// Read a `static final String` off a class, as UTF-8.
@@ -200,6 +238,10 @@ pub fn read(allocator: std.mem.Allocator, j: Jni, activity: jobject) !DeviceInfo
200238
201239 const fingerprint = try staticString(allocator, j, build, "FINGERPRINT");
202240 defer allocator.free(fingerprint);
241 const hardware = try staticString(allocator, j, build, "HARDWARE");
242 defer allocator.free(hardware);
243 const product = try staticString(allocator, j, build, "PRODUCT");
244 defer allocator.free(product);
203245
204246 // `Build$VERSION` — a nested class, and the `$` is not decoration: the JVM
205247 // knows this type by that binary name and `android/os/Build/VERSION` finds
@@ -227,7 +269,14 @@ pub fn read(allocator: std.mem.Allocator, j: Jni, activity: jobject) !DeviceInfo
227269 .screen_height = metrics.height,
228270 .density = metrics.density,
229271 .app_build = package.build,
230 .is_emulator = fingerprintLooksEmulated(fingerprint),
272 .is_emulator = looksEmulated(.{
273 .hardware = hardware,
274 .fingerprint = fingerprint,
275 .product = product,
276 .model = model,
277 .manufacturer = manufacturer,
278 .brand = brand,
279 }),
231280 };
232281}
233282
@@ -450,15 +499,73 @@ test "density renders the way JSONObject renders a float" {
450499}
451500
452501test "the emulator test asks exactly what the Kotlin asks" {
453 // Real fingerprints, and the two markers the shim looks for.
454 try testing.expect(fingerprintLooksEmulated("generic/sdk_gphone64_arm64/emu64a:14/UE1A.230829.036/11228894:user/release-keys"));
455 try testing.expect(fingerprintLooksEmulated("Android/aosp_cf_x86_64_phone/emulator:13/x/y:userdebug/test-keys"));
456 try testing.expect(!fingerprintLooksEmulated("google/husky/husky:14/AP1A.240405.002/11480754:user/release-keys"));
457 try testing.expect(!fingerprintLooksEmulated("samsung/dm3qxxx/dm3q:14/UP1A.231005.007/S918BXXU4BWL5:user/release-keys"));
458
459 // Not widened past the shim: these are emulator markers this deliberately
460 // does not know about, because the Kotlin does not either.
461 try testing.expect(!fingerprintLooksEmulated("unknown/vbox86p/vbox86p:9/x/y:userdebug/test-keys"));
502 const phone: BuildIdentity = .{
503 .hardware = "husky",
504 .fingerprint = "google/husky/husky:14/AP1A.240405.002/11480754:user/release-keys",
505 .product = "husky",
506 .model = "Pixel 8 Pro",
507 .manufacturer = "Google",
508 .brand = "google",
509 };
510 try testing.expect(!looksEmulated(phone));
511
512 var samsung = phone;
513 samsung.hardware = "qcom";
514 samsung.fingerprint = "samsung/dm3qxxx/dm3q:14/UP1A.231005.007/S918BXXU4BWL5:user/release-keys";
515 samsung.product = "dm3qxxx";
516 samsung.model = "SM-S918B";
517 samsung.manufacturer = "samsung";
518 samsung.brand = "samsung";
519 try testing.expect(!looksEmulated(samsung));
520
521 // The image the E2E suite runs on, and the one the old fingerprint test
522 // answered `false` for: neither "generic" nor "emulator" appears anywhere
523 // in it. HARDWARE is what gives it away.
524 var avd = phone;
525 avd.hardware = "ranchu";
526 avd.fingerprint = "google/sdk_gphone64_x86_64/emu64xa:14/UE1A.230829.036/11228894:user/release-keys";
527 avd.product = "sdk_gphone64_x86_64";
528 avd.model = "sdk_gphone64_x86_64";
529 try testing.expect(looksEmulated(avd));
530
531 // Each marker on its own, so a mutation to any one line fails.
532 var hardware_only = phone;
533 hardware_only.hardware = "goldfish";
534 try testing.expect(looksEmulated(hardware_only));
535
536 var fingerprint_only = phone;
537 fingerprint_only.fingerprint = "generic/sdk/generic:9/x/y:eng/test-keys";
538 try testing.expect(looksEmulated(fingerprint_only));
539
540 var cuttlefish = phone;
541 cuttlefish.fingerprint = "Android/aosp_cf_x86_64_phone/emulator:13/x/y:userdebug/test-keys";
542 try testing.expect(looksEmulated(cuttlefish));
543
544 var product_only = phone;
545 product_only.product = "sdk_google_phone_x86";
546 try testing.expect(looksEmulated(product_only));
547
548 var model_only = phone;
549 model_only.model = "Android SDK built for x86";
550 try testing.expect(looksEmulated(model_only));
551
552 var genymotion = phone;
553 genymotion.manufacturer = "Genymotion";
554 try testing.expect(looksEmulated(genymotion));
555
556 var brand_only = phone;
557 brand_only.brand = "generic_x86";
558 try testing.expect(looksEmulated(brand_only));
559
560 // Genymotion, which the old fingerprint test missed and the `unknown`
561 // prefix now catches on its own - before MANUFACTURER is even consulted.
562 var vbox = phone;
563 vbox.fingerprint = "unknown/vbox86p/vbox86p:9/x/y:userdebug/test-keys";
564 try testing.expect(looksEmulated(vbox));
565
566 // Still a hint and not a boundary, on both sides: anything that can rewrite
567 // these properties defeats it, and nothing here is proof of where the code
568 // is running.
462569}
463570
464571test "the action name matches the Kotlin method exactly" {