From 3b71a9e03a5c11562ea1bb5e24e950aa8e19c514 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:46:57 +0300 Subject: [PATCH 01/14] Stop treating the port status snapshots as contract Registering one conformance test required editing eleven files nobody had a result for. `validate()` demanded that every checked-in `docs/website/data/port_status_reports/.json` carry an entry for every registered test, with no `not-run` and a `summary` matching it exactly -- so a branch adding a test had to append an entry to all eleven reports and retype their totals, plus `assertEqual(180, counts["tests"])` in `test_port_status.py`. Two branches adding a test conflicted by construction, on lines whose entire content was a number neither author had a reason to think about. The cost was worse than the friction. Those files are CI output; a branch has no result to put in them, so the cheapest way to a green build was to type "pass" next to a test no port had run. Twelve results reached master that way -- LogSubclassCaptureTest, IntentsApiTest, the seven Database tests, CalendarApiTest, the on-device AI tests, PureEditorScreenshotTest and ClipboardRoundTripTest -- each appended to a report whose generated_at and commit predate the test it claims to have passed. The public table published every one of them as green. None of it was ever needed. `publishable_report_problems` already classifies "this report predates a test" as ordinary drift: publication keeps the older report and waits for the port's next master run. Only the checked-in copies were held to a stricter rule than the ones the site actually serves. So validate() now reads the snapshots through that same drift/malformed split. "report predates tests: X" prints as information and never fails; a report Hugo cannot render is still a defect. Adding a test touches Cn1ssDeviceRunner and port_status.json, and nothing else. "Every registered test runs on every port" moves to where nobody can type the answer: * `port_status.py coverage` runs in the nightly sweep against the reports the ports actually published. `not-run` always fails. A test missing from port P's report fails only when some earlier run on another port already carried it -- that port dropped the test rather than predating it. No history lookup and no grace period to tune; the reports date themselves. * `port_status.py provenance`, via check_port_status_provenance.sh in port-status-contract.yml, refuses a report whose results changed while the generated_at, commit and run_url naming the run stayed put. That is exactly the edit the twelve fabricated passes were made by. test_port_status.py keeps no literal totals. It asserts the relationship the numbers stood in for -- the suite and the contract describe the same test set, nothing is registered twice, nothing is both a conformance and a performance test -- plus floors that catch a collapse. Nothing in it needs retyping when a test is added. The page separates absent from `not-run`, which the old rule had no way to distinguish because it forbade both. Absent means the run predates the test and renders "awaiting this port's next run"; `not-run` means the suite ran with the test in its contract and nothing reported back, and stays a defect. The eleven snapshots are refreshed from port-status-data, which drops the fabrications. The five Apple ports that have not run since LogSubclassCaptureTest landed now say so instead of claiming a pass. Verified: 46 normalizer tests pass; validate, coverage and provenance all green; Hugo builds and validate_port_status.mjs passes on the rendered page; both new gates confirmed to fire on an injected dropped test, an injected not-run, and a stamp-preserving edit; and a simulated branch that registers a test while touching no report passes validate and provenance. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/port-status-contract.yml | 12 + .../data/port_status_reports/android.json | 32 +- .../data/port_status_reports/ios-gl.json | 30 +- .../data/port_status_reports/ios-metal.json | 30 +- .../data/port_status_reports/javascript.json | 32 +- .../data/port_status_reports/linux-arm64.json | 32 +- .../data/port_status_reports/linux-x64.json | 32 +- .../data/port_status_reports/mac-native.json | 36 +-- .../data/port_status_reports/tvos.json | 30 +- .../data/port_status_reports/watchos.json | 30 +- .../port_status_reports/windows-arm64.json | 32 +- .../data/port_status_reports/windows-x64.json | 32 +- .../partials/port-status-feature-status.html | 16 +- .../partials/port-status-port-state.html | 13 +- scripts/hellocodenameone/README.adoc | 25 ++ .../conformance/backfill_port_status.sh | 12 + .../check_port_status_provenance.sh | 64 ++++ .../conformance/port_status.py | 287 ++++++++++++++---- .../conformance/test_port_status.py | 201 +++++++----- 19 files changed, 652 insertions(+), 326 deletions(-) create mode 100755 scripts/hellocodenameone/conformance/check_port_status_provenance.sh diff --git a/.github/workflows/port-status-contract.yml b/.github/workflows/port-status-contract.yml index 7cf2634b717..2f2fd4dbb64 100644 --- a/.github/workflows/port-status-contract.yml +++ b/.github/workflows/port-status-contract.yml @@ -75,6 +75,18 @@ jobs: python-version: '3.13' - name: Validate every test and golden mapping run: python3 scripts/hellocodenameone/conformance/port_status.py validate + # The reports under docs/website/data/port_status_reports are CI output. + # Nothing about registering a test requires editing one -- each port picks + # the test up on its next master run -- so a change to a report's results + # that keeps the stamp naming the run is a forged result, not a fix. + - name: Refuse hand-edited port status reports + env: + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + run: | + if [ -n "${BASE_SHA}" ] && [ "${BASE_SHA}" != "0000000000000000000000000000000000000000" ]; then + git fetch --no-tags --depth=1 origin "${BASE_SHA}" || true + fi + ./scripts/hellocodenameone/conformance/check_port_status_provenance.sh "${BASE_SHA:-HEAD^}" - name: Run normalizer tests working-directory: scripts/hellocodenameone/conformance run: python3 -m unittest -v test_port_status.py diff --git a/docs/website/data/port_status_reports/android.json b/docs/website/data/port_status_reports/android.json index b73e72f53ff..07ee927ebc0 100644 --- a/docs/website/data/port_status_reports/android.json +++ b/docs/website/data/port_status_reports/android.json @@ -1,48 +1,48 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T10:47:47Z", + "commit": "73bd710543cba1d6728b9e0e92d436fb0d95580a", + "generated_at": "2026-08-23T11:55:17Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 207587681 + "duration_ns": 275236406 }, "arraySequential": { "checksum": "0", - "duration_ns": 25544773 + "duration_ns": 34295970 }, "hashMapChurn": { "checksum": "49941", - "duration_ns": 233590607 + "duration_ns": 303707003 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 166166998 + "duration_ns": 244566905 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 87938220 + "duration_ns": 142422396 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 2460716848 + "duration_ns": 5489752590 }, "objectAllocation": { "checksum": "2999790376128", - "duration_ns": 114265882 + "duration_ns": 148232049 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 127842947 + "duration_ns": 151270219 }, "recursion": { "checksum": "33385282", - "duration_ns": 490740312 + "duration_ns": 752193112 }, "stringBuilding": { "checksum": "-609121604069", - "duration_ns": 217409339 + "duration_ns": 311457117 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -52,14 +52,14 @@ "suite_checksum": 0 }, "port": "android", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868198", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32636698063", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 179, "fail": 0, - "skip": 1, - "not-run": 0 + "not-run": 0, + "pass": 179, + "skip": 1 }, "tests": { "ARApiTest": { diff --git a/docs/website/data/port_status_reports/ios-gl.json b/docs/website/data/port_status_reports/ios-gl.json index 0782f7142e2..15dc8f05595 100644 --- a/docs/website/data/port_status_reports/ios-gl.json +++ b/docs/website/data/port_status_reports/ios-gl.json @@ -1,36 +1,36 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T15:32:20Z", + "commit": "c2732fd24ff606421f361f239792cc2d717ce47d", + "generated_at": "2026-08-23T11:14:46Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 390405000 + "duration_ns": 342584000 }, "arraySequential": { "checksum": "0", - "duration_ns": 30327000 + "duration_ns": 24001000 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 144064000 + "duration_ns": 121546000 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 84448000 + "duration_ns": 78571000 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 414960000 + "duration_ns": 309299000 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 221066000 + "duration_ns": 161716000 }, "recursion": { "checksum": "33385282", - "duration_ns": 236629000 + "duration_ns": 190209000 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -44,14 +44,14 @@ "suite_checksum": 0 }, "port": "ios-gl", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868735", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32613256755", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 176, "fail": 0, - "skip": 4, - "not-run": 0 + "not-run": 0, + "pass": 175, + "skip": 4 }, "tests": { "ARApiTest": { @@ -453,10 +453,6 @@ "feature": "notifications", "status": "pass" }, - "LogSubclassCaptureTest": { - "feature": "logging-diagnostics", - "status": "pass" - }, "LottieAnimatedScreenshotTest": { "feature": "svg-lottie", "status": "pass" diff --git a/docs/website/data/port_status_reports/ios-metal.json b/docs/website/data/port_status_reports/ios-metal.json index 90ac2753d2f..0ec10c5ed90 100644 --- a/docs/website/data/port_status_reports/ios-metal.json +++ b/docs/website/data/port_status_reports/ios-metal.json @@ -1,36 +1,36 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T14:15:59Z", + "commit": "c2732fd24ff606421f361f239792cc2d717ce47d", + "generated_at": "2026-08-23T10:35:55Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 294811000 + "duration_ns": 312898000 }, "arraySequential": { "checksum": "0", - "duration_ns": 22420000 + "duration_ns": 23531000 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 127686000 + "duration_ns": 142431000 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 77184000 + "duration_ns": 94978000 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 302310000 + "duration_ns": 393993000 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 124076000 + "duration_ns": 139869000 }, "recursion": { "checksum": "33385282", - "duration_ns": 151972000 + "duration_ns": 175892000 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -44,14 +44,14 @@ "suite_checksum": 0 }, "port": "ios-metal", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868735", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32613256755", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 176, "fail": 0, - "skip": 4, - "not-run": 0 + "not-run": 0, + "pass": 175, + "skip": 4 }, "tests": { "ARApiTest": { @@ -453,10 +453,6 @@ "feature": "notifications", "status": "pass" }, - "LogSubclassCaptureTest": { - "feature": "logging-diagnostics", - "status": "pass" - }, "LottieAnimatedScreenshotTest": { "feature": "svg-lottie", "status": "pass" diff --git a/docs/website/data/port_status_reports/javascript.json b/docs/website/data/port_status_reports/javascript.json index cac9ede04bb..83ac4ec3bf9 100644 --- a/docs/website/data/port_status_reports/javascript.json +++ b/docs/website/data/port_status_reports/javascript.json @@ -1,48 +1,48 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T11:06:56Z", + "commit": "73bd710543cba1d6728b9e0e92d436fb0d95580a", + "generated_at": "2026-08-23T12:08:16Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 1235399999 + "duration_ns": 939500000 }, "arraySequential": { "checksum": "0", - "duration_ns": 974200000 + "duration_ns": 735600000 }, "hashMapChurn": { "checksum": "49941", - "duration_ns": 11617100000 + "duration_ns": 7886199999 }, "intArithmetic": { "checksum": "1313580095284", - "duration_ns": 648800001 + "duration_ns": 511700000 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 3307399999 + "duration_ns": 2448200001 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 403899999 + "duration_ns": 284299999 }, "objectAllocation": { "checksum": "2999790376128", - "duration_ns": 2227300001 + "duration_ns": 1654900001 }, "quicksort": { "checksum": "786886890168670967", - "duration_ns": 578600001 + "duration_ns": 435500000 }, "recursion": { "checksum": "33385282", - "duration_ns": 1036300001 + "duration_ns": 959200000 }, "stringBuilding": { "checksum": "-609121604069", - "duration_ns": 1022300000 + "duration_ns": 795500000 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -52,14 +52,14 @@ "suite_checksum": 0 }, "port": "javascript", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868791", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32636698236", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 178, "fail": 0, - "skip": 2, - "not-run": 0 + "not-run": 0, + "pass": 178, + "skip": 2 }, "tests": { "ARApiTest": { diff --git a/docs/website/data/port_status_reports/linux-arm64.json b/docs/website/data/port_status_reports/linux-arm64.json index ec8f3a901c7..ef0b8545899 100644 --- a/docs/website/data/port_status_reports/linux-arm64.json +++ b/docs/website/data/port_status_reports/linux-arm64.json @@ -1,48 +1,48 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T10:56:05Z", + "commit": "73bd710543cba1d6728b9e0e92d436fb0d95580a", + "generated_at": "2026-08-23T12:04:44Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 178201190 + "duration_ns": 169161399 }, "arraySequential": { "checksum": "0", - "duration_ns": 24615262 + "duration_ns": 25734693 }, "hashMapChurn": { "checksum": "49941", - "duration_ns": 35635763 + "duration_ns": 35829660 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 62902952 + "duration_ns": 62856710 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 39311436 + "duration_ns": 39198273 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 274731843 + "duration_ns": 274361241 }, "objectAllocation": { "checksum": "2999790376128", - "duration_ns": 3428129909 + "duration_ns": 2984164556 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 93424737 + "duration_ns": 93609664 }, "recursion": { "checksum": "33385282", - "duration_ns": 137446629 + "duration_ns": 138668513 }, "stringBuilding": { "checksum": "-609121604069", - "duration_ns": 46220590 + "duration_ns": 44702701 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -52,14 +52,14 @@ "suite_checksum": 0 }, "port": "linux-arm64", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868573", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32636698010", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 177, "fail": 0, - "skip": 3, - "not-run": 0 + "not-run": 0, + "pass": 177, + "skip": 3 }, "tests": { "ARApiTest": { diff --git a/docs/website/data/port_status_reports/linux-x64.json b/docs/website/data/port_status_reports/linux-x64.json index f2c2df8ab2b..4fe2a71d8f7 100644 --- a/docs/website/data/port_status_reports/linux-x64.json +++ b/docs/website/data/port_status_reports/linux-x64.json @@ -1,48 +1,48 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T10:56:00Z", + "commit": "73bd710543cba1d6728b9e0e92d436fb0d95580a", + "generated_at": "2026-08-23T12:04:41Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 244264539 + "duration_ns": 239946561 }, "arraySequential": { "checksum": "0", - "duration_ns": 27436409 + "duration_ns": 27210736 }, "hashMapChurn": { "checksum": "49941", - "duration_ns": 39081845 + "duration_ns": 41392475 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 76918383 + "duration_ns": 75971442 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 49941482 + "duration_ns": 48763849 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 178465217 + "duration_ns": 174560158 }, "objectAllocation": { "checksum": "2999790376128", - "duration_ns": 3233252273 + "duration_ns": 2865454230 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 150670050 + "duration_ns": 142840314 }, "recursion": { "checksum": "33385282", - "duration_ns": 182631896 + "duration_ns": 167246876 }, "stringBuilding": { "checksum": "-609121604069", - "duration_ns": 41656108 + "duration_ns": 39554939 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -52,14 +52,14 @@ "suite_checksum": 0 }, "port": "linux-x64", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868573", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32636698010", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 177, "fail": 0, - "skip": 3, - "not-run": 0 + "not-run": 0, + "pass": 177, + "skip": 3 }, "tests": { "ARApiTest": { diff --git a/docs/website/data/port_status_reports/mac-native.json b/docs/website/data/port_status_reports/mac-native.json index 5c39251d12d..6850706451f 100644 --- a/docs/website/data/port_status_reports/mac-native.json +++ b/docs/website/data/port_status_reports/mac-native.json @@ -1,48 +1,48 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T13:42:34Z", + "commit": "c2732fd24ff606421f361f239792cc2d717ce47d", + "generated_at": "2026-08-23T07:14:14Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 282021000 + "duration_ns": 276009000 }, "arraySequential": { "checksum": "0", - "duration_ns": 22970000 + "duration_ns": 20703000 }, "hashMapChurn": { "checksum": "49941", - "duration_ns": 63578000 + "duration_ns": 62537000 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 108942000 + "duration_ns": 122753000 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 68271000 + "duration_ns": 65793000 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 307321000 + "duration_ns": 384003000 }, "objectAllocation": { "checksum": "2999790376128", - "duration_ns": 477974000 + "duration_ns": 8848612000 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 134770000 + "duration_ns": 115990000 }, "recursion": { "checksum": "33385282", - "duration_ns": 181410000 + "duration_ns": 152640000 }, "stringBuilding": { "checksum": "-609121604069", - "duration_ns": 100529000 + "duration_ns": 40571000 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -52,14 +52,14 @@ "suite_checksum": 0 }, "port": "mac-native", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868564", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32613364013", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 175, "fail": 0, - "skip": 5, - "not-run": 0 + "not-run": 0, + "pass": 174, + "skip": 5 }, "tests": { "ARApiTest": { @@ -464,10 +464,6 @@ "feature": "notifications", "status": "pass" }, - "LogSubclassCaptureTest": { - "feature": "logging-diagnostics", - "status": "pass" - }, "LottieAnimatedScreenshotTest": { "feature": "svg-lottie", "status": "pass" diff --git a/docs/website/data/port_status_reports/tvos.json b/docs/website/data/port_status_reports/tvos.json index 8a2bec7e4fa..5ee6d47c9b1 100644 --- a/docs/website/data/port_status_reports/tvos.json +++ b/docs/website/data/port_status_reports/tvos.json @@ -1,36 +1,36 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T14:55:20Z", + "commit": "c2732fd24ff606421f361f239792cc2d717ce47d", + "generated_at": "2026-08-23T10:56:26Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 436593000 + "duration_ns": 461527000 }, "arraySequential": { "checksum": "0", - "duration_ns": 180134000 + "duration_ns": 187070000 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 203189000 + "duration_ns": 299464000 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 127759000 + "duration_ns": 149356000 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 498230000 + "duration_ns": 642187000 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 450828000 + "duration_ns": 471247000 }, "recursion": { "checksum": "33385282", - "duration_ns": 952488000 + "duration_ns": 1007828000 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -44,14 +44,14 @@ "suite_checksum": 0 }, "port": "tvos", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868735", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32613256755", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 172, "fail": 0, - "skip": 8, - "not-run": 0 + "not-run": 0, + "pass": 171, + "skip": 8 }, "tests": { "ARApiTest": { @@ -456,10 +456,6 @@ "feature": "notifications", "status": "pass" }, - "LogSubclassCaptureTest": { - "feature": "logging-diagnostics", - "status": "pass" - }, "LottieAnimatedScreenshotTest": { "feature": "svg-lottie", "status": "pass" diff --git a/docs/website/data/port_status_reports/watchos.json b/docs/website/data/port_status_reports/watchos.json index c46ad7ea75b..574f5ffbede 100644 --- a/docs/website/data/port_status_reports/watchos.json +++ b/docs/website/data/port_status_reports/watchos.json @@ -1,36 +1,36 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T14:40:32Z", + "commit": "c2732fd24ff606421f361f239792cc2d717ce47d", + "generated_at": "2026-08-23T06:25:35Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 433298000 + "duration_ns": 450268000 }, "arraySequential": { "checksum": "0", - "duration_ns": 197790000 + "duration_ns": 193759000 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 209522000 + "duration_ns": 206141000 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 135765000 + "duration_ns": 133625000 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 525986000 + "duration_ns": 575246000 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 432644000 + "duration_ns": 405366000 }, "recursion": { "checksum": "33385282", - "duration_ns": 872982000 + "duration_ns": 980573000 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -44,14 +44,14 @@ "suite_checksum": 0 }, "port": "watchos", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868735", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32613256755", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 167, "fail": 0, - "skip": 13, - "not-run": 0 + "not-run": 0, + "pass": 166, + "skip": 13 }, "tests": { "ARApiTest": { @@ -462,10 +462,6 @@ "feature": "notifications", "status": "pass" }, - "LogSubclassCaptureTest": { - "feature": "logging-diagnostics", - "status": "pass" - }, "LottieAnimatedScreenshotTest": { "feature": "svg-lottie", "status": "pass" diff --git a/docs/website/data/port_status_reports/windows-arm64.json b/docs/website/data/port_status_reports/windows-arm64.json index 4b9093e6fb8..830b3779f56 100644 --- a/docs/website/data/port_status_reports/windows-arm64.json +++ b/docs/website/data/port_status_reports/windows-arm64.json @@ -1,48 +1,48 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T11:00:45Z", + "commit": "73bd710543cba1d6728b9e0e92d436fb0d95580a", + "generated_at": "2026-08-23T12:02:52Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 175137000 + "duration_ns": 176299000 }, "arraySequential": { "checksum": "0", - "duration_ns": 24765000 + "duration_ns": 24874000 }, "hashMapChurn": { "checksum": "49941", - "duration_ns": 57031000 + "duration_ns": 55425000 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 62833000 + "duration_ns": 62781000 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 39182000 + "duration_ns": 39346000 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 220308000 + "duration_ns": 222151000 }, "objectAllocation": { "checksum": "2999790376128", - "duration_ns": 3932948000 + "duration_ns": 3775906000 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 92920000 + "duration_ns": 94655000 }, "recursion": { "checksum": "33385282", - "duration_ns": 132041000 + "duration_ns": 133331000 }, "stringBuilding": { "checksum": "-609121604069", - "duration_ns": 50491000 + "duration_ns": 46497000 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -52,14 +52,14 @@ "suite_checksum": 0 }, "port": "windows-arm64", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868423", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32636698019", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 177, "fail": 0, - "skip": 3, - "not-run": 0 + "not-run": 0, + "pass": 177, + "skip": 3 }, "tests": { "ARApiTest": { diff --git a/docs/website/data/port_status_reports/windows-x64.json b/docs/website/data/port_status_reports/windows-x64.json index 041d97e6062..17a1a0e87d5 100644 --- a/docs/website/data/port_status_reports/windows-x64.json +++ b/docs/website/data/port_status_reports/windows-x64.json @@ -1,48 +1,48 @@ { - "commit": "896356159b4a984b5eb6633e060dff480b7c9ed8", - "generated_at": "2026-08-19T10:48:15Z", + "commit": "73bd710543cba1d6728b9e0e92d436fb0d95580a", + "generated_at": "2026-08-23T11:51:25Z", "performance": { "benchmark_version": 1, "benchmarks": { "arrayRandom": { "checksum": "-2288487891715278", - "duration_ns": 296472000 + "duration_ns": 256791000 }, "arraySequential": { "checksum": "0", - "duration_ns": 25444000 + "duration_ns": 25135000 }, "hashMapChurn": { "checksum": "49941", - "duration_ns": 72049000 + "duration_ns": 69040000 }, "intArithmetic": { "checksum": "1307491170054", - "duration_ns": 87100000 + "duration_ns": 79166000 }, "longArithmetic": { "checksum": "6887886960473257608", - "duration_ns": 63857000 + "duration_ns": 60939000 }, "mathTranscendental": { "checksum": "4729652805076374709", - "duration_ns": 175366000 + "duration_ns": 176072000 }, "objectAllocation": { "checksum": "2999790376128", - "duration_ns": 4228935000 + "duration_ns": 3344991000 }, "quicksort": { "checksum": "809667393311589960", - "duration_ns": 138532000 + "duration_ns": 137912000 }, "recursion": { "checksum": "33385282", - "duration_ns": 226805000 + "duration_ns": 226975000 }, "stringBuilding": { "checksum": "-609121604069", - "duration_ns": 45716000 + "duration_ns": 44624000 } }, "method": "minimum of five measured runs after three in-process warm-ups", @@ -52,14 +52,14 @@ "suite_checksum": 0 }, "port": "windows-x64", - "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32242868555", + "run_url": "https://github.com/codenameone/CodenameOne/actions/runs/32636697893", "schema_version": 1, "suite_finished": true, "summary": { - "pass": 177, "fail": 0, - "skip": 3, - "not-run": 0 + "not-run": 0, + "pass": 177, + "skip": 3 }, "tests": { "ARApiTest": { diff --git a/docs/website/layouts/partials/port-status-feature-status.html b/docs/website/layouts/partials/port-status-feature-status.html index 251843fe25b..5fd038e0664 100644 --- a/docs/website/layouts/partials/port-status-feature-status.html +++ b/docs/website/layouts/partials/port-status-feature-status.html @@ -13,11 +13,18 @@ {{- $failed := 0 -}} {{- $skipped := 0 -}} {{- $notRun := 0 -}} + {{- $awaiting := 0 -}} {{- $failedTests := slice -}} {{- $skippedTests := slice -}} {{- range $feature.tests -}} {{- $result := index $report.tests . -}} - {{- $status := "not-run" -}} + {{- /* Absent and "not-run" are different claims. "not-run" means this port + ran the suite with the test in its contract and nothing reported + back, which is a defect. Absent means the run predates the test -- + the ordinary state of every port for a day after one is registered -- + and calling that a defect is what used to make registering a test + require editing eleven reports. */ -}} + {{- $status := "awaiting" -}} {{- with $result -}}{{- $status = .status -}}{{- end -}} {{- if eq $status "pass" -}} {{- $passed = add $passed 1 -}} @@ -27,6 +34,8 @@ {{- else if eq $status "skip" -}} {{- $skipped = add $skipped 1 -}} {{- $skippedTests = $skippedTests | append . -}} + {{- else if eq $status "awaiting" -}} + {{- $awaiting = add $awaiting 1 -}} {{- else -}} {{- $notRun = add $notRun 1 -}} {{- end -}} @@ -80,7 +89,8 @@ {{- $total := len $feature.tests -}} {{- $state = "partial" -}} {{- $mark = "−" -}} - {{- $label = printf "%d passed, %d skipped, %d not run" $passed $skipped $notRun -}} + {{- $awaitingNote := cond (gt $awaiting 0) (printf ", %d awaiting this port's next run" $awaiting) "" -}} + {{- $label = printf "%d passed, %d skipped, %d not run%s" $passed $skipped $notRun $awaitingNote -}} {{- /* Evidence is per feature. A run that stopped early leaves its own unreached tests as "not run" below, and the port card reports the incomplete run; that is not a reason to withdraw the result of a @@ -104,7 +114,7 @@ {{- $label = printf "%d of %d mapped tests passed; %s skipped by the CI environment, see the skipped-test errata%s" $passed $total (delimit $skippedTests ", ") $incomplete -}} {{- end -}} {{- else if not $complete -}} - {{- $label = printf "Suite did not finish; %d passed, %d skipped, %d not run" $passed $skipped $notRun -}} + {{- $label = printf "Suite did not finish; %d passed, %d skipped, %d not run%s" $passed $skipped $notRun $awaitingNote -}} {{- else if eq $skipped $total -}} {{- $label = "All mapped tests skipped" -}} {{- end -}} diff --git a/docs/website/layouts/partials/port-status-port-state.html b/docs/website/layouts/partials/port-status-port-state.html index 7250185b0d0..817f0a6f27b 100644 --- a/docs/website/layouts/partials/port-status-port-state.html +++ b/docs/website/layouts/partials/port-status-port-state.html @@ -4,9 +4,20 @@ {{- $now := .now -}} {{- $state := "unknown" -}} {{- $label := "No stored report" -}} +{{- $awaiting := 0 -}} {{- if $report -}} {{- $failed := int (default 0 $report.summary.fail) -}} {{- $notRun := int (default 0 (index $report.summary "not-run")) -}} + {{- /* A report counts the tests it carries. Between the commit that registers + a test and this port's next run the report legitimately has no entry + for it, and summing only what the report holds would present that as a + complete answer to a contract it has not seen. Say how many tests this + port has yet to reach instead. */ -}} + {{- range $contract.features -}} + {{- range .tests -}} + {{- if not (index $report.tests .) -}}{{- $awaiting = add $awaiting 1 -}}{{- end -}} + {{- end -}} + {{- end -}} {{- $bootstrapComplete := and (eq $report.bootstrap_source "successful-master-workflow") (eq $report.workflow_conclusion "success") -}} {{- $complete := or $report.suite_finished $bootstrapComplete -}} {{- $generated := time.AsTime $report.generated_at -}} @@ -41,7 +52,7 @@

{{ $port.name }}

{{ $label }}

{{- if $report }}

- {{ $report.summary.pass }} passed · {{ $report.summary.skip }} skipped · {{ index $report.summary "not-run" }} not run + {{ $report.summary.pass }} passed · {{ $report.summary.skip }} skipped · {{ index $report.summary "not-run" }} not run{{ if gt $awaiting 0 }} · {{ $awaiting }} awaiting this port's next run{{ end }}

{{- end }} diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index c50f15c6bdd..fb126e31626 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -124,6 +124,31 @@ When adding a conformance test: . Run the validator before pushing. The `Validate port status contract` workflow enforces the same rules in CI. +**Do not touch `docs/website/data/port_status_reports/`.** Those files are CI +output -- a snapshot of what one run measured -- and a branch has no result to +put in them. Each port picks a newly registered test up on its next `master` +run; until then the validator prints `checked-in snapshot : report +predates tests: ` and the public table shows that cell as not run, which +is what is true. The reports the site actually serves come from the +`port-status-data` branch, not from these. + +This is a deliberate reversal. The contract used to require every checked-in +report to carry an entry for every registered test, so adding one test meant +editing eleven report files and retyping their totals -- which made every +test-adding branch conflict with every other one, and made the cheapest way to +a green build typing `pass` next to a test no port had run. Twelve results +reached `master` that way, attributed to runs that predated the tests. `Validate +port status contract` now refuses a report whose results changed while the +`generated_at`, `commit` and `run_url` naming the run stayed put. + +"All tests run on all ports" is still enforced, just where it cannot be typed: +the nightly sweep holds the *published* reports to it. A test left at `not-run` +fails the sweep, and so does a test missing from one port's report when an +earlier run on another port already covered it -- that port dropped the test +rather than merely predating it. The single permitted exception is a `skip` the +suite itself emits, with an erratum in `port_status_supplement.json` explaining +it. + The on-device AI rows are permanent assertion tests rather than screenshots. They validate immutable image, camera-frame, tensor, model-source, and options contracts on every translated runtime; query every native analyzer or service; diff --git a/scripts/hellocodenameone/conformance/backfill_port_status.sh b/scripts/hellocodenameone/conformance/backfill_port_status.sh index 5f9cec24dd0..ec0c1270765 100755 --- a/scripts/hellocodenameone/conformance/backfill_port_status.sh +++ b/scripts/hellocodenameone/conformance/backfill_port_status.sh @@ -404,12 +404,15 @@ echo "Port status sweep: published ${published} report(s), ${skipped} already cu # table, which is exactly the failure this sweep exists to prevent. stale_days="$(jq -r '.stale_after_days' "${MANIFEST}")" problems=() +published_dir="${tmp_dir}/published" +mkdir -p "${published_dir}" while IFS= read -r port; do if ! gh api "repos/${GITHUB_REPOSITORY}/contents/ports/${port}.json?ref=${DATA_BRANCH}" \ --jq '.content' 2>/dev/null | decode_base64 > "${tmp_dir}/check.json" 2>/dev/null; then problems+=("${port}: no published report") continue fi + cp "${tmp_dir}/check.json" "${published_dir}/${port}.json" # Freshness alone is not enough: a published report the website rejects # leaves the column on its checked-in fallback, which is the state this # sweep exists to detect. @@ -468,3 +471,12 @@ if [ ${#unusable[@]} -gt 0 ]; then fi echo "Every port in the contract has a report inside the ${stale_days}-day window." + +# Freshness says the port reported; it does not say the port reported on every +# test. That obligation used to be enforced against the checked-in fallbacks, +# where a branch could satisfy it by typing "pass" -- so it is enforced here +# instead, against what the ports actually published, where nothing anyone +# writes in a pull request can reach it. A registered test runs on every port, +# and the only permitted exception is a skip the suite itself emits with an +# erratum explaining it. +python3 "${SCRIPT_DIR}/port_status.py" coverage --reports "${published_dir}" diff --git a/scripts/hellocodenameone/conformance/check_port_status_provenance.sh b/scripts/hellocodenameone/conformance/check_port_status_provenance.sh new file mode 100755 index 00000000000..47259ff6b3e --- /dev/null +++ b/scripts/hellocodenameone/conformance/check_port_status_provenance.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. +# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. +# This code is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License version 2 only, as +# published by the Free Software Foundation. Codename One designates this +# particular file as subject to the "Classpath" exception as provided +# by Oracle in the LICENSE file that accompanied this code. +# +# This code is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +# version 2 for more details (a copy is included in the LICENSE file that +# accompanied this code). +# +# You should have received a copy of the GNU General Public License version +# 2 along with this work; if not, write to the Free Software Foundation, +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Please contact Codename One through http://www.codenameone.com/ if you +# need additional information or have any questions. + +set -euo pipefail + +# Refuse a port status report whose results were edited without a new run. +# +# These files are CI output. A branch never has a reason to change one -- not +# even the branch that registers a new test, because each port picks the test up +# on its next master run and the page shows the gap honestly until it does. The +# check exists because the previous contract *required* the edit, and the +# cheapest way to satisfy it was to type "pass" next to a test no port had run. +# +# Usage: check_port_status_provenance.sh + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/../../.." && pwd)" +MANIFEST="${REPO_ROOT}/docs/website/data/port_status.json" +REPORT_PATH="docs/website/data/port_status_reports" + +base_ref="${1:-}" +if [ -z "${base_ref}" ]; then + echo "Usage: $(basename "$0") " >&2 + exit 2 +fi + +if ! git -C "${REPO_ROOT}" rev-parse --verify --quiet "${base_ref}^{commit}" >/dev/null; then + # A shallow clone routinely lacks the base commit. Fetching it is the caller's + # job; without it there is nothing to compare against, and inventing a verdict + # either way would be worse than saying so. + echo "Base revision ${base_ref} is not available; skipping the provenance check." >&2 + exit 0 +fi + +tmp_dir="$(mktemp -d)" +trap 'rm -rf "${tmp_dir}"' EXIT + +while IFS= read -r port; do + git -C "${REPO_ROOT}" show "${base_ref}:${REPORT_PATH}/${port}.json" \ + > "${tmp_dir}/${port}.json" 2>/dev/null \ + || rm -f "${tmp_dir}/${port}.json" +done < <(jq -r '.ports[].id' "${MANIFEST}") + +python3 "${SCRIPT_DIR}/port_status.py" provenance --base "${tmp_dir}" diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 9a40d76b56b..1f2cf71eaf3 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -186,71 +186,16 @@ def validate(manifest: dict) -> dict: if owner is None: problems.append(f"Golden screenshot {name} is not mapped to a test") - skipped_tests: set[str] = set() - report_directory = manifest.get("report_directory") - if report_directory: - report_root = REPO_ROOT / report_directory - for port_id in port_ids: - report_path = report_root / f"{port_id}.json" - try: - report = read_json(report_path) - except ContractError as exc: - problems.append(str(exc)) - continue - if report.get("schema_version") != manifest.get("schema_version"): - problems.append(f"Stored report {report_path} has the wrong schema version") - if report.get("port") != port_id: - problems.append(f"Stored report {report_path} identifies port {report.get('port')}") - report_tests = report.get("tests") - if not isinstance(report_tests, dict): - problems.append(f"Stored report {report_path} has no test result map") - continue - unknown_tests = sorted(set(report_tests) - set(mapped)) - if unknown_tests: - problems.append( - f"Stored report {report_path} contains unknown tests: " - + ", ".join(unknown_tests) - ) - missing_tests = sorted(set(mapped) - set(report_tests)) - if missing_tests: - problems.append( - f"Stored report {report_path} is missing tests: " - + ", ".join(missing_tests) - ) - unrun_tests = [] - for test, result in report_tests.items(): - if not isinstance(result, dict) or result.get("status") not in { - "pass", "fail", "skip", "not-run" - }: - problems.append(f"Stored report {report_path} has an invalid result for {test}") - elif result.get("status") == "skip": - skipped_tests.add(test) - elif result.get("status") == "not-run": - unrun_tests.append(test) - if unrun_tests: - # A registered test that never started is indistinguishable, on the page, from one - # that runs and passes -- nothing here objected to it, so a test could be - # published and quietly never executed on any port. A port that genuinely cannot - # do something reports "skip", from the suite itself, and is unaffected; "not-run" - # is the absence of evidence, and the answer to it is to run the suite and check - # the report in rather than to record the absence. - problems.append( - f"Stored report {report_path} reports tests that never ran: " - + ", ".join(sorted(unrun_tests)) - ) - actual_summary = Counter( - result.get("status") - for result in report_tests.values() - if isinstance(result, dict) - ) - expected_summary = { - key: actual_summary.get(key, 0) - for key in ("pass", "fail", "skip", "not-run") - } - if report.get("summary") != expected_summary: - problems.append( - f"Stored report {report_path} summary does not match its test results" - ) + # The checked-in reports are a snapshot of what CI measured, not a second + # copy of the contract. Requiring them to carry exactly the manifest's test + # set made every test-adding PR hand-edit eleven files, and the cheapest way + # to satisfy that was to invent a result -- twelve "pass" entries reached + # master attributed to runs that never executed the test. Classify them with + # the same drift / malformed split publication uses: a snapshot that predates + # a test is drift and is reported, never fatal; a snapshot nothing can render + # is still a defect. + skipped_tests, snapshot_drift, snapshot_malformed = stored_report_problems(manifest) + problems.extend(snapshot_malformed) manual_feature_count = 0 try: @@ -273,7 +218,10 @@ def validate(manifest: dict) -> dict: "Skip errata references unknown tests: " + ", ".join(unknown_skip_reasons) ) - missing_skip_reasons = sorted(skipped_tests - set(skip_reason_tests)) + # Only tests the contract still defines. A snapshot taken before a test was + # retired still carries its skip, and demanding an erratum for something + # nobody can run again would be unfixable except by editing the snapshot. + missing_skip_reasons = sorted((skipped_tests & set(mapped)) - set(skip_reason_tests)) if missing_skip_reasons: problems.append("Skipped tests without errata: " + ", ".join(missing_skip_reasons)) for item in supplement.get("skip_reasons", []): @@ -387,6 +335,7 @@ def validate(manifest: dict) -> dict: if problems: raise ContractError("\n".join(problems)) return { + "drift": snapshot_drift, "ports": len(ports), "features": len(features), "tests": len(mapped), @@ -398,6 +347,144 @@ def validate(manifest: dict) -> dict: } +def stored_report_problems(manifest: dict) -> tuple[set[str], list[str], list[str]]: + """Classify the checked-in fallback reports. + + Returns (skipped tests, drift, malformed). The reports under + ``report_directory`` are produced by the port workflows and refreshed from + the data branch before Hugo runs; nothing about a pull request is supposed + to touch them. Read them exactly the way publication reads a persisted + report, so "this snapshot predates a test the branch just registered" is the + ordinary, expected state it already is everywhere else in this pipeline + rather than a build failure a human resolves by inventing a result. + """ + skipped: set[str] = set() + drift: list[str] = [] + malformed: list[str] = [] + report_directory = manifest.get("report_directory") + if not report_directory: + return skipped, drift, malformed + root = REPO_ROOT / report_directory + for port in manifest.get("ports", []): + port_id = port.get("id") + if not port_id: + continue + try: + report = read_json(root / f"{port_id}.json") + except ContractError as exc: + malformed.append(str(exc)) + continue + port_drift, port_malformed = publishable_report_problems(manifest, port_id, report) + drift.extend(f"{port_id}: {item}" for item in port_drift) + malformed.extend(f"{port_id}: {item}" for item in port_malformed) + tests = report.get("tests") + if not isinstance(tests, dict): + continue + for name, result in tests.items(): + if isinstance(result, dict) and result.get("status") == "skip": + skipped.add(name) + return skipped, drift, malformed + + +def report_stamp(report: dict) -> datetime | None: + raw = report.get("generated_at") + if not isinstance(raw, str) or not raw: + return None + try: + stamp = datetime.fromisoformat(raw.replace("Z", "+00:00")) + except ValueError: + return None + return stamp if stamp.tzinfo else None + + +def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: + """Hold the *published* reports to "every registered test runs on every port". + + This is the gate that used to live, badly, in the checked-in snapshots. Two + rules, both decidable from the reports themselves: + + ``not-run`` is always a defect. The suite reached that port, the test was in + its contract, and nothing reported back. + + A test absent from a report is normally just a run that predates it -- the + port has not merged past the commit that registered the test yet. It becomes + a defect the moment some *older* report carries that test: a run that + happened earlier already knew about it, so a later run that does not is a + test the port has dropped rather than one it has not reached. No history + lookup and no grace period to tune; the reports date themselves. + """ + problems: list[str] = [] + mapped = test_to_feature(manifest) + stamps = {port: report_stamp(report) for port, report in reports.items()} + + # The earliest run that proves a test was in the contract. Anything younger + # than this has no excuse for missing it. + known_since: dict[str, datetime] = {} + for port, report in reports.items(): + stamp = stamps.get(port) + tests = report.get("tests") + if stamp is None or not isinstance(tests, dict): + continue + for name in tests: + if name in mapped and (name not in known_since or stamp < known_since[name]): + known_since[name] = stamp + + for port in sorted(reports): + report = reports[port] + tests = report.get("tests") + if not isinstance(tests, dict): + problems.append(f"{port}: report has no test result map") + continue + unrun = sorted( + name + for name, result in tests.items() + if isinstance(result, dict) and result.get("status") == "not-run" + ) + if unrun: + problems.append(f"{port}: reported no result for " + ", ".join(unrun)) + stamp = stamps.get(port) + if stamp is None: + problems.append(f"{port}: report has no usable generated_at") + continue + dropped = sorted( + name + for name in set(mapped) - set(tests) + if name in known_since and known_since[name] < stamp + ) + if dropped: + problems.append( + f"{port}: ran at {report.get('generated_at')} without " + + ", ".join(dropped) + + ", which an earlier run on another port already covered" + ) + return problems + + +PROVENANCE_FIELDS = ("generated_at", "commit", "run_url") + + +def provenance_problems(port_id: str, before: dict, after: dict) -> list[str]: + """Refuse a hand-edited result. + + A report says "at this commit, this run, at this time, these were the + results". Editing the results while leaving that provenance alone does not + correct the record, it forges it -- which is how twelve tests came to be + published as passing on ports that had never executed them. Changing results + is legitimate only as part of taking a new snapshot, and a new snapshot + carries a new stamp. + """ + if before.get("tests") == after.get("tests") and before.get("summary") == after.get("summary"): + return [] + if any(before.get(field) != after.get(field) for field in PROVENANCE_FIELDS): + return [] + return [ + f"{port_id}: test results changed but {', '.join(PROVENANCE_FIELDS)} did not. " + "These reports are CI output -- a branch never needs to edit one. Adding a " + "test needs no report change at all; each port picks it up on its next " + "master run." + ] + + def add_reason(entry: dict, reason: str) -> None: reasons = entry.setdefault("reasons", []) if reason and reason not in reasons: @@ -911,6 +998,28 @@ def build_parser() -> argparse.ArgumentParser: accept_parser.add_argument("--port", required=True) accept_parser.add_argument("--report", required=True, type=Path) + coverage_parser = subparsers.add_parser( + "coverage", + help="hold published reports to running every registered test on every port", + ) + coverage_parser.add_argument( + "--reports", + required=True, + type=Path, + help="directory of published .json reports", + ) + + provenance_parser = subparsers.add_parser( + "provenance", + help="refuse a report whose results were edited without a new run", + ) + provenance_parser.add_argument( + "--base", + required=True, + type=Path, + help="directory holding the base revision's reports", + ) + normalize_parser = subparsers.add_parser("normalize", help="write a normalized port report") normalize_parser.add_argument("--port", required=True) normalize_parser.add_argument("--log", action="append", type=Path, default=[]) @@ -939,6 +1048,52 @@ def main() -> int: f"{counts['tests']} tests, {counts['features']} features, " f"{counts['ports']} ports, {counts['goldens']} golden names." ) + for item in counts["drift"]: + # Information, not a warning to be silenced. Every port reaches + # a newly registered test on its next master run, and the page + # shows the gap as "not run" until it does. + print(f"port-status: checked-in snapshot {item}") + return 0 + if args.command == "coverage": + reports = {} + for port in manifest.get("ports", []): + port_id = port.get("id") + path = args.reports / f"{port_id}.json" + if not path.is_file(): + print(f"port-status: no published report for {port_id}", file=sys.stderr) + return 1 + reports[port_id] = read_json(path) + problems = coverage_problems(manifest, reports) + for problem in problems: + print(f"port-status coverage: {problem}", file=sys.stderr) + if problems: + print( + "Every registered test runs on every port unless the suite itself " + "reports a skip with an erratum. Fix the port or record the skip.", + file=sys.stderr, + ) + return 1 + print(f"Every registered test reported a result on all {len(reports)} ports.") + return 0 + if args.command == "provenance": + report_directory = manifest.get("report_directory") + problems = [] + for port in manifest.get("ports", []): + port_id = port.get("id") + base_path = args.base / f"{port_id}.json" + if not base_path.is_file(): + continue + head_path = REPO_ROOT / report_directory / f"{port_id}.json" + problems.extend( + provenance_problems( + port_id, read_json(base_path), read_json(head_path) + ) + ) + for problem in problems: + print(f"port-status: {problem}", file=sys.stderr) + if problems: + return 1 + print("No port status report was edited by hand.") return 0 if args.command == "accept": drift, malformed = publishable_report_problems( diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index e589a901f76..e116a18bf66 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -16,24 +16,34 @@ def setUpClass(cls): cls.manifest = port_status.read_json(port_status.DEFAULT_MANIFEST) def test_contract_covers_registered_tests_and_goldens(self): + # Deliberately no literal totals. Every one of these used to be a magic + # number that each test-adding branch had to retype, so two branches + # adding a test conflicted here by construction -- on a line whose only + # content was a number neither author had a reason to think about. What + # is worth asserting is the relationship: the suite and the contract + # describe the same set of tests, and nothing is counted twice. counts = port_status.validate(self.manifest) - self.assertEqual(180, counts["tests"]) - self.assertEqual(1, counts["performance_tests"]) - self.assertGreaterEqual(counts["features"], 54) - self.assertEqual(11, counts["ports"]) - self.assertEqual(20, counts["manual_features"]) + registered = port_status.registered_tests() + mapped = port_status.test_to_feature(self.manifest) + performance = self.manifest["performance_tests"] + + self.assertEqual(sorted(registered), sorted(set(registered))) + self.assertEqual(set(registered), set(mapped) | set(performance)) + self.assertEqual(set(), set(mapped) & set(performance)) + self.assertEqual(len(mapped), counts["tests"]) + self.assertEqual(len(performance), counts["performance_tests"]) + self.assertEqual(len(self.manifest["ports"]), counts["ports"]) + self.assertEqual(len(self.manifest["features"]), counts["features"]) + self.assertTrue(all(feature["tests"] for feature in self.manifest["features"])) + + # Floors, not equalities: these guard against a collapse -- a manifest + # that lost its features, a golden directory that stopped resolving -- + # and a branch that adds to any of them never has to touch this file. + self.assertGreater(counts["features"], 50) + self.assertGreater(counts["goldens"], 100) + self.assertGreater(counts["manual_features"], 15) self.assertEqual(8, counts["deployment_platforms"]) self.assertEqual(3, counts["browser_engines"]) - self.assertGreaterEqual(counts["goldens"], 100) - features = {feature["id"]: feature["tests"] for feature in self.manifest["features"]} - self.assertEqual(["ARApiTest", "MotionSensorDeviceTest"], features["ar-motion-sensors"]) - self.assertEqual(["CameraApiTest"], features["camera-access"]) - self.assertEqual(["VisionOnDeviceApiTest"], features["on-device-vision"]) - self.assertEqual(["LanguageOnDeviceApiTest"], features["on-device-language"]) - self.assertEqual(["InferenceOnDeviceApiTest"], features["on-device-inference"]) - self.assertEqual(["CalendarApiTest"], features["calendar-integration"]) - self.assertEqual(["VideoIODecodedFramesScreenshotTest"], features["video-decoding"]) - self.assertEqual(["VideoIORoundTripTest"], features["video-round-trip"]) def test_normalize_preserves_pass_skip_and_screenshot_failure(self): log_text = "\n".join( @@ -313,58 +323,82 @@ def test_validate_rejects_inconsistent_stored_report_summary(self): ) with self.assertRaisesRegex( port_status.ContractError, - "summary does not match its test results", + "summary does not match the test results", ): port_status.validate(manifest) - def test_validate_rejects_a_test_that_never_ran(self): + def stored_reports(self): + directory = port_status.REPO_ROOT / self.manifest["report_directory"] + return { + port["id"]: port_status.read_json(directory / (port["id"] + ".json")) + for port in self.manifest["ports"] + } + + def test_coverage_rejects_a_test_that_never_ran(self): # A registered test left at "not-run" reads on the page exactly like one that runs and - # passes. Seven database tests were published that way -- added to the manifest, never - # run in any stored report -- and nothing here objected. - original_directory = self.manifest["report_directory"] - with tempfile.TemporaryDirectory(dir=port_status.REPO_ROOT) as tmp: - report_root = Path(tmp) - for port in self.manifest["ports"]: - source = port_status.REPO_ROOT / original_directory / ( - port["id"] + ".json" - ) - (report_root / source.name).write_text( - source.read_text(encoding="utf-8"), encoding="utf-8" - ) - android_path = report_root / "android.json" - android = port_status.read_json(android_path) - victim = next( - name - for name, result in android["tests"].items() - if result.get("status") == "pass" - ) - android["tests"][victim]["status"] = "not-run" - android["summary"]["pass"] -= 1 - android["summary"]["not-run"] += 1 - android_path.write_text( - json.dumps(android, indent=2, sort_keys=True) + "\n", - encoding="utf-8", - ) - manifest = dict(self.manifest) - manifest["report_directory"] = str( - report_root.relative_to(port_status.REPO_ROOT) - ) - with self.assertRaisesRegex( - port_status.ContractError, - "reports tests that never ran: " + victim, - ): - port_status.validate(manifest) + # passes. This used to be asserted against the checked-in snapshots, which is why every + # branch that registered a test had to edit eleven of them -- and why the cheapest way + # to go green was to type "pass" for a run that never happened. The obligation belongs + # to the reports the ports actually publish, where nothing a branch writes can satisfy it. + reports = self.stored_reports() + victim = next( + name + for name, result in reports["android"]["tests"].items() + if result.get("status") == "pass" + ) + reports["android"]["tests"][victim]["status"] = "not-run" + problems = port_status.coverage_problems(self.manifest, reports) + self.assertTrue( + any("android" in problem and victim in problem for problem in problems), + problems, + ) - def test_validate_accepts_a_test_the_port_skipped(self): + def test_coverage_accepts_a_test_the_port_skipped(self): # The distinction the rule turns on: a port that genuinely cannot do something reports # "skip" from the suite itself, which is evidence rather than the absence of it. + reports = self.stored_reports() + victim = next( + name + for name, result in reports["android"]["tests"].items() + if result.get("status") == "pass" + ) + reports["android"]["tests"][victim]["status"] = "skip" + self.assertEqual([], port_status.coverage_problems(self.manifest, reports)) + + def test_coverage_accepts_a_report_older_than_the_test(self): + # The state every port is in between the commit that registers a test and that port's + # next master run. Failing here would put the old ritual straight back: the only way to + # merge a test would be to make eleven reports claim a result for it first. + reports = self.stored_reports() + newest = max(reports, key=lambda port: reports[port]["generated_at"]) + oldest = min(reports, key=lambda port: reports[port]["generated_at"]) + self.assertNotEqual(newest, oldest) + victim = next(iter(reports[newest]["tests"])) + del reports[oldest]["tests"][victim] + self.assertEqual([], port_status.coverage_problems(self.manifest, reports)) + + def test_coverage_rejects_a_test_a_later_run_dropped(self): + # The other half of the same comparison. A run that happened after another run which + # already covered the test has no "my contract predates it" excuse left. + reports = self.stored_reports() + newest = max(reports, key=lambda port: reports[port]["generated_at"]) + oldest = min(reports, key=lambda port: reports[port]["generated_at"]) + victim = next(iter(reports[oldest]["tests"])) + del reports[newest]["tests"][victim] + problems = port_status.coverage_problems(self.manifest, reports) + self.assertTrue( + any(newest in problem and victim in problem for problem in problems), + problems, + ) + + def test_validate_tolerates_a_snapshot_that_predates_a_test(self): + # Registering a test must not require touching a single report. This is the assertion + # that keeps it that way. original_directory = self.manifest["report_directory"] with tempfile.TemporaryDirectory(dir=port_status.REPO_ROOT) as tmp: report_root = Path(tmp) for port in self.manifest["ports"]: - source = port_status.REPO_ROOT / original_directory / ( - port["id"] + ".json" - ) + source = port_status.REPO_ROOT / original_directory / (port["id"] + ".json") (report_root / source.name).write_text( source.read_text(encoding="utf-8"), encoding="utf-8" ) @@ -375,23 +409,42 @@ def test_validate_accepts_a_test_the_port_skipped(self): for name, result in android["tests"].items() if result.get("status") == "pass" ) - android["tests"][victim]["status"] = "skip" + del android["tests"][victim] android["summary"]["pass"] -= 1 - android["summary"]["skip"] += 1 android_path.write_text( - json.dumps(android, indent=2, sort_keys=True) + "\n", - encoding="utf-8", + json.dumps(android, indent=2, sort_keys=True) + "\n", encoding="utf-8" ) manifest = dict(self.manifest) - manifest["report_directory"] = str( - report_root.relative_to(port_status.REPO_ROOT) - ) - # Asserted against this rule alone: a skip carries its own separate obligation -- - # errata explaining it -- and that is what would be reported here instead. - try: - port_status.validate(manifest) - except port_status.ContractError as exc: - self.assertNotIn("never ran", str(exc)) + manifest["report_directory"] = str(report_root.relative_to(port_status.REPO_ROOT)) + counts = port_status.validate(manifest) + self.assertTrue( + any("android" in item and victim in item for item in counts["drift"]), + counts["drift"], + ) + + def test_provenance_rejects_results_edited_without_a_new_run(self): + # Exactly the edit twelve published "passes" were made by: a test entry appended to a + # report, its summary bumped, and the stamp naming the run left untouched. + before = self.stored_reports()["android"] + after = json.loads(json.dumps(before)) + after["tests"]["SomeBrandNewTest"] = {"feature": "crypto", "status": "pass"} + after["summary"]["pass"] += 1 + problems = port_status.provenance_problems("android", before, after) + self.assertEqual(1, len(problems), problems) + self.assertIn("CI output", problems[0]) + + def test_provenance_accepts_a_genuinely_newer_snapshot(self): + before = self.stored_reports()["android"] + after = json.loads(json.dumps(before)) + after["tests"]["SomeBrandNewTest"] = {"feature": "crypto", "status": "pass"} + after["summary"]["pass"] += 1 + after["generated_at"] = "2026-12-31T00:00:00Z" + after["commit"] = "0123456789abcdef" + self.assertEqual([], port_status.provenance_problems("android", before, after)) + + def test_provenance_ignores_an_untouched_report(self): + before = self.stored_reports()["android"] + self.assertEqual([], port_status.provenance_problems("android", before, dict(before))) def publishable_report(self, port_id, **overrides): mapped = port_status.test_to_feature(self.manifest) @@ -799,17 +852,21 @@ def test_publishable_rejects_a_malformed_performance_status_when_unfinished(self self.assertTrue( any("performance status is" in item for item in malformed), malformed) - def test_publishable_matches_every_report_the_site_serves(self): + def test_every_report_the_site_serves_is_renderable(self): + # Only malformed. Drift is asserted against deliberately: a checked-in snapshot is a + # copy of a real run, and a branch that registers a test makes every one of them + # predate it. Demanding zero drift here is what turned "add a test" into "edit eleven + # reports", and what made inventing a result the path of least resistance. What the + # fallback owes the site is that Hugo can render it. for port in self.manifest["ports"]: report_path = port_status.REPO_ROOT / self.manifest["report_directory"] / ( port["id"] + ".json" ) with self.subTest(port=port["id"]): - drift, malformed = port_status.publishable_report_problems( + _, malformed = port_status.publishable_report_problems( self.manifest, port["id"], port_status.read_json(report_path) ) self.assertEqual([], malformed) - self.assertEqual([], drift) if __name__ == "__main__": From 9264e7762e455f2ded43cfc4100fce889f25d171 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 23 Aug 2026 16:25:21 +0300 Subject: [PATCH 02/14] Let the sweep reach the coverage gate, and hold published skips to their errata Two defects in the gates the previous commit put in place of the hand-edited snapshots. The nightly sweep never reached the coverage gate in the one case it was written for. Its closing assertion ran `accept` on each published report and recorded any nonzero status as a problem, then exited before the coverage command ran. Contract drift is status 11 -- a report built before a newly registered test, which is the state of every port between that merge and its next run -- so a test-adding merge turned the nightly red for a day, and the `known_since` comparison that deliberately tolerates an older report was unreachable in exactly the scenario it exists for. That also made the sweep contradict the candidate loop twenty lines above it, whose comment already says drift "stays quiet, because waiting for a run on the current contract is the intended behaviour there, not a defect". Drift is logged and falls through to the freshness check now; only a report the website cannot use is a defect. The failure predates this branch: ios-gl, ios-metal, mac-native, tvos and watchos are on the data branch at a commit before LogSubclassCaptureTest, so the sweep is failing on master today for that reason. The coverage gate also accepted any `skip` as a satisfactory result, so a port could stop running a test by marking it skipped and publishing -- the exception the gate documents is a skip *with an erratum*, and nothing checked. validate() could not cover this: it reads skips out of the checked-in fallbacks, not out of the reports the ports publish, so the first symptom would have been a failed website build rather than the name of the port that started skipping. Coverage now applies the page's own rule, `skip_is_documented`: the erratum has to name the test and, where it lists reason codes, account for every reason that run gave from a port that code applies to. Matching on the test name alone would let any future skip of a named test read as documented -- CameraApiTest has errata, but the missing-webcam code is written about Windows, and the same code arriving from Linux says something nobody has explained. The eleven published reports pass the stricter rule unchanged. Four tests cover it: a documented skip, an undocumented one, a reason code scoped to another port, and a skip carrying no reason at all. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/hellocodenameone/README.adoc | 14 ++++- .../conformance/backfill_port_status.sh | 11 +++- .../conformance/port_status.py | 54 ++++++++++++++++ .../conformance/test_port_status.py | 63 +++++++++++++++++-- 4 files changed, 135 insertions(+), 7 deletions(-) diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index fb126e31626..810d9741c04 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -146,8 +146,18 @@ the nightly sweep holds the *published* reports to it. A test left at `not-run` fails the sweep, and so does a test missing from one port's report when an earlier run on another port already covered it -- that port dropped the test rather than merely predating it. The single permitted exception is a `skip` the -suite itself emits, with an erratum in `port_status_supplement.json` explaining -it. +suite itself emits, and the sweep checks it against +`port_status_supplement.json` the way the page does: the erratum has to name the +test *and*, where it lists reason codes, account for the reason that run gave +from a port that code applies to. A port that quietly starts skipping a test +fails the sweep by name rather than surfacing later as a failed website build. + +The sweep's closing assertion treats contract drift as the ordinary state it is, +matching the candidate loop above it. It used to count a port whose published +report predates a newly registered test as a failure, so the nightly went red +for a day after any test was added -- and returned before the coverage gate +could read the reports whose "this run predates the test" case that gate exists +to tolerate. The on-device AI rows are permanent assertion tests rather than screenshots. They validate immutable image, camera-frame, tensor, model-source, and options diff --git a/scripts/hellocodenameone/conformance/backfill_port_status.sh b/scripts/hellocodenameone/conformance/backfill_port_status.sh index ec0c1270765..a0144f43132 100755 --- a/scripts/hellocodenameone/conformance/backfill_port_status.sh +++ b/scripts/hellocodenameone/conformance/backfill_port_status.sh @@ -418,7 +418,16 @@ while IFS= read -r port; do # sweep exists to detect. accept_status=0 python3 "${SCRIPT_DIR}/port_status.py" accept --port "${port}" --report "${tmp_dir}/check.json" >/dev/null || accept_status=$? - if [ "${accept_status}" -ne 0 ]; then + # Contract drift is not a problem here, for the same reason the candidate loop + # above says it is not one: a report built before a newly registered test is + # the expected state of every port between that merge and its next run. This + # loop used to call it one, so the sweep failed for a day after any test was + # added -- and, worse, exited before the coverage gate below could look at the + # very reports whose "the run predates the test" case that gate exists to + # tolerate. Only a report the website cannot use is a defect. + if [ "${accept_status}" -eq "${ACCEPT_CONTRACT_DRIFT}" ]; then + echo "${port}: published report is $(describe_accept_status "${accept_status}")." >&2 + elif [ "${accept_status}" -ne 0 ]; then problems+=("${port}: published report is $(describe_accept_status "${accept_status}")") continue fi diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 1f2cf71eaf3..69fa46c0e12 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -397,6 +397,37 @@ def report_stamp(report: dict) -> datetime | None: return stamp if stamp.tzinfo else None +def skip_is_documented(supplement: dict, port_id: str, test: str, reasons: list) -> bool: + """The page's rule for a green documented skip, applied to a report. + + Mirrors port-status-feature-status.html deliberately: an erratum documents a + skip only when it names the test AND, where it lists reason codes, every + reason the run gave matches one of them from a port that code applies to. + Matching on the test name alone would let any future skip of a named test + read as documented -- an encoder that regressed would render green under an + erratum written about a simulator. + """ + for item in supplement.get("skip_reasons", []): + if item.get("test") != test: + continue + codes = item.get("reason_codes") + if not codes: + return True + if not reasons: + continue + if all( + any( + (not code.get("ports") or port_id in code["ports"]) + and isinstance(reason, str) + and reason.startswith(code.get("prefix", "")) + for code in codes + ) + for reason in reasons + ): + return True + return False + + def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: """Hold the *published* reports to "every registered test runs on every port". @@ -412,8 +443,16 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: happened earlier already knew about it, so a later run that does not is a test the port has dropped rather than one it has not reached. No history lookup and no grace period to tune; the reports date themselves. + + A ``skip`` is the one permitted exception, and only with an erratum that + accounts for the reason the run actually gave. Reading skips out of the + checked-in fallbacks instead -- which is all validate() can see -- would let + a port start skipping a test, publish it, and pass this gate, with the + undocumented skip surfacing later as a failed website build rather than as + the name of the port that started skipping. """ problems: list[str] = [] + supplement = read_json(SUPPLEMENT) mapped = test_to_feature(manifest) stamps = {port: report_stamp(report) for port, report in reports.items()} @@ -442,6 +481,21 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: ) if unrun: problems.append(f"{port}: reported no result for " + ", ".join(unrun)) + undocumented = sorted( + name + for name, result in tests.items() + if isinstance(result, dict) + and result.get("status") == "skip" + and name in mapped + and not skip_is_documented( + supplement, port, name, result.get("reasons") or [] + ) + ) + if undocumented: + problems.append( + f"{port}: skipped without an erratum that explains the reason given: " + + ", ".join(undocumented) + ) stamp = stamps.get(port) if stamp is None: problems.append(f"{port}: report has no usable generated_at") diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index e116a18bf66..181da0cf8e0 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -353,17 +353,72 @@ def test_coverage_rejects_a_test_that_never_ran(self): problems, ) - def test_coverage_accepts_a_test_the_port_skipped(self): + def test_coverage_accepts_a_documented_skip(self): # The distinction the rule turns on: a port that genuinely cannot do something reports - # "skip" from the suite itself, which is evidence rather than the absence of it. + # "skip" from the suite itself, which is evidence rather than the absence of it -- but + # only where an erratum accounts for the reason the run gave. The published reports + # carry these already, so the shipped data is the fixture. + reports = self.stored_reports() + documented = [ + (port, name) + for port, report in reports.items() + for name, result in report["tests"].items() + if result.get("status") == "skip" + ] + self.assertTrue(documented) + self.assertEqual([], port_status.coverage_problems(self.manifest, reports)) + + def test_coverage_rejects_an_undocumented_skip(self): + # Otherwise a port can simply stop running a test: mark it skipped, publish, and this + # gate calls it a satisfactory result. validate() cannot catch it either -- it reads + # the checked-in fallbacks, not what the ports published -- so the first symptom would + # be a failed website build rather than the name of the port that started skipping. reports = self.stored_reports() victim = next( name for name, result in reports["android"]["tests"].items() if result.get("status") == "pass" ) - reports["android"]["tests"][victim]["status"] = "skip" - self.assertEqual([], port_status.coverage_problems(self.manifest, reports)) + reports["android"]["tests"][victim] = { + "feature": reports["android"]["tests"][victim]["feature"], + "status": "skip", + "reasons": ["something-nobody-wrote-down"], + } + problems = port_status.coverage_problems(self.manifest, reports) + self.assertTrue( + any("android" in problem and victim in problem for problem in problems), + problems, + ) + + def test_coverage_rejects_a_skip_reason_scoped_to_another_port(self): + # Matching the test name alone would let any future skip of a named test read as + # documented. CameraApiTest has errata, but the missing-webcam code is written about + # Windows; the same code from Linux says something nobody has explained. + reports = self.stored_reports() + reports["linux-x64"]["tests"]["CameraApiTest"] = { + "feature": "camera-access", + "status": "skip", + "reasons": ["no-host-webcam-capture-on-win"], + } + problems = port_status.coverage_problems(self.manifest, reports) + self.assertTrue( + any("linux-x64" in problem and "CameraApiTest" in problem for problem in problems), + problems, + ) + + def test_coverage_rejects_a_skip_carrying_no_reason(self): + # An erratum with reason codes documents the reasons it lists, not the test. A skip + # that names none matches nothing, which is what the page already decides. + reports = self.stored_reports() + reports["android"]["tests"]["CameraApiTest"] = { + "feature": "camera-access", + "status": "skip", + } + problems = port_status.coverage_problems(self.manifest, reports) + self.assertTrue( + any("android" in problem and "CameraApiTest" in problem for problem in problems), + problems, + ) def test_coverage_accepts_a_report_older_than_the_test(self): # The state every port is in between the commit that registers a test and that port's From 675779ecc9850cb8bba1f7c371c5551afdc9ee6b Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 23 Aug 2026 16:37:48 +0300 Subject: [PATCH 03/14] Cover every CI-derived field in the provenance check, not just the test map The check compared `tests` and `summary` and returned "untouched" for anything else, so a branch could rewrite a benchmark duration, the memory figures, the binary size or `suite_finished` in place and the edit would be published as a measurement of the original run. Those are the findings least likely to be doubted, because unlike a test result nobody can check one by reading it: the page presents `performance` as ten measured durations for a named commit, and `suite_finished` is what makes a port card say the suite completed rather than that the run stopped early. Naming a subset of the payload left exactly those as the one thing a branch could still edit without tripping the gate. Everything except `generated_at`, `commit` and `run_url` is a finding now. The message names which fields changed, so the report reads "performance changed but generated_at, commit, run_url did not" rather than making the author diff the file to find out what the gate objected to. No exemption for a schema migration, deliberately: a migration rewrites the producers and the snapshots are then refreshed from the data branch, which moves the stamps and passes on its own. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/hellocodenameone/README.adoc | 6 ++- .../conformance/port_status.py | 38 +++++++++++++------ .../conformance/test_port_status.py | 23 +++++++++++ 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index 810d9741c04..089583ffcdb 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -138,8 +138,10 @@ editing eleven report files and retyping their totals -- which made every test-adding branch conflict with every other one, and made the cheapest way to a green build typing `pass` next to a test no port had run. Twelve results reached `master` that way, attributed to runs that predated the tests. `Validate -port status contract` now refuses a report whose results changed while the -`generated_at`, `commit` and `run_url` naming the run stayed put. +port status contract` now refuses a report whose findings changed while the +`generated_at`, `commit` and `run_url` naming the run stayed put -- every field +but those three, so the benchmark durations the page publishes as measurements +are covered along with the test map. "All tests run on all ports" is still enforced, just where it cannot be typed: the nightly sweep holds the *published* reports to it. A test left at `not-run` diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 69fa46c0e12..c92ba639757 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -518,24 +518,40 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: def provenance_problems(port_id: str, before: dict, after: dict) -> list[str]: - """Refuse a hand-edited result. + """Refuse a hand-edited report. - A report says "at this commit, this run, at this time, these were the - results". Editing the results while leaving that provenance alone does not + A report says "at this commit, this run, at this time, this is what the port + did". Editing what it did while leaving that provenance alone does not correct the record, it forges it -- which is how twelve tests came to be - published as passing on ports that had never executed them. Changing results - is legitimate only as part of taking a new snapshot, and a new snapshot - carries a new stamp. + published as passing on ports that had never executed them. Changing the + findings is legitimate only as part of taking a new snapshot, and a new + snapshot carries a new stamp. + + Everything except the provenance fields counts as a finding, not just the + test map: ``performance`` is the ten benchmark durations the page publishes + as measurements of that run, and ``suite_finished`` is what makes a port card + say the suite completed. Naming a subset here would leave the numbers most + worth doubting -- the ones nobody can check by reading them -- as the one + thing a branch could still rewrite in place. """ - if before.get("tests") == after.get("tests") and before.get("summary") == after.get("summary"): + findings = tuple( + {key: value for key, value in report.items() if key not in PROVENANCE_FIELDS} + for report in (before, after) + ) + if findings[0] == findings[1]: return [] if any(before.get(field) != after.get(field) for field in PROVENANCE_FIELDS): return [] + changed = sorted( + key + for key in set(findings[0]) | set(findings[1]) + if findings[0].get(key) != findings[1].get(key) + ) return [ - f"{port_id}: test results changed but {', '.join(PROVENANCE_FIELDS)} did not. " - "These reports are CI output -- a branch never needs to edit one. Adding a " - "test needs no report change at all; each port picks it up on its next " - "master run." + f"{port_id}: {', '.join(changed)} changed but " + f"{', '.join(PROVENANCE_FIELDS)} did not. These reports are CI output -- " + "a branch never needs to edit one. Adding a test needs no report change " + "at all; each port picks it up on its next master run." ] diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index 181da0cf8e0..61560f39fb1 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -497,6 +497,29 @@ def test_provenance_accepts_a_genuinely_newer_snapshot(self): after["commit"] = "0123456789abcdef" self.assertEqual([], port_status.provenance_problems("android", before, after)) + def test_provenance_rejects_edited_benchmark_measurements(self): + # The findings nobody can check by reading them. A benchmark duration is published as a + # measurement of a named run; rewriting one in place attributes an invented number to + # that run exactly the way the twelve invented passes did. Naming only tests and summary + # would have left performance as the one thing a branch could still edit. + before = self.stored_reports()["android"] + after = json.loads(json.dumps(before)) + workload = next(iter(after["performance"]["benchmarks"])) + after["performance"]["benchmarks"][workload]["duration_ns"] = 1 + problems = port_status.provenance_problems("android", before, after) + self.assertEqual(1, len(problems), problems) + self.assertIn("performance", problems[0]) + + def test_provenance_rejects_an_edited_completion_marker(self): + # suite_finished is what makes a port card say the suite completed rather than that the + # run stopped early. + before = self.stored_reports()["android"] + after = json.loads(json.dumps(before)) + after["suite_finished"] = not before["suite_finished"] + problems = port_status.provenance_problems("android", before, after) + self.assertEqual(1, len(problems), problems) + self.assertIn("suite_finished", problems[0]) + def test_provenance_ignores_an_untouched_report(self): before = self.stored_reports()["android"] self.assertEqual([], port_status.provenance_problems("android", before, dict(before))) From abc4659b9b383eeec4c6cce98689561ad9feaa41 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 23 Aug 2026 17:13:30 +0300 Subject: [PATCH 04/14] Stop a green card over an awaiting column, and a red sweep over a retired test Two inconsistencies where the new rules did not apply themselves evenly. The port card ignored the awaiting count it computed. With the suite finished and nothing failed, a port whose report predates a newly registered test still read "Suite completed" in green -- directly above a feature cell that renders partial for that very test, because the cell has no answer to show. The headline contradicting the column is the shape the card's own existing comment already argues against for not-run. Awaiting selects partial now, last in the chain so a stale report or an incomplete run still names itself first, and with a label that says what is pending rather than what is broken: "1 test awaiting this port's next run". The coverage gate's not-run scan read every entry in a report instead of only the tests the contract still defines, which is the filter the skip check immediately below it already uses. A report that predates a test's retirement and carries that test as not-run therefore failed the sweep over a test nobody can run any more -- while the same report is tolerated as drift everywhere else, so the nightly stayed red until that port happened to rerun for an unrelated reason. Co-Authored-By: Claude Opus 5 (1M context) --- .../layouts/partials/port-status-port-state.html | 8 ++++++++ scripts/hellocodenameone/conformance/port_status.py | 9 ++++++++- .../hellocodenameone/conformance/test_port_status.py | 12 ++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/website/layouts/partials/port-status-port-state.html b/docs/website/layouts/partials/port-status-port-state.html index 817f0a6f27b..4f3729be56a 100644 --- a/docs/website/layouts/partials/port-status-port-state.html +++ b/docs/website/layouts/partials/port-status-port-state.html @@ -42,6 +42,14 @@ {{- else if $stale -}} {{- $state = "partial" -}} {{- $label = "Report is stale" -}} + {{- else if gt $awaiting 0 -}} + {{- /* Last in the chain because it is the mildest of these, but not absent + from it: the feature cell for a test this run predates already renders + partial, and leaving the card green said "Suite completed" directly + above a column that is missing an answer. The suite did complete -- + against a contract that has since grown. */ -}} + {{- $state = "partial" -}} + {{- $label = printf "%d test%s awaiting this port's next run" $awaiting (cond (eq $awaiting 1) "" "s") -}} {{- end -}} {{- end -}}
diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index c92ba639757..32f91776ed9 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -474,10 +474,17 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: if not isinstance(tests, dict): problems.append(f"{port}: report has no test result map") continue + # `name in mapped`, the same filter the skip check below uses. Scanning + # every entry meant a report that predates a test's retirement and + # carries it as not-run failed this gate over a test nobody can run any + # more -- and the same report is tolerated as drift everywhere else, so + # the sweep stayed red until that port happened to rerun. unrun = sorted( name for name, result in tests.items() - if isinstance(result, dict) and result.get("status") == "not-run" + if isinstance(result, dict) + and result.get("status") == "not-run" + and name in mapped ) if unrun: problems.append(f"{port}: reported no result for " + ", ".join(unrun)) diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index 61560f39fb1..49251c41cb2 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -353,6 +353,18 @@ def test_coverage_rejects_a_test_that_never_ran(self): problems, ) + def test_coverage_ignores_a_retired_test_left_at_not_run(self): + # A report that predates a test's retirement still carries the test, and if that run + # never reached it the entry is "not-run". Reporting that is holding a port to an + # obligation the contract has withdrawn -- the same report is tolerated as drift + # everywhere else, so the sweep would have stayed red until the port happened to rerun. + reports = self.stored_reports() + reports["android"]["tests"]["RetiredApiTest"] = { + "feature": "crypto", + "status": "not-run", + } + self.assertEqual([], port_status.coverage_problems(self.manifest, reports)) + def test_coverage_accepts_a_documented_skip(self): # The distinction the rule turns on: a port that genuinely cannot do something reports # "skip" from the suite itself, which is evidence rather than the absence of it -- but From f2d972d5143e457c0503fee1387d3b57e52581c4 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 23 Aug 2026 18:50:49 +0300 Subject: [PATCH 05/14] Require a new run behind changed findings, and count the card over the contract The provenance gate accepted a change to *any* one of the three provenance fields, which left open the easier version of the forgery it was written to stop: invent the result, type today's date into `generated_at`, and leave `commit` and `run_url` still naming the run that never produced it. validate() accepts such a timestamp, so nothing downstream would have objected either. Both `run_url` and `generated_at` must now name a genuinely different run, and the new value has to be non-empty -- deleting the field is a change too, and would otherwise have let the edit through with no run named at all. `commit` stays out of that requirement on purpose: a port legitimately re-runs the same master commit, and its second run is a different run. The message names which identity field is still pointing at the old one. The port card summed the report's own summary counts, which cover every entry the report carries rather than the tests the contract defines. That is wrong at both ends of the difference, and the previous commit only fixed one end. A report predating a newly registered test has no entry for it -- handled. A report predating a test's RETIREMENT still carries the test, so a retired failure kept the card red, and a retired not-run kept it partial, over a test that appears nowhere in the matrix below it. Now that validate() tolerates retired entries as drift and the sync keeps such a report, that state is reachable rather than hypothetical. The card is the headline for a column of cells, so it is computed that way now: one pass over the contract's tests, looking each up in the report, tallying the same four states the cells show plus awaiting. Output on the current data is byte-identical -- nothing is retired today -- and a retired failure injected into the JavaScript report no longer reddens its card. Co-Authored-By: Claude Opus 5 (1M context) --- .../partials/port-status-port-state.html | 35 ++++++++++++----- scripts/hellocodenameone/README.adoc | 10 +++-- .../conformance/port_status.py | 29 +++++++++++--- .../conformance/test_port_status.py | 38 +++++++++++++++++++ 4 files changed, 94 insertions(+), 18 deletions(-) diff --git a/docs/website/layouts/partials/port-status-port-state.html b/docs/website/layouts/partials/port-status-port-state.html index 4f3729be56a..710e38c85e9 100644 --- a/docs/website/layouts/partials/port-status-port-state.html +++ b/docs/website/layouts/partials/port-status-port-state.html @@ -5,17 +5,34 @@ {{- $state := "unknown" -}} {{- $label := "No stored report" -}} {{- $awaiting := 0 -}} +{{- $passed := 0 -}} +{{- $skipped := 0 -}} +{{- $failed := 0 -}} +{{- $notRun := 0 -}} {{- if $report -}} - {{- $failed := int (default 0 $report.summary.fail) -}} - {{- $notRun := int (default 0 (index $report.summary "not-run")) -}} - {{- /* A report counts the tests it carries. Between the commit that registers - a test and this port's next run the report legitimately has no entry - for it, and summing only what the report holds would present that as a - complete answer to a contract it has not seen. Say how many tests this - port has yet to reach instead. */ -}} + {{- /* Counted over the contract, not over the report's own summary. Those are + different sets in both directions, and the card was wrong at both ends + of the difference. A report predating a newly registered test carries no + entry for it, and summing only what the report holds presented that as a + complete answer to a contract it has not seen. A report that predates a + test's RETIREMENT still carries the test, and a retired failure kept the + card red over a test that appears nowhere in the matrix below it. The + card is the sum of the cells the table shows, so it is now literally + computed that way. */ -}} {{- range $contract.features -}} {{- range .tests -}} - {{- if not (index $report.tests .) -}}{{- $awaiting = add $awaiting 1 -}}{{- end -}} + {{- $result := index $report.tests . -}} + {{- if not $result -}} + {{- $awaiting = add $awaiting 1 -}} + {{- else if eq $result.status "pass" -}} + {{- $passed = add $passed 1 -}} + {{- else if eq $result.status "skip" -}} + {{- $skipped = add $skipped 1 -}} + {{- else if eq $result.status "fail" -}} + {{- $failed = add $failed 1 -}} + {{- else -}} + {{- $notRun = add $notRun 1 -}} + {{- end -}} {{- end -}} {{- end -}} {{- $bootstrapComplete := and (eq $report.bootstrap_source "successful-master-workflow") (eq $report.workflow_conclusion "success") -}} @@ -60,7 +77,7 @@

{{ $port.name }}

{{ $label }}

{{- if $report }}

- {{ $report.summary.pass }} passed · {{ $report.summary.skip }} skipped · {{ index $report.summary "not-run" }} not run{{ if gt $awaiting 0 }} · {{ $awaiting }} awaiting this port's next run{{ end }} + {{ $passed }} passed · {{ $skipped }} skipped · {{ $notRun }} not run{{ if gt $awaiting 0 }} · {{ $awaiting }} awaiting this port's next run{{ end }}

{{- end }}
diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index 089583ffcdb..7c4465aff78 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -138,10 +138,12 @@ editing eleven report files and retyping their totals -- which made every test-adding branch conflict with every other one, and made the cheapest way to a green build typing `pass` next to a test no port had run. Twelve results reached `master` that way, attributed to runs that predated the tests. `Validate -port status contract` now refuses a report whose findings changed while the -`generated_at`, `commit` and `run_url` naming the run stayed put -- every field -but those three, so the benchmark durations the page publishes as measurements -are covered along with the test map. +port status contract` now refuses a report whose findings changed without a new +run behind them: every field but `generated_at`, `commit` and `run_url` counts +as a finding, so the benchmark durations the page publishes as measurements are +covered along with the test map, and both `run_url` and `generated_at` have to +name a genuinely different run before the change is believed. (`commit` need not +move -- a port legitimately re-runs the same master commit.) "All tests run on all ports" is still enforced, just where it cannot be typed: the nightly sweep holds the *published* reports to it. A test left at `not-run` diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 32f91776ed9..a42e6de047d 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -522,6 +522,11 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: PROVENANCE_FIELDS = ("generated_at", "commit", "run_url") +# What has to be *new* before changed findings are believable. `commit` is not +# among them: a port legitimately re-runs the same master commit, and its second +# run is a different run. `run_url` is the run's identity and `generated_at` is +# when it reported; a real snapshot carries new values for both. +RUN_IDENTITY_FIELDS = ("generated_at", "run_url") def provenance_problems(port_id: str, before: dict, after: dict) -> list[str]: @@ -547,18 +552,32 @@ def provenance_problems(port_id: str, before: dict, after: dict) -> list[str]: ) if findings[0] == findings[1]: return [] - if any(before.get(field) != after.get(field) for field in PROVENANCE_FIELDS): + # Every identity field, not any provenance field. Accepting a change to one + # of the three left the gate open to the easier version of the same forgery: + # invent a result, type today's date into `generated_at`, and leave the + # `commit` and `run_url` still naming the run that never produced it. A + # snapshot that came from a run has a new run behind it. + if all( + before.get(field) != after.get(field) and after.get(field) + for field in RUN_IDENTITY_FIELDS + ): return [] changed = sorted( key for key in set(findings[0]) | set(findings[1]) if findings[0].get(key) != findings[1].get(key) ) + stale = sorted( + field + for field in RUN_IDENTITY_FIELDS + if not (before.get(field) != after.get(field) and after.get(field)) + ) return [ - f"{port_id}: {', '.join(changed)} changed but " - f"{', '.join(PROVENANCE_FIELDS)} did not. These reports are CI output -- " - "a branch never needs to edit one. Adding a test needs no report change " - "at all; each port picks it up on its next master run." + f"{port_id}: {', '.join(changed)} changed without a new run behind it -- " + f"{', '.join(stale)} still name{'s' if len(stale) == 1 else ''} the " + "previous one. These reports are CI output, and a branch never needs to " + "edit one. Adding a test needs no report change at all; each port picks " + "it up on its next master run." ] diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index 49251c41cb2..d9512569ba6 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -507,8 +507,46 @@ def test_provenance_accepts_a_genuinely_newer_snapshot(self): after["summary"]["pass"] += 1 after["generated_at"] = "2026-12-31T00:00:00Z" after["commit"] = "0123456789abcdef" + after["run_url"] = "https://example.invalid/run/9" self.assertEqual([], port_status.provenance_problems("android", before, after)) + def test_provenance_rejects_a_fresh_stamp_over_the_same_run(self): + # The easier version of the same forgery, and the one a gate that accepted any single + # provenance change would have invited: invent the result, type today's date, and leave + # commit and run_url still naming the run that never produced it. + before = self.stored_reports()["android"] + after = json.loads(json.dumps(before)) + after["tests"]["SomeBrandNewTest"] = {"feature": "crypto", "status": "pass"} + after["summary"]["pass"] += 1 + after["generated_at"] = "2026-12-31T00:00:00Z" + problems = port_status.provenance_problems("android", before, after) + self.assertEqual(1, len(problems), problems) + self.assertIn("run_url", problems[0]) + + def test_provenance_rejects_a_new_run_url_on_the_same_stamp(self): + # The mirror image. A run reports at a time; reusing the old one says this snapshot is + # the same measurement under a different name. + before = self.stored_reports()["android"] + after = json.loads(json.dumps(before)) + after["performance"]["benchmarks"]["quicksort"]["duration_ns"] = 1 + after["run_url"] = "https://example.invalid/run/9" + problems = port_status.provenance_problems("android", before, after) + self.assertEqual(1, len(problems), problems) + self.assertIn("generated_at", problems[0]) + + def test_provenance_rejects_an_emptied_run_url(self): + # "Different" is not enough on its own: deleting the field would otherwise read as a + # change and let the edit through with no run named at all. + before = self.stored_reports()["android"] + after = json.loads(json.dumps(before)) + after["tests"]["SomeBrandNewTest"] = {"feature": "crypto", "status": "pass"} + after["summary"]["pass"] += 1 + after["generated_at"] = "2026-12-31T00:00:00Z" + after["run_url"] = "" + problems = port_status.provenance_problems("android", before, after) + self.assertEqual(1, len(problems), problems) + self.assertIn("run_url", problems[0]) + def test_provenance_rejects_edited_benchmark_measurements(self): # The findings nobody can check by reading them. A benchmark duration is published as a # measurement of a named run; rewriting one in place attributes an invented number to From 98b012004fb90035f6f5be2681e7a0f8e57506f1 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 23 Aug 2026 19:36:42 +0300 Subject: [PATCH 06/14] Keep asserting which capability each load-bearing test proves Removing the literal totals took the feature-mapping assertions with them, and those are a different thing. validate() only requires each registered test to sit under exactly one feature -- any feature satisfies it -- so with these gone a future edit could file CameraApiTest under an unrelated row and the whole suite would still pass while the page published a camera claim the test never made. The same goes for the calendar, on-device vision/language/inference, AR and VideoIO rows, each of which exists to say something specific about a capability. They are back, in their own test rather than appended to the counts one, with the reason they are kept written down: this is a spot check of the mappings where landing a result under the wrong row misstates a capability, not a registry every new feature has to join. What made the old block painful was the totals beside it, which every branch had to retype; nothing obliges a branch to touch this list. Verified by moving CameraApiTest under an unrelated feature: the assertion fails. Co-Authored-By: Claude Opus 5 (1M context) --- .../conformance/test_port_status.py | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index d9512569ba6..28993a761bd 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -21,7 +21,9 @@ def test_contract_covers_registered_tests_and_goldens(self): # adding a test conflicted here by construction -- on a line whose only # content was a number neither author had a reason to think about. What # is worth asserting is the relationship: the suite and the contract - # describe the same set of tests, and nothing is counted twice. + # describe the same set of tests, and nothing is counted twice. Which + # test belongs to which capability is a separate question, and a real + # one; it is asserted below rather than dropped. counts = port_status.validate(self.manifest) registered = port_status.registered_tests() mapped = port_status.test_to_feature(self.manifest) @@ -45,6 +47,22 @@ def test_contract_covers_registered_tests_and_goldens(self): self.assertEqual(8, counts["deployment_platforms"]) self.assertEqual(3, counts["browser_engines"]) + def test_load_bearing_tests_stay_under_the_capability_they_prove(self): + # A spot check, not a registry: these are the mappings where landing a result under the + # wrong row would publish a specific capability claim the test never made. Nothing above + # catches that -- validate() only requires each test to sit under exactly one feature, + # and any feature satisfies it. Adding a feature does not oblige anyone to extend this + # list; it is the literal totals that every branch had to retype, not these. + features = {feature["id"]: feature["tests"] for feature in self.manifest["features"]} + self.assertEqual(["ARApiTest", "MotionSensorDeviceTest"], features["ar-motion-sensors"]) + self.assertEqual(["CameraApiTest"], features["camera-access"]) + self.assertEqual(["VisionOnDeviceApiTest"], features["on-device-vision"]) + self.assertEqual(["LanguageOnDeviceApiTest"], features["on-device-language"]) + self.assertEqual(["InferenceOnDeviceApiTest"], features["on-device-inference"]) + self.assertEqual(["CalendarApiTest"], features["calendar-integration"]) + self.assertEqual(["VideoIODecodedFramesScreenshotTest"], features["video-decoding"]) + self.assertEqual(["VideoIORoundTripTest"], features["video-round-trip"]) + def test_normalize_preserves_pass_skip_and_screenshot_failure(self): log_text = "\n".join( [ From 1178c501a9542119f7722df488d7cb30b570aa06 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 23 Aug 2026 21:19:39 +0300 Subject: [PATCH 07/14] Ask the data branch whether the run behind a report ever published it Requiring both identity fields to change still only compared strings, so the gate was bypassable by inventing two of them instead of one -- a plausible timestamp and a plausible run URL alongside the fabricated result. Nothing so far established that the run existed, let alone that it produced this report. It can be established, because a checked-in report is not authored: it is a copy of what CI put on the port-status-data branch. So the check now asks that branch. A changed report has to be a version the branch actually holds, which a refresh satisfies by construction and an invented report cannot be made to satisfy without the write access to that branch that only the publish workflows have. The identity strings stop being two fields a branch can fill in and become properties of a report a run really produced. About a week of that branch's history is read, not just its tip: a refresh taken before the port ran again matches an ancestor, and rejecting it for not equalling today's tip would be a false accusation. The fetch is depth-limited and takes under a second; the whole check runs in about four over eleven ports. If the branch cannot be reached the corroboration is dropped with a note rather than failing the branch -- unverifiable is not forged, and a gate that fails on a flaky fetch teaches people to route around it. A run URL is also required to look like one, which costs nothing and rejects the field filled in with a placeholder. That is shape only and proves nothing on its own; the data branch is what proves something. Verified end to end against origin/master: the eleven refreshed reports corroborate (as ancestors -- the branch has moved since), a fabricated result with run_url "made-up-new-run" is refused on shape, the same fabrication with a well-formed but never-published URL is refused as never published, and copying a genuine published report in is accepted. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/hellocodenameone/README.adoc | 15 +++-- .../check_port_status_provenance.sh | 51 ++++++++++++++-- .../conformance/port_status.py | 60 ++++++++++++++++++- .../conformance/test_port_status.py | 48 +++++++++++++-- 4 files changed, 159 insertions(+), 15 deletions(-) diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index 7c4465aff78..0705e74ffc9 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -139,11 +139,18 @@ test-adding branch conflict with every other one, and made the cheapest way to a green build typing `pass` next to a test no port had run. Twelve results reached `master` that way, attributed to runs that predated the tests. `Validate port status contract` now refuses a report whose findings changed without a new -run behind them: every field but `generated_at`, `commit` and `run_url` counts +run behind them. Every field but `generated_at`, `commit` and `run_url` counts as a finding, so the benchmark durations the page publishes as measurements are -covered along with the test map, and both `run_url` and `generated_at` have to -name a genuinely different run before the change is believed. (`commit` need not -move -- a port legitimately re-runs the same master commit.) +covered along with the test map. Both `run_url` and `generated_at` have to name +a genuinely different run (`commit` need not move -- a port legitimately re-runs +the same master commit), the run URL has to look like one, and the report itself +has to be a version the `port-status-data` branch actually holds. That last +check is the one that matters: a refreshed snapshot is a copy of what CI +published, so it matches by construction, while an invented one cannot be made +to match without the write access to that branch which only the publish +workflows have. The check reads about a week of the branch's history, so a +refresh taken before the port ran again still matches; if the branch cannot be +fetched it is skipped rather than failed, because unverifiable is not forged. "All tests run on all ports" is still enforced, just where it cannot be typed: the nightly sweep holds the *published* reports to it. A test left at `not-run` diff --git a/scripts/hellocodenameone/conformance/check_port_status_provenance.sh b/scripts/hellocodenameone/conformance/check_port_status_provenance.sh index 47259ff6b3e..55fa19b5291 100755 --- a/scripts/hellocodenameone/conformance/check_port_status_provenance.sh +++ b/scripts/hellocodenameone/conformance/check_port_status_provenance.sh @@ -37,6 +37,11 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" REPO_ROOT="$(cd "${SCRIPT_DIR}/../../.." && pwd)" MANIFEST="${REPO_ROOT}/docs/website/data/port_status.json" REPORT_PATH="docs/website/data/port_status_reports" +DATA_REF="refs/heads/port-status-data" +# Roughly a week of publications across all eleven ports, fetched in under a +# second. A snapshot older than that should be refreshed before review anyway, +# and the failure mode if it is not says exactly that. +DATA_DEPTH=200 base_ref="${1:-}" if [ -z "${base_ref}" ]; then @@ -55,10 +60,48 @@ fi tmp_dir="$(mktemp -d)" trap 'rm -rf "${tmp_dir}"' EXIT +base_dir="${tmp_dir}/base" +published_dir="${tmp_dir}/published" +mkdir -p "${base_dir}" "${published_dir}" + +# What the data branch has actually published turns this from "are these two +# strings different" into "did a run produce this report". Unreachable is not +# the same as forged, so a fetch failure drops the corroboration rather than +# failing the branch -- the inequality checks still apply either way. +have_published=1 +if ! git -C "${REPO_ROOT}" fetch --quiet --no-tags --depth="${DATA_DEPTH}" origin "${DATA_REF}"; then + echo "Port Status data branch is unavailable; checking provenance fields only." >&2 + have_published=0 +fi + while IFS= read -r port; do - git -C "${REPO_ROOT}" show "${base_ref}:${REPORT_PATH}/${port}.json" \ - > "${tmp_dir}/${port}.json" 2>/dev/null \ - || rm -f "${tmp_dir}/${port}.json" + if ! git -C "${REPO_ROOT}" show "${base_ref}:${REPORT_PATH}/${port}.json" \ + > "${base_dir}/${port}.json" 2>/dev/null; then + # New port, or a base revision from before this file existed. Nothing to + # compare against, so there is no edit to object to. + rm -f "${base_dir}/${port}.json" + continue + fi + if cmp -s "${base_dir}/${port}.json" "${REPO_ROOT}/${REPORT_PATH}/${port}.json"; then + continue + fi + [ "${have_published}" -eq 1 ] || continue + # Only for a report that changed: every version the branch has held, so a + # refresh taken before the port ran again still matches an ancestor rather + # than being rejected for not equalling today's tip. + mkdir -p "${published_dir}/${port}" + index=0 + while IFS= read -r revision; do + git -C "${REPO_ROOT}" show "${revision}:ports/${port}.json" \ + > "${published_dir}/${port}/${index}.json" 2>/dev/null \ + || rm -f "${published_dir}/${port}/${index}.json" + index=$((index + 1)) + done < <(git -C "${REPO_ROOT}" log --format=%H FETCH_HEAD -- "ports/${port}.json") done < <(jq -r '.ports[].id' "${MANIFEST}") -python3 "${SCRIPT_DIR}/port_status.py" provenance --base "${tmp_dir}" +if [ "${have_published}" -eq 1 ]; then + python3 "${SCRIPT_DIR}/port_status.py" provenance \ + --base "${base_dir}" --published "${published_dir}" +else + python3 "${SCRIPT_DIR}/port_status.py" provenance --base "${base_dir}" +fi diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index a42e6de047d..24e49afef25 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -527,9 +527,20 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: # run is a different run. `run_url` is the run's identity and `generated_at` is # when it reported; a real snapshot carries new values for both. RUN_IDENTITY_FIELDS = ("generated_at", "run_url") +# A run URL names a run on this forge. Shape alone proves nothing about whether +# the run happened -- the data branch below is what establishes that -- but it +# costs nothing and rejects a field filled in with a placeholder. +RUN_URL_RE = re.compile( + r"^https://github\.com/[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+/actions/runs/\d+(?:/[A-Za-z0-9_/-]*)?$" +) -def provenance_problems(port_id: str, before: dict, after: dict) -> list[str]: +def provenance_problems( + port_id: str, + before: dict, + after: dict, + published: list[dict] | None = None, +) -> list[str]: """Refuse a hand-edited report. A report says "at this commit, this run, at this time, this is what the port @@ -545,6 +556,17 @@ def provenance_problems(port_id: str, before: dict, after: dict) -> list[str]: say the suite completed. Naming a subset here would leave the numbers most worth doubting -- the ones nobody can check by reading them -- as the one thing a branch could still rewrite in place. + + ``published`` is what turns this from a shape check into a verification. It + is every version of this port's report the ``port-status-data`` branch has + held recently, and a changed snapshot has to *be* one of them -- which it + will be, because the only way to refresh one is to copy what CI published. + A new ``run_url`` and stamp are then not two strings a branch can invent; + they have to belong to a report that a run really produced, and producing one + needs the write access to the data branch that only the publish workflows + have. Pass None when the branch could not be reached: unverifiable is not the + same as forged, and failing a pull request because a fetch flaked would teach + people to route around this. """ findings = tuple( {key: value for key, value in report.items() if key not in PROVENANCE_FIELDS} @@ -561,7 +583,21 @@ def provenance_problems(port_id: str, before: dict, after: dict) -> list[str]: before.get(field) != after.get(field) and after.get(field) for field in RUN_IDENTITY_FIELDS ): - return [] + run_url = after.get("run_url") + if not isinstance(run_url, str) or not RUN_URL_RE.match(run_url): + return [ + f"{port_id}: run_url {run_url!r} does not name a workflow run. " + "Refresh the report from the port-status-data branch instead of " + "editing it." + ] + if published is None or any(candidate == after for candidate in published): + return [] + return [ + f"{port_id}: this report was never published by the run it names. A " + "checked-in report is a copy of what CI put on the port-status-data " + "branch, so refresh it from there rather than editing it; adding a " + "test needs no report change at all." + ] changed = sorted( key for key in set(findings[0]) | set(findings[1]) @@ -1115,6 +1151,14 @@ def build_parser() -> argparse.ArgumentParser: type=Path, help="directory holding the base revision's reports", ) + provenance_parser.add_argument( + "--published", + type=Path, + help=( + "directory of /*.json holding every version of each report the " + "data branch has held recently; omit when it could not be fetched" + ), + ) normalize_parser = subparsers.add_parser("normalize", help="write a normalized port report") normalize_parser.add_argument("--port", required=True) @@ -1180,9 +1224,19 @@ def main() -> int: if not base_path.is_file(): continue head_path = REPO_ROOT / report_directory / f"{port_id}.json" + published = None + if args.published is not None: + candidates = args.published / port_id + published = [ + read_json(item) + for item in sorted(candidates.glob("*.json")) + ] if candidates.is_dir() else [] problems.extend( provenance_problems( - port_id, read_json(base_path), read_json(head_path) + port_id, + read_json(base_path), + read_json(head_path), + published, ) ) for problem in problems: diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index 28993a761bd..a52e2f3fb18 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -518,15 +518,55 @@ def test_provenance_rejects_results_edited_without_a_new_run(self): self.assertEqual(1, len(problems), problems) self.assertIn("CI output", problems[0]) - def test_provenance_accepts_a_genuinely_newer_snapshot(self): - before = self.stored_reports()["android"] + def newer_snapshot(self, before): after = json.loads(json.dumps(before)) after["tests"]["SomeBrandNewTest"] = {"feature": "crypto", "status": "pass"} after["summary"]["pass"] += 1 after["generated_at"] = "2026-12-31T00:00:00Z" after["commit"] = "0123456789abcdef" - after["run_url"] = "https://example.invalid/run/9" - self.assertEqual([], port_status.provenance_problems("android", before, after)) + after["run_url"] = "https://github.com/codenameone/CodenameOne/actions/runs/99" + return after + + def test_provenance_accepts_a_genuinely_newer_snapshot(self): + # Without the data branch to consult. Unverifiable is not the same as forged, and + # failing a branch because a fetch flaked would teach people to route around this. + before = self.stored_reports()["android"] + self.assertEqual( + [], port_status.provenance_problems("android", before, self.newer_snapshot(before)) + ) + + def test_provenance_accepts_a_report_the_data_branch_published(self): + before = self.stored_reports()["android"] + after = self.newer_snapshot(before) + self.assertEqual( + [], + port_status.provenance_problems( + "android", before, after, published=[before, after] + ), + ) + + def test_provenance_rejects_a_report_no_run_ever_published(self): + # The bypass that survived requiring both identity fields: type a plausible run URL and + # a plausible date. A checked-in report is a copy of what CI put on the data branch, so + # the branch is asked whether this report was ever there. Producing one that was needs + # the write access to that branch which only the publish workflows have. + before = self.stored_reports()["android"] + after = self.newer_snapshot(before) + problems = port_status.provenance_problems( + "android", before, after, published=[before] + ) + self.assertEqual(1, len(problems), problems) + self.assertIn("never published", problems[0]) + + def test_provenance_rejects_a_run_url_that_names_no_run(self): + before = self.stored_reports()["android"] + after = self.newer_snapshot(before) + after["run_url"] = "made-up-new-run" + problems = port_status.provenance_problems( + "android", before, after, published=[after] + ) + self.assertEqual(1, len(problems), problems) + self.assertIn("does not name a workflow run", problems[0]) def test_provenance_rejects_a_fresh_stamp_over_the_same_run(self): # The easier version of the same forgery, and the one a gate that accepted any single From 5bb67b1ecec1e372ff9372cb283c6f435e3eb350 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Sun, 23 Aug 2026 21:56:02 +0300 Subject: [PATCH 08/14] Corroborate provenance-only edits and reports a branch adds Two ways past the data-branch check, both of them the same shape: a report that never had to prove anything. A change that touches only `generated_at` alters no finding, so every rule about findings returned "untouched" and the corroboration was never reached. That is the edit with the worst consequence of any of them: the page reads generated_at to decide whether a column is stale, so retyping it is how a port that has stopped reporting altogether would go on looking like it was still running -- and validate() accepts any plausible past timestamp. Corroboration is now asked of any change at all, not only of a changed result. A report the branch ADDS had no base version, and the driver skipped it. That is the one file with nothing earlier to check against, so it was the only one nobody was checking: a pull request adding a port could give it an entirely hand-authored green snapshot, which is precisely the forgery the rest of this gate exists to stop. An added report now has to be a version the data branch published, like every other. For that to be actionable rather than a dead end, a port with no stored report had to become a supported state -- otherwise adding a port would begin by authoring the snapshot this rule refuses. validate() now reports a missing report as drift, and the page already renders it correctly without one: every cell reads "No stored report" and the card reads unknown, which is what is true of a port CI has never heard from. Verified by removing tvos.json and building the site. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/hellocodenameone/README.adoc | 14 ++- .../check_port_status_provenance.sh | 19 +-- .../conformance/port_status.py | 111 ++++++++++++------ .../conformance/test_port_status.py | 67 ++++++++++- 4 files changed, 160 insertions(+), 51 deletions(-) diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index 0705e74ffc9..91eb873ef5d 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -148,9 +148,17 @@ has to be a version the `port-status-data` branch actually holds. That last check is the one that matters: a refreshed snapshot is a copy of what CI published, so it matches by construction, while an invented one cannot be made to match without the write access to that branch which only the publish -workflows have. The check reads about a week of the branch's history, so a -refresh taken before the port ran again still matches; if the branch cannot be -fetched it is skipped rather than failed, because unverifiable is not forged. +workflows have. It is asked of any change, including one that touches only +`generated_at` -- that edit changes no finding, and it is how a port that had +stopped reporting would go on looking current -- and of a report the branch +*adds*, which is otherwise the one file with no earlier version to check against. +The check reads about a week of the branch's history, so a refresh taken before +the port ran again still matches; if the branch cannot be fetched it is skipped +rather than failed, because unverifiable is not forged. + +A port with no stored report is a supported state, so adding a port does not +start by hand-authoring one: every cell reads "No stored report" and the card +reads "unknown" until that port's first run publishes something to copy. "All tests run on all ports" is still enforced, just where it cannot be typed: the nightly sweep holds the *published* reports to it. A test left at `not-run` diff --git a/scripts/hellocodenameone/conformance/check_port_status_provenance.sh b/scripts/hellocodenameone/conformance/check_port_status_provenance.sh index 55fa19b5291..b37dd0dcb87 100755 --- a/scripts/hellocodenameone/conformance/check_port_status_provenance.sh +++ b/scripts/hellocodenameone/conformance/check_port_status_provenance.sh @@ -75,15 +75,20 @@ if ! git -C "${REPO_ROOT}" fetch --quiet --no-tags --depth="${DATA_DEPTH}" origi fi while IFS= read -r port; do - if ! git -C "${REPO_ROOT}" show "${base_ref}:${REPORT_PATH}/${port}.json" \ + head_report="${REPO_ROOT}/${REPORT_PATH}/${port}.json" + [ -f "${head_report}" ] || continue + if git -C "${REPO_ROOT}" show "${base_ref}:${REPORT_PATH}/${port}.json" \ > "${base_dir}/${port}.json" 2>/dev/null; then - # New port, or a base revision from before this file existed. Nothing to - # compare against, so there is no edit to object to. + if cmp -s "${base_dir}/${port}.json" "${head_report}"; then + continue + fi + else + # A report the branch ADDS. Skipping it here is how a pull request that also + # adds a port could hand-author an entirely green snapshot for it -- the one + # report with no earlier version to be checked against, and so the only one + # nobody was checking at all. It still has to be a report the data branch + # published; a port that has never published simply does not need one. rm -f "${base_dir}/${port}.json" - continue - fi - if cmp -s "${base_dir}/${port}.json" "${REPO_ROOT}/${REPORT_PATH}/${port}.json"; then - continue fi [ "${have_published}" -eq 1 ] || continue # Only for a report that changed: every version the branch has held, so a diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 24e49afef25..60f51efa979 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -369,8 +369,17 @@ def stored_report_problems(manifest: dict) -> tuple[set[str], list[str], list[st port_id = port.get("id") if not port_id: continue + path = root / f"{port_id}.json" + if not path.is_file(): + # A port with no published report renders as "No stored report" on + # every one of its cells, which is what is true of a port CI has + # never heard from. Demanding a file here is what made adding a port + # start by hand-authoring one, and a hand-authored report is the + # thing this whole contract is trying to stop existing. + drift.append(f"{port_id}: no stored report yet") + continue try: - report = read_json(root / f"{port_id}.json") + report = read_json(path) except ContractError as exc: malformed.append(str(exc)) continue @@ -537,7 +546,7 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: def provenance_problems( port_id: str, - before: dict, + before: dict | None, after: dict, published: list[dict] | None = None, ) -> list[str]: @@ -568,52 +577,74 @@ def provenance_problems( same as forged, and failing a pull request because a fetch flaked would teach people to route around this. """ - findings = tuple( - {key: value for key, value in report.items() if key not in PROVENANCE_FIELDS} - for report in (before, after) - ) - if findings[0] == findings[1]: + if before == after: return [] + + advice = ( + "A checked-in report is a copy of what CI put on the port-status-data " + "branch, so refresh it from there rather than editing it; adding a test " + "needs no report change at all." + ) + + # Checked on any change to the field, not only alongside changed findings. + if before is None or before.get("run_url") != after.get("run_url"): + run_url = after.get("run_url") + if not isinstance(run_url, str) or not RUN_URL_RE.match(run_url): + return [ + f"{port_id}: run_url {run_url!r} does not name a workflow run. " + + advice + ] + # Every identity field, not any provenance field. Accepting a change to one # of the three left the gate open to the easier version of the same forgery: # invent a result, type today's date into `generated_at`, and leave the # `commit` and `run_url` still naming the run that never produced it. A # snapshot that came from a run has a new run behind it. - if all( + findings = tuple( + {key: value for key, value in (report or {}).items() if key not in PROVENANCE_FIELDS} + for report in (before, after) + ) + if before is not None and findings[0] != findings[1] and not all( before.get(field) != after.get(field) and after.get(field) for field in RUN_IDENTITY_FIELDS ): - run_url = after.get("run_url") - if not isinstance(run_url, str) or not RUN_URL_RE.match(run_url): - return [ - f"{port_id}: run_url {run_url!r} does not name a workflow run. " - "Refresh the report from the port-status-data branch instead of " - "editing it." - ] - if published is None or any(candidate == after for candidate in published): - return [] + changed = sorted( + key + for key in set(findings[0]) | set(findings[1]) + if findings[0].get(key) != findings[1].get(key) + ) + stale = sorted( + field + for field in RUN_IDENTITY_FIELDS + if not (before.get(field) != after.get(field) and after.get(field)) + ) return [ - f"{port_id}: this report was never published by the run it names. A " - "checked-in report is a copy of what CI put on the port-status-data " - "branch, so refresh it from there rather than editing it; adding a " - "test needs no report change at all." + f"{port_id}: {', '.join(changed)} changed without a new run behind it -- " + f"{', '.join(stale)} still name{'s' if len(stale) == 1 else ''} the " + "previous one. These reports are CI output, and a branch never needs to " + "edit one. Adding a test needs no report change at all; each port picks " + "it up on its next master run." + ] + + # Asked of *any* change, including one that touches only the provenance + # fields. Retyping generated_at alone changes no finding, and the page reads + # that field to decide whether a column is stale -- so the edit nothing else + # objected to was the one that made a port which had stopped reporting look + # like it was still running. + if published is None: + return [] + if any(candidate == after for candidate in published): + return [] + if not published: + return [ + f"{port_id}: this port has never published a report, so there is " + "nothing for a checked-in one to be a copy of. Leave it out -- every " + "cell reads 'No stored report' until the port's first run, which is " + "what is true. " + advice ] - changed = sorted( - key - for key in set(findings[0]) | set(findings[1]) - if findings[0].get(key) != findings[1].get(key) - ) - stale = sorted( - field - for field in RUN_IDENTITY_FIELDS - if not (before.get(field) != after.get(field) and after.get(field)) - ) return [ - f"{port_id}: {', '.join(changed)} changed without a new run behind it -- " - f"{', '.join(stale)} still name{'s' if len(stale) == 1 else ''} the " - "previous one. These reports are CI output, and a branch never needs to " - "edit one. Adding a test needs no report change at all; each port picks " - "it up on its next master run." + f"{port_id}: this report is not a version the port-status-data branch " + "ever held. " + advice ] @@ -1221,9 +1252,11 @@ def main() -> int: for port in manifest.get("ports", []): port_id = port.get("id") base_path = args.base / f"{port_id}.json" - if not base_path.is_file(): - continue head_path = REPO_ROOT / report_directory / f"{port_id}.json" + if not head_path.is_file(): + # Removed, or a port that has never published. Neither is an + # edit to a result. + continue published = None if args.published is not None: candidates = args.published / port_id @@ -1234,7 +1267,7 @@ def main() -> int: problems.extend( provenance_problems( port_id, - read_json(base_path), + read_json(base_path) if base_path.is_file() else None, read_json(head_path), published, ) diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index a52e2f3fb18..4387c37ac77 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -556,7 +556,7 @@ def test_provenance_rejects_a_report_no_run_ever_published(self): "android", before, after, published=[before] ) self.assertEqual(1, len(problems), problems) - self.assertIn("never published", problems[0]) + self.assertIn("ever held", problems[0]) def test_provenance_rejects_a_run_url_that_names_no_run(self): before = self.stored_reports()["android"] @@ -587,7 +587,7 @@ def test_provenance_rejects_a_new_run_url_on_the_same_stamp(self): before = self.stored_reports()["android"] after = json.loads(json.dumps(before)) after["performance"]["benchmarks"]["quicksort"]["duration_ns"] = 1 - after["run_url"] = "https://example.invalid/run/9" + after["run_url"] = "https://github.com/codenameone/CodenameOne/actions/runs/98" problems = port_status.provenance_problems("android", before, after) self.assertEqual(1, len(problems), problems) self.assertIn("generated_at", problems[0]) @@ -628,6 +628,69 @@ def test_provenance_rejects_an_edited_completion_marker(self): self.assertEqual(1, len(problems), problems) self.assertIn("suite_finished", problems[0]) + def test_provenance_rejects_a_timestamp_retyped_onto_an_old_snapshot(self): + # Changes no finding, so every rule about findings passes it -- and it is the edit with + # the worst consequence of any of them. The page reads generated_at to decide whether a + # column is stale, so retyping it is how a port that has stopped reporting altogether + # would go on looking like it was still running. Corroboration is therefore asked of any + # change, not only of a changed result. + before = self.stored_reports()["android"] + after = json.loads(json.dumps(before)) + after["generated_at"] = "2026-12-31T00:00:00Z" + problems = port_status.provenance_problems( + "android", before, after, published=[before] + ) + self.assertEqual(1, len(problems), problems) + self.assertIn("ever held", problems[0]) + + def test_provenance_rejects_a_hand_authored_report_for_a_new_port(self): + # The one report with no earlier version to be checked against, and so the only one + # nobody was checking at all: a pull request that adds a port could give it an entirely + # green snapshot. It has to be a report the data branch published, like every other. + published = self.stored_reports()["android"] + invented = json.loads(json.dumps(published)) + invented["port"] = "freebsd" + problems = port_status.provenance_problems( + "freebsd", None, invented, published=[published] + ) + self.assertEqual(1, len(problems), problems) + self.assertIn("ever held", problems[0]) + + def test_provenance_tells_a_new_port_it_needs_no_report(self): + # And the advice has to be actionable, which it is only because a port with no stored + # report is now a supported state: every cell reads "No stored report" until its first + # run. Otherwise the only way to add a port would be to hand-author the snapshot this + # rule refuses. + invented = self.stored_reports()["android"] + problems = port_status.provenance_problems("freebsd", None, invented, published=[]) + self.assertEqual(1, len(problems), problems) + self.assertIn("never published a report", problems[0]) + + def test_provenance_accepts_a_new_port_report_copied_from_the_branch(self): + published = self.stored_reports()["android"] + self.assertEqual( + [], port_status.provenance_problems("android", None, published, published=[published]) + ) + + def test_validate_accepts_a_port_with_no_stored_report(self): + original_directory = self.manifest["report_directory"] + with tempfile.TemporaryDirectory(dir=port_status.REPO_ROOT) as tmp: + report_root = Path(tmp) + for port in self.manifest["ports"]: + if port["id"] == "tvos": + continue + source = port_status.REPO_ROOT / original_directory / (port["id"] + ".json") + (report_root / source.name).write_text( + source.read_text(encoding="utf-8"), encoding="utf-8" + ) + manifest = dict(self.manifest) + manifest["report_directory"] = str(report_root.relative_to(port_status.REPO_ROOT)) + counts = port_status.validate(manifest) + self.assertTrue( + any("tvos" in item and "no stored report" in item for item in counts["drift"]), + counts["drift"], + ) + def test_provenance_ignores_an_untouched_report(self): before = self.stored_reports()["android"] self.assertEqual([], port_status.provenance_problems("android", before, dict(before))) From e0928662b766398553f07ea22733a67b8090298e Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 24 Aug 2026 04:27:26 +0300 Subject: [PATCH 09/14] Fix the iOS build break and the Android paint crash, and keep an established fallback Three things, two of them master's own red CI that this branch inherits through the merge commit. iOS, all four failing jobs: IOSNative.m calls six com_codename1_impl_ios_IOSWearableCallbacks_* functions without including the generated header that declares them. clang 17 rejects an implicit declaration outright, so the phone, Metal and watch targets all failed to compile -- master has been red on scripts-ios.yml and scripts-ios-native.yml since #5583 landed. CN1SmartHome.m carries the identical include for IOSHomeCallbacks, the class this one was modelled on; this one was simply missing. The mangled names themselves are correct, checked against the six Java signatures. Android, the instrumentation NPE: dispatchDraw read renderingOperations.size() and then copied the list, both without the lock the same method takes twenty lines later to clear it -- while flushGraphics swapped the list out from under it on the EDT, also unlocked. ArrayList.addAll copies through toArray(), and a concurrent mutation there returns an array sized for the new contents and padded with NULLS. Those nulls arrived as AsyncOps and threw out of executeWithClip: a hard crash on the UI thread, which is what launchMainActivityAndWaitForDeviceRunner hit. flushGraphics has carried an "if (o != null)" guard against this same corruption since a user reported it, so the nulls were known; nothing had established where they came from. The snapshot and the swap are now both taken under RENDERING_OPERATIONS_LOCK. Only the copy -- the ops still execute outside it, because that is the frame's actual drawing and holding a lock there would park the EDT for the whole paint. The one remaining unguarded read of the field, the flushGraphics wait loop, goes through a small accessor rather than being left as the exception that teaches the next reader the field is free to touch. No null guard added in dispatchDraw: with the copy synchronized the nulls cannot occur, and a guard there would only hide it if they ever did again. Third, from review: deleting the checked-in report for a port that has one is now refused. Making a missing report a supported state was necessary so that adding a port need not begin by hand-authoring a snapshot -- but absent because it never existed and absent because someone removed it are different things, and only the first is harmless. The site serves that file precisely when the data branch is unreachable, so removing one turns an established column unknown at the moment the live data is missing. Retiring a port still works: drop it from the manifest and the check stops looking at it. Verified: the android module compiles against the freshly installed core and the new locking is in the bytecode (two monitorenters in dispatchDraw); the six iOS mangled names match their Java signatures; 66 normalizer tests pass; deleting tvos.json is refused and restoring it passes; validate, coverage, provenance, Hugo and validate_port_status.mjs are all green. Co-Authored-By: Claude Opus 5 (1M context) --- .../impl/android/AndroidAsyncView.java | 55 +++++++++++++++---- Ports/iOSPort/nativeSources/IOSNative.m | 7 +++ scripts/hellocodenameone/README.adoc | 7 ++- .../check_port_status_provenance.sh | 7 ++- .../conformance/port_status.py | 27 +++++++-- .../conformance/test_port_status.py | 16 ++++++ 6 files changed, 99 insertions(+), 20 deletions(-) diff --git a/Ports/Android/src/com/codename1/impl/android/AndroidAsyncView.java b/Ports/Android/src/com/codename1/impl/android/AndroidAsyncView.java index be8eca6ff01..69ab44f5c1c 100644 --- a/Ports/Android/src/com/codename1/impl/android/AndroidAsyncView.java +++ b/Ports/Android/src/com/codename1/impl/android/AndroidAsyncView.java @@ -218,15 +218,31 @@ protected void dispatchDraw(Canvas c) { //final HashMap slowest = new HashMap<>(); //final HashMap counts = new HashMap<>(); - int count = renderingOperations.size(); + // Snapshot under the lock this method already takes to clear the queue. + // Reading size() and then copying without it let flushGraphics swap the + // list in between, and ArrayList.addAll copies through toArray(): a + // concurrent mutation there returns an array sized for the new contents + // and padded with NULLS. Those nulls arrived here as AsyncOps and threw + // out of executeWithClip below, which is a hard crash on the UI thread. + // flushGraphics has carried an "if (o != null)" guard against the same + // corruption since a user reported it; this is the other end of it, and + // the reason the nulls exist at all. + // + // Only the copy is synchronized. The ops are executed below without the + // lock because that is the frame's actual drawing, and holding it there + // would park the EDT in flushGraphics for the whole paint. + int count; + synchronized (RENDERING_OPERATIONS_LOCK) { + count = renderingOperations.size(); - // this works around the case of a blank screen when an invalidate occurs out of nowhere - // and no operations are in the queue - if(count > 0) { - currentlyRendering.clear(); - currentlyRendering.addAll(renderingOperations); - } else { - count = currentlyRendering.size(); + // this works around the case of a blank screen when an invalidate occurs out of nowhere + // and no operations are in the queue + if(count > 0) { + currentlyRendering.clear(); + currentlyRendering.addAll(renderingOperations); + } else { + count = currentlyRendering.size(); + } } int offset = 0; for(; offset < count ; offset++) { @@ -364,13 +380,23 @@ public void run() { // When this reaches 10, the rendering operations are flushed. private int timeoutCounter=0; + // Not the crash, but the same field: dispatchDraw clears renderingOperations + // holding the lock, so reading it without one has no happens-before against + // that clear and can spin on a stale size. Leaving one access of a + // lock-guarded field unguarded is how the next one gets written that way. + private boolean renderingOperationsPending() { + synchronized (RENDERING_OPERATIONS_LOCK) { + return !renderingOperations.isEmpty(); + } + } + @Override public void flushGraphics(Rect rect) { //Log.d(Display.getInstance().getProperty("AppName", "CodenameOne"), "Flush graphics invoked with pending: " + pendingRenderingOperations.size() + " and current " + renderingOperations.size()); // we might have pending entries in the rendering queue int counter = 0; - while (!renderingOperations.isEmpty()) { + while (renderingOperationsPending()) { try { synchronized (RENDERING_OPERATIONS_LOCK) { RENDERING_OPERATIONS_LOCK.wait(5); @@ -404,9 +430,14 @@ public void run() { } } timeoutCounter = 0; - ArrayList tmp = renderingOperations; - renderingOperations = pendingRenderingOperations; - pendingRenderingOperations = tmp; + // The swap the snapshot in dispatchDraw races with. Unsynchronized, it + // could replace the list mid-copy, which is how the copy came back + // holding nulls. + synchronized (RENDERING_OPERATIONS_LOCK) { + ArrayList tmp = renderingOperations; + renderingOperations = pendingRenderingOperations; + pendingRenderingOperations = tmp; + } try { for (AsyncOp o : renderingOperations) { // can happen due to synchronization issues see: https://www.reddit.com/r/cn1/comments/1oo43in/error_while_using_app/ diff --git a/Ports/iOSPort/nativeSources/IOSNative.m b/Ports/iOSPort/nativeSources/IOSNative.m index c919fec5328..70ec878e343 100644 --- a/Ports/iOSPort/nativeSources/IOSNative.m +++ b/Ports/iOSPort/nativeSources/IOSNative.m @@ -16490,6 +16490,13 @@ JAVA_BOOLEAN com_codename1_impl_ios_IOSNative_intentsIndexingSupported___R_boole #if defined(CN1_USE_WATCHCONNECTIVITY) && !TARGET_OS_TV && !TARGET_OS_MACCATALYST #import "CN1WatchConnectivity.h" +// The generated declarations for the callbacks below. Without it every call into +// IOSWearableCallbacks is an implicit declaration, which clang 17 rejects +// outright -- the phone, Metal and watch targets all failed to compile. Kept +// beside the code it declares rather than with the includes at the top of the +// file, which is how CN1SmartHome.m carries the identical include for +// IOSHomeCallbacks, the class this one was modelled on. +#include "com_codename1_impl_ios_IOSWearableCallbacks.h" #if TARGET_OS_WATCH // Brings the session up on the watch without anyone asking for it. diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index 91eb873ef5d..761d440cc16 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -158,7 +158,12 @@ rather than failed, because unverifiable is not forged. A port with no stored report is a supported state, so adding a port does not start by hand-authoring one: every cell reads "No stored report" and the card -reads "unknown" until that port's first run publishes something to copy. +reads "unknown" until that port's first run publishes something to copy. That +does not make removing one free -- deleting the fallback for a port that has +published is refused, because the site serves this file exactly when the data +branch cannot be reached, and an established column would go unknown at the +moment the live data is missing. Retiring a port is still fine: drop it from the +manifest and the check no longer looks at it. "All tests run on all ports" is still enforced, just where it cannot be typed: the nightly sweep holds the *published* reports to it. A test left at `not-run` diff --git a/scripts/hellocodenameone/conformance/check_port_status_provenance.sh b/scripts/hellocodenameone/conformance/check_port_status_provenance.sh index b37dd0dcb87..ec493c66ef6 100755 --- a/scripts/hellocodenameone/conformance/check_port_status_provenance.sh +++ b/scripts/hellocodenameone/conformance/check_port_status_provenance.sh @@ -76,12 +76,15 @@ fi while IFS= read -r port; do head_report="${REPO_ROOT}/${REPORT_PATH}/${port}.json" - [ -f "${head_report}" ] || continue if git -C "${REPO_ROOT}" show "${base_ref}:${REPORT_PATH}/${port}.json" \ > "${base_dir}/${port}.json" 2>/dev/null; then - if cmp -s "${base_dir}/${port}.json" "${head_report}"; then + # Deliberately reached with no head report: deleting an existing fallback is + # its own finding, and skipping absent files was how it went unnoticed. + if [ -f "${head_report}" ] && cmp -s "${base_dir}/${port}.json" "${head_report}"; then continue fi + elif [ ! -f "${head_report}" ]; then + continue else # A report the branch ADDS. Skipping it here is how a pull request that also # adds a port could hand-author an entirely green snapshot for it -- the one diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 60f51efa979..0cdf7b884da 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -547,7 +547,7 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: def provenance_problems( port_id: str, before: dict | None, - after: dict, + after: dict | None, published: list[dict] | None = None, ) -> list[str]: """Refuse a hand-edited report. @@ -577,6 +577,24 @@ def provenance_problems( same as forged, and failing a pull request because a fetch flaked would teach people to route around this. """ + if after is None: + if before is None: + return [] + # Absent because it never existed and absent because someone removed it + # are different things, and only the first is harmless. The site serves + # this file whenever the data branch cannot be reached or its newest + # report predates the contract, so deleting one turns an established + # port's whole column unknown at exactly the moment the live data is + # missing -- which is the moment the fallback exists for. Retiring a + # port is still fine: drop it from the manifest and this check never + # looks at it. + return [ + f"{port_id}: the checked-in report was deleted. It is the fallback " + "the site serves when the data branch is unreachable, so an " + "established port would render as unknown. Only a port that has " + "never published needs no report." + ] + if before == after: return [] @@ -1253,9 +1271,8 @@ def main() -> int: port_id = port.get("id") base_path = args.base / f"{port_id}.json" head_path = REPO_ROOT / report_directory / f"{port_id}.json" - if not head_path.is_file(): - # Removed, or a port that has never published. Neither is an - # edit to a result. + if not head_path.is_file() and not base_path.is_file(): + # A port that has never published. Nothing to check. continue published = None if args.published is not None: @@ -1268,7 +1285,7 @@ def main() -> int: provenance_problems( port_id, read_json(base_path) if base_path.is_file() else None, - read_json(head_path), + read_json(head_path) if head_path.is_file() else None, published, ) ) diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index 4387c37ac77..c57c85e0728 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -691,6 +691,22 @@ def test_validate_accepts_a_port_with_no_stored_report(self): counts["drift"], ) + def test_provenance_rejects_deleting_an_established_fallback(self): + # A port with no stored report became a supported state so that ADDING a port would not + # have to begin by hand-authoring one. That must not also make removing an existing + # fallback free: the site serves this file whenever the data branch is unreachable or + # its newest report predates the contract, so deleting one turns a working port's whole + # column unknown at exactly the moment the live data is missing. + before = self.stored_reports()["tvos"] + problems = port_status.provenance_problems("tvos", before, None, published=[before]) + self.assertEqual(1, len(problems), problems) + self.assertIn("deleted", problems[0]) + + def test_provenance_ignores_a_port_that_has_no_report_either_side(self): + # Retiring a port drops it from the manifest, and the check never looks at it. This is + # the port that has simply never published. + self.assertEqual([], port_status.provenance_problems("freebsd", None, None, published=[])) + def test_provenance_ignores_an_untouched_report(self): before = self.stored_reports()["android"] self.assertEqual([], port_status.provenance_problems("android", before, dict(before))) From d8747d62d5d3da842eb2aa0c71e66ffb0babc923 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:46:51 +0300 Subject: [PATCH 10/14] Refuse a skip reason code that has no prefix Every string starts with the empty one. A reason_codes entry that lost its `prefix` to a typo therefore matched every reason a run could give, so the skip it was written to explain and any unrelated future skip of the same test both read as documented -- on the nightly coverage gate and on the public page, which would draw a green tick with a footnote pointing at an erratum about something else. Nothing objected: validate() checked only the erratum's four top-level descriptive fields and never looked inside its codes. Fixed at the source and on both sides that consume it. validate() now requires every reason code to carry a non-empty string prefix, and any `ports` it names to be a non-empty list of real port ids -- an unknown port silently narrowed a code to nothing in the same quiet way. skip_is_documented and the Hugo partial each refuse an empty prefix on their own rather than trusting the validator, because they are two independent copies of this matching rule and the page is the one that draws the tick. The twenty-four codes in the supplement today all pass unchanged. While adding the validator rule I named a local `ports`, which is the manifest's port list in scope for the whole function, and the returned port count started reading whichever erratum came last. test_contract_covers_registered_tests_and_goldens caught it -- the test whose literal totals this branch replaced with relationships, doing exactly the job the numbers could not. Co-Authored-By: Claude Opus 5 (1M context) --- .../partials/port-status-feature-status.html | 7 ++++- scripts/hellocodenameone/README.adoc | 5 ++- .../conformance/port_status.py | 29 +++++++++++++++-- .../conformance/test_port_status.py | 31 +++++++++++++++++++ 4 files changed, 68 insertions(+), 4 deletions(-) diff --git a/docs/website/layouts/partials/port-status-feature-status.html b/docs/website/layouts/partials/port-status-feature-status.html index 5fd038e0664..55379cea772 100644 --- a/docs/website/layouts/partials/port-status-feature-status.html +++ b/docs/website/layouts/partials/port-status-feature-status.html @@ -72,7 +72,12 @@ {{- with .ports -}} {{- $portAllowed = in . $port.id -}} {{- end -}} - {{- if and $portAllowed (hasPrefix $reason .prefix) -}}{{- $ok = true -}}{{- end -}} + {{- /* An empty prefix is not a match. hasPrefix answers true for + every reason against "", so a code that lost its prefix to a + typo would render any future skip of that test as green and + documented. The contract validator refuses such an erratum, + and this is the same rule on the side that draws the tick. */ -}} + {{- if and $portAllowed .prefix (hasPrefix $reason .prefix) -}}{{- $ok = true -}}{{- end -}} {{- end -}} {{- if not $ok -}}{{- $allMatched = false -}}{{- end -}} {{- end -}} diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index 761d440cc16..be3d8f909b7 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -173,7 +173,10 @@ rather than merely predating it. The single permitted exception is a `skip` the suite itself emits, and the sweep checks it against `port_status_supplement.json` the way the page does: the erratum has to name the test *and*, where it lists reason codes, account for the reason that run gave -from a port that code applies to. A port that quietly starts skipping a test +from a port that code applies to. Every reason code needs a non-empty `prefix` +and, if it names ports, real ones -- the validator enforces both, because every +string starts with the empty prefix, so a code that lost that field would +document any future skip of its test instead of the one it was written for. A port that quietly starts skipping a test fails the sweep by name rather than surfacing later as a failed website build. The sweep's closing assertion treats contract drift as the ordinary state it is, diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 0cdf7b884da..55529317d99 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -230,6 +230,30 @@ def validate(manifest: dict) -> dict: problems.append( "Every skip erratum needs test, reason, platform_support, and verification" ) + # A reason code with no prefix documents everything. Both matchers ask + # whether the reason starts with it, and every string starts with the + # empty one -- so an erratum that lost this field by a typo would turn + # any future skip of that test green, on the nightly gate and on the + # page alike, which is the opposite of what writing an erratum is for. + for code in item.get("reason_codes") or []: + prefix = code.get("prefix") + if not isinstance(prefix, str) or not prefix: + problems.append( + f"Skip erratum {item.get('test')} has a reason code with no prefix" + ) + # Deliberately not named `ports`: that is the manifest's port list, + # in scope for the whole of validate(), and rebinding it here left + # the returned port count reading whichever erratum happened to be + # last. The counts test caught it, which is what it is for. + code_ports = code.get("ports") + if code_ports is not None and ( + not isinstance(code_ports, list) + or not code_ports + or any(port not in port_ids for port in code_ports) + ): + problems.append( + f"Skip erratum {item.get('test')} has a reason code naming unknown ports" + ) manual_features = supplement.get("features", []) manual_feature_count = len(manual_features) @@ -426,9 +450,10 @@ def skip_is_documented(supplement: dict, port_id: str, test: str, reasons: list) continue if all( any( - (not code.get("ports") or port_id in code["ports"]) + code.get("prefix") + and (not code.get("ports") or port_id in code["ports"]) and isinstance(reason, str) - and reason.startswith(code.get("prefix", "")) + and reason.startswith(code["prefix"]) for code in codes ) for reason in reasons diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index c57c85e0728..61e682e6754 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -436,6 +436,37 @@ def test_coverage_rejects_a_skip_reason_scoped_to_another_port(self): problems, ) + def test_a_reason_code_with_no_prefix_documents_nothing(self): + # Both matchers ask whether the reason starts with the prefix, and every string starts + # with the empty one. An erratum that lost this field to a typo would therefore turn any + # future skip of that test green -- the exact opposite of what writing one is for. + supplement = { + "skip_reasons": [ + {"test": "CameraApiTest", "reason_codes": [{"ports": ["android"]}]} + ] + } + self.assertFalse( + port_status.skip_is_documented( + supplement, "android", "CameraApiTest", ["something-entirely-unrelated"] + ) + ) + self.assertFalse( + port_status.skip_is_documented( + supplement, "android", "CameraApiTest", ["needs-runtime-permission-on-and"] + ) + ) + + def test_validate_rejects_a_reason_code_with_no_prefix(self): + # And the configuration error is caught where it is made, rather than only failing to + # match later. Both halves matter: the matcher cannot be the only guard, because the + # page draws its own tick from its own copy of this rule. + supplement = { + "skip_reasons": [{"test": "CameraApiTest", "reason_codes": [{"prefix": ""}]}] + } + self.assertFalse( + port_status.skip_is_documented(supplement, "android", "CameraApiTest", ["anything"]) + ) + def test_coverage_rejects_a_skip_carrying_no_reason(self): # An erratum with reason codes documents the reasons it lists, not the test. A skip # that names none matches nothing, which is what the page already decides. From 7c3ece43e585dd10c1cf9c6cc45cdf3a3c670dc9 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 24 Aug 2026 18:25:10 +0300 Subject: [PATCH 11/14] Let a documented skip account for its golden in the screenshot count guard Build Android JDK 21 failed on one uncovered golden: GoogleWebMap. The JDK 8 and 17 legs of the same run produced all 151, and master is green, so nothing was broken -- the Google Maps tiles did not load over the network inside the runner's budget, which GoogleWebMapScreenshotTest already anticipates. After a full 24-second wait and a retry on a warm WebKit it prints "status=SKIPPED reason=map-tiles-never-loaded" and deliberately does not capture, and port_status_supplement.json documents that reason for android, ios-gl and ios-metal precisely so the public table renders it as a documented skip. That skip path could never actually succeed. The count guard runs first and counts a golden as covered only when it compared equal or different, so the skipped golden was uncovered and the job died on it regardless of what the test had said or what the errata allowed. Every component downstream agreed the skip was legitimate; the gate in front of them all disagreed, so a network the runner cannot reach reds a port at random. The guard now discounts a golden whose owning test reported a skip that THIS port's errata explain. That is not the weakening the comment above it warns against, and the distinction is the one the comment itself draws: the guard exists because "a dropped test leaves no per-test record behind to count", and a test that prints its skip left exactly such a record. Silence still fails. So does an unexplained skip, and so does a reason code written about another port -- map-tiles-never-loaded arriving from Linux, where the errata expect no-api-key, is a port misbehaving rather than an unreachable network, and stays a failure. A python that cannot answer discounts nothing. port_status.py grows a "documented-skips" query for it, reusing the same errata matching the coverage gate and the page already share, so there is one rule for what a documented skip is rather than a fourth copy of it. Four tests cover the discrimination: the documented skip, silence, an undocumented reason, and the right reason from the wrong port. Co-Authored-By: Claude Opus 5 (1M context) --- .../conformance/port_status.py | 66 +++++++++++++++++++ .../conformance/test_port_status.py | 55 ++++++++++++++++ scripts/lib/cn1ss.sh | 51 ++++++++++++++ 3 files changed, 172 insertions(+) diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 55529317d99..6209ee21395 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -691,6 +691,56 @@ def provenance_problems( ] +def documented_skip_goldens( + manifest: dict, port_id: str, logs: list[Path], reference: Path +) -> tuple[list[str], list[str]]: + """Golden names whose test reported a documented skip, and why. + + The screenshot count guard fails a run when a golden is not re-produced, + because a test that hangs or crashes leaves no per-test record and the + missing file is the only evidence there is. A test that prints + ``status=SKIPPED reason=...`` is the opposite of that: it left a record, and + the errata already say the reason is expected on this port. GoogleWebMap + skips on android and both iOS renderers when the Google Maps tiles never + load, which is a network the run cannot reach rather than anything about the + port -- and the guard failed the whole job over it anyway, so that skip path + could never actually succeed on a port that owns a golden. + + Only a skip that is *documented for this port* counts. Silence still fails, + an unexplained skip still fails, and a reason code written about another + port still fails, which is what keeps this from being a hole. + """ + supplement = read_json(SUPPLEMENT) + skipped: dict[str, list[str]] = {} + for path in logs: + try: + text = path.read_text(encoding="utf-8", errors="replace") + except OSError: + continue + for line in text.splitlines(): + match = SKIP_RE.search(line) + if not match: + continue + name, reason = match.group(1), match.group(2) + owner = name if name in test_to_feature(manifest) else screenshot_test(manifest, name) + if owner: + skipped.setdefault(owner, []).append(reason or "") + + accounted: list[str] = [] + notes: list[str] = [] + if not reference.is_dir(): + return accounted, notes + for golden in sorted(reference.glob("*.png")): + owner = screenshot_test(manifest, golden.stem) + if owner is None or owner not in skipped: + continue + reasons = [reason for reason in skipped[owner] if reason] + if skip_is_documented(supplement, port_id, owner, reasons): + accounted.append(golden.stem) + notes.append(f"{golden.stem}: {owner} skipped ({', '.join(reasons)})") + return accounted, notes + + def add_reason(entry: dict, reason: str) -> None: reasons = entry.setdefault("reasons", []) if reason and reason not in reasons: @@ -1234,6 +1284,14 @@ def build_parser() -> argparse.ArgumentParser: ), ) + skips_parser = subparsers.add_parser( + "documented-skips", + help="goldens whose test reported a skip this port's errata explain", + ) + skips_parser.add_argument("--port", required=True) + skips_parser.add_argument("--log", action="append", type=Path, default=[]) + skips_parser.add_argument("--reference", required=True, type=Path) + normalize_parser = subparsers.add_parser("normalize", help="write a normalized port report") normalize_parser.add_argument("--port", required=True) normalize_parser.add_argument("--log", action="append", type=Path, default=[]) @@ -1268,6 +1326,14 @@ def main() -> int: # shows the gap as "not run" until it does. print(f"port-status: checked-in snapshot {item}") return 0 + if args.command == "documented-skips": + accounted, notes = documented_skip_goldens( + manifest, args.port, args.log, args.reference + ) + for note in notes: + print(f"port-status: documented skip accounts for {note}", file=sys.stderr) + print(len(accounted)) + return 0 if args.command == "coverage": reports = {} for port in manifest.get("ports", []): diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index 61e682e6754..64174595ec6 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -467,6 +467,61 @@ def test_validate_rejects_a_reason_code_with_no_prefix(self): port_status.skip_is_documented(supplement, "android", "CameraApiTest", ["anything"]) ) + def documented_skip_count(self, port, marker, reference): + with tempfile.TemporaryDirectory() as tmp: + log = Path(tmp) / "suite.log" + log.write_text(marker, encoding="utf-8") + accounted, _ = port_status.documented_skip_goldens( + self.manifest, port, [log], port_status.REPO_ROOT / reference + ) + return accounted + + def test_a_documented_skip_accounts_for_its_golden(self): + # The screenshot count guard reads an unproduced golden as a test that hung, crashed or + # never delivered its frame -- which it is, when nothing else was said. A test that + # prints status=SKIPPED said something. GoogleWebMap takes that path when the Maps tiles + # never load, the errata document it on android, and the guard failed the whole job on + # the uncovered golden anyway -- so the skip path could never succeed on a port that + # owns a golden. + self.assertEqual( + ["GoogleWebMap"], + self.documented_skip_count( + "android", + "CN1SS:INFO:test=GoogleWebMap status=SKIPPED reason=map-tiles-never-loaded\n", + "scripts/android/screenshots", + ), + ) + + def test_silence_accounts_for_nothing(self): + # The case the guard exists for, and the one this must not soften: a test that hangs or + # crashes leaves no record, and the missing golden is the only evidence there is. + self.assertEqual( + [], self.documented_skip_count("android", "", "scripts/android/screenshots") + ) + + def test_an_undocumented_skip_accounts_for_nothing(self): + self.assertEqual( + [], + self.documented_skip_count( + "android", + "CN1SS:INFO:test=GoogleWebMap status=SKIPPED reason=something-nobody-wrote-down\n", + "scripts/android/screenshots", + ), + ) + + def test_a_skip_documented_for_another_port_accounts_for_nothing(self): + # map-tiles-never-loaded is written about android and the two iOS renderers. The same + # reason arriving from Linux, where the errata expect no-api-key instead, is a port + # behaving unexpectedly rather than a network nobody can reach. + self.assertEqual( + [], + self.documented_skip_count( + "linux-x64", + "CN1SS:INFO:test=GoogleWebMap status=SKIPPED reason=map-tiles-never-loaded\n", + "scripts/linux/screenshots", + ), + ) + def test_coverage_rejects_a_skip_carrying_no_reason(self): # An erratum with reason codes documents the reasons it lists, not the test. A skip # that names none matches nothing, which is what the page already decides. diff --git a/scripts/lib/cn1ss.sh b/scripts/lib/cn1ss.sh index 26c141791f5..38d51d775e1 100644 --- a/scripts/lib/cn1ss.sh +++ b/scripts/lib/cn1ss.sh @@ -361,6 +361,50 @@ print(sum(1 for r in results if isinstance(r, dict) and r.get("status") in ("equ PY } +# Goldens whose owning test reported a skip this port's errata explain. +# +# The count guard below treats an unproduced golden as evidence that a test hung, +# crashed or never delivered its frame -- which it is, when nothing else was +# said. A test that prints "status=SKIPPED reason=..." said something, and +# port_status.py checks that reason against port_status_supplement.json for THIS +# port before agreeing. Silence still counts as missing; so does an unexplained +# skip, and so does a reason code written about a different port. +# +# Without this the skip path some tests deliberately take could never succeed on +# a port that owns a golden: GoogleWebMapScreenshotTest skips when the Google +# Maps tiles never load -- documented for android, ios-gl and ios-metal, and +# rendered as a documented skip on the public table -- and the guard failed the +# whole job on the uncovered golden anyway. +cn1ss_count_documented_skips() { + local ref_dir="$1" + local script_dir repo_root status_script python_bin + if [ -z "${CN1SS_PORT_ID:-}" ] || [ -z "$ref_dir" ] || [ ! -d "$ref_dir" ]; then + echo 0 + return + fi + script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + repo_root="$(cd "$script_dir/../.." && pwd)" + status_script="$repo_root/scripts/hellocodenameone/conformance/port_status.py" + python_bin="${CN1SS_PYTHON_BIN:-python3}" + if [ ! -f "$status_script" ] || ! command -v "$python_bin" >/dev/null 2>&1; then + echo 0 + return + fi + local -a args=("$status_script" documented-skips --port "$CN1SS_PORT_ID" --reference "$ref_dir") + local log_var log_path + for log_var in CN1SS_SUITE_LOG CN1SS_SUITE_LOG_2 CN1SS_SUITE_LOG_3; do + log_path="${!log_var:-}" + if [ -n "$log_path" ] && [ -f "$log_path" ]; then + args+=(--log "$log_path") + fi + done + local count + # A failure here must not excuse anything: no answer means no discount. + count="$("$python_bin" "${args[@]}" 2>/dev/null || echo 0)" + count="${count//[^0-9]/}" + echo "${count:-0}" +} + # Count "missing_expected" results: a screenshot the suite captured and delivered # but which has NO committed golden under the reference directory. This is the # signal that a test ran for real yet its reference was never integrated -- the @@ -621,6 +665,13 @@ cn1ss_process_and_report() { allowed_missing="${allowed_missing//[^0-9]/}"; : "${allowed_missing:=0}" uncovered_count=$(( expected_count - covered_count )) [ "$uncovered_count" -lt 0 ] && uncovered_count=0 + local documented_skips + documented_skips=$(cn1ss_count_documented_skips "$ref_dir") + if [ "$documented_skips" -gt 0 ]; then + cn1ss_log "$documented_skips golden(s) accounted for by a skip this port's errata explain (see the lines above)." + uncovered_count=$(( uncovered_count - documented_skips )) + [ "$uncovered_count" -lt 0 ] && uncovered_count=0 + fi if [ "$uncovered_count" -gt "$allowed_missing" ]; then cn1ss_log "FATAL: $uncovered_count of $expected_count expected screenshot(s) were not produced and compared (only $covered_count covered); $allowed_missing tolerated (CN1SS_ALLOWED_MISSING)." cn1ss_log " A test failed to emit its screenshot, or the suite hung/crashed before finishing. The golden set under the comparison directory is the source of truth for how many screenshots must be produced." From 76b3de335e9fcf62d485520f055f1d5aa8a3670f Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 24 Aug 2026 18:31:22 +0300 Subject: [PATCH 12/14] Discount only the skipped goldens the run did not produce The discount named every golden a documented-skipped test owns, whether or not the run produced it. A test that owns several screenshots and captures a few before skipping -- CenteredDialogTitleScreenshotTest owns a light and a dark frame -- would have every one of them subtracted, while cn1ss_count_covered had already counted the captured ones. Each double subtraction cancels an uncovered golden belonging to some OTHER test, so the guard could pass a run that had genuinely lost a screenshot: the exact regression it exists to catch, reopened by the fix for the neighbouring hole. The query takes the comparison results now and skips any golden that compared equal or different, so what it returns is only what is actually absent. Passing the comparison is the caller's job and it already has the file open for count_covered; with no comparison available nothing changes, because then there is nothing recorded as produced. Co-Authored-By: Claude Opus 5 (1M context) --- .../conformance/port_status.py | 32 ++++++++++++++-- .../conformance/test_port_status.py | 37 +++++++++++++++++++ scripts/lib/cn1ss.sh | 9 ++++- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 6209ee21395..4770afa3d45 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -692,7 +692,11 @@ def provenance_problems( def documented_skip_goldens( - manifest: dict, port_id: str, logs: list[Path], reference: Path + manifest: dict, + port_id: str, + logs: list[Path], + reference: Path, + comparisons: list[Path] | None = None, ) -> tuple[list[str], list[str]]: """Golden names whose test reported a documented skip, and why. @@ -709,6 +713,13 @@ def documented_skip_goldens( Only a skip that is *documented for this port* counts. Silence still fails, an unexplained skip still fails, and a reason code written about another port still fails, which is what keeps this from being a hole. + + And only goldens that are actually absent. The caller subtracts this count + from the number of uncovered goldens, so naming one the run did compare + would subtract a golden nothing was missing -- and the spare subtraction + would then hide a genuinely uncovered golden belonging to some other test. + A test that owns several screenshots and captures a few before skipping is + exactly that case. """ supplement = read_json(SUPPLEMENT) skipped: dict[str, list[str]] = {} @@ -726,13 +737,27 @@ def documented_skip_goldens( if owner: skipped.setdefault(owner, []).append(reason or "") + compared: set[str] = set() + for path in comparisons or []: + if not path.is_file(): + continue + try: + payload = read_json(path) + except ContractError: + continue + for result in payload.get("results", []): + if isinstance(result, dict) and result.get("status") in {"equal", "different"}: + name = result.get("test") + if name: + compared.add(name) + accounted: list[str] = [] notes: list[str] = [] if not reference.is_dir(): return accounted, notes for golden in sorted(reference.glob("*.png")): owner = screenshot_test(manifest, golden.stem) - if owner is None or owner not in skipped: + if owner is None or owner not in skipped or golden.stem in compared: continue reasons = [reason for reason in skipped[owner] if reason] if skip_is_documented(supplement, port_id, owner, reasons): @@ -1291,6 +1316,7 @@ def build_parser() -> argparse.ArgumentParser: skips_parser.add_argument("--port", required=True) skips_parser.add_argument("--log", action="append", type=Path, default=[]) skips_parser.add_argument("--reference", required=True, type=Path) + skips_parser.add_argument("--compare", action="append", type=Path, default=[]) normalize_parser = subparsers.add_parser("normalize", help="write a normalized port report") normalize_parser.add_argument("--port", required=True) @@ -1328,7 +1354,7 @@ def main() -> int: return 0 if args.command == "documented-skips": accounted, notes = documented_skip_goldens( - manifest, args.port, args.log, args.reference + manifest, args.port, args.log, args.reference, args.compare ) for note in notes: print(f"port-status: documented skip accounts for {note}", file=sys.stderr) diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index 64174595ec6..8b32c3fa25a 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -522,6 +522,43 @@ def test_a_skip_documented_for_another_port_accounts_for_nothing(self): ), ) + def test_only_goldens_the_run_did_not_produce_are_discounted(self): + # The caller subtracts this count from the number of UNCOVERED goldens, so naming one the + # run did compare would subtract a golden nothing was missing -- and that spare + # subtraction would then hide a genuinely uncovered golden belonging to some other test, + # which is the regression the guard exists to catch. A test owning several screenshots + # that captures a few before skipping is exactly the case. + marker = "CN1SS:INFO:test=CenteredDialogTitle status=SKIPPED reason=phone-dialog-on-watch\n" + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + reference = root / "ref" + reference.mkdir() + for name in ("CenteredDialogTitle_dark", "CenteredDialogTitle_light"): + (reference / (name + ".png")).write_bytes(b"") + log = root / "suite.log" + log.write_text(marker, encoding="utf-8") + nothing_compared = root / "none.json" + nothing_compared.write_text(json.dumps({"results": []}), encoding="utf-8") + one_compared = root / "one.json" + one_compared.write_text( + json.dumps( + {"results": [{"test": "CenteredDialogTitle_light", "status": "equal"}]} + ), + encoding="utf-8", + ) + self.assertEqual( + ["CenteredDialogTitle_dark", "CenteredDialogTitle_light"], + port_status.documented_skip_goldens( + self.manifest, "watchos", [log], reference, [nothing_compared] + )[0], + ) + self.assertEqual( + ["CenteredDialogTitle_dark"], + port_status.documented_skip_goldens( + self.manifest, "watchos", [log], reference, [one_compared] + )[0], + ) + def test_coverage_rejects_a_skip_carrying_no_reason(self): # An erratum with reason codes documents the reasons it lists, not the test. A skip # that names none matches nothing, which is what the page already decides. diff --git a/scripts/lib/cn1ss.sh b/scripts/lib/cn1ss.sh index 38d51d775e1..72ff6914ecd 100644 --- a/scripts/lib/cn1ss.sh +++ b/scripts/lib/cn1ss.sh @@ -377,6 +377,7 @@ PY # whole job on the uncovered golden anyway. cn1ss_count_documented_skips() { local ref_dir="$1" + local compare_json="$2" local script_dir repo_root status_script python_bin if [ -z "${CN1SS_PORT_ID:-}" ] || [ -z "$ref_dir" ] || [ ! -d "$ref_dir" ]; then echo 0 @@ -391,6 +392,12 @@ cn1ss_count_documented_skips() { return fi local -a args=("$status_script" documented-skips --port "$CN1SS_PORT_ID" --reference "$ref_dir") + # The comparison results, so a golden the run DID produce is never discounted: + # it is already in covered_count, and subtracting it again would hide an + # uncovered golden belonging to some other test. + if [ -n "$compare_json" ] && [ -s "$compare_json" ]; then + args+=(--compare "$compare_json") + fi local log_var log_path for log_var in CN1SS_SUITE_LOG CN1SS_SUITE_LOG_2 CN1SS_SUITE_LOG_3; do log_path="${!log_var:-}" @@ -666,7 +673,7 @@ cn1ss_process_and_report() { uncovered_count=$(( expected_count - covered_count )) [ "$uncovered_count" -lt 0 ] && uncovered_count=0 local documented_skips - documented_skips=$(cn1ss_count_documented_skips "$ref_dir") + documented_skips=$(cn1ss_count_documented_skips "$ref_dir" "$compare_json_out") if [ "$documented_skips" -gt 0 ]; then cn1ss_log "$documented_skips golden(s) accounted for by a skip this port's errata explain (see the lines above)." uncovered_count=$(( uncovered_count - documented_skips )) From b2823c92479dd2046e969beb946ece638577d2c0 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:54:22 +0300 Subject: [PATCH 13/14] Catch a registered test that no port reports at all The coverage gate decided a missing test was tolerable by asking whether some OLDER report carried it -- a run that already knew about the test proves a later run has no excuse. That comparison is blind in exactly the case that matters most: when the test is missing from every report there is no older report left to prove anything, so every port is excused and the sweep prints success over a test nothing runs anywhere. A stuck producer republishing fresh reports from an outdated contract holds that state indefinitely, and the gate stays green the whole time. The sweep now also reads the manifest at each report's own commit, through the same gh contents API it already uses to read the reports. A run whose own contract listed the test and reported nothing for it fails on its own, whatever the other ports did. The transient this must never break -- a run that genuinely predates the test -- is untouched, because that run's contract does not define it: verified against the live data, where the five Apple ports sit at a commit before LogSubclassCaptureTest and stay green. coverage_problems takes the contracts as an optional argument, so it stays offline and testable; a port whose manifest could not be fetched keeps the weaker report-to-report comparison rather than being excused or accused on a guess. Co-Authored-By: Claude Opus 5 (1M context) --- .../conformance/backfill_port_status.sh | 19 ++++++- .../conformance/port_status.py | 49 +++++++++++++++-- .../conformance/test_port_status.py | 53 +++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/scripts/hellocodenameone/conformance/backfill_port_status.sh b/scripts/hellocodenameone/conformance/backfill_port_status.sh index a0144f43132..16a269dd9c6 100755 --- a/scripts/hellocodenameone/conformance/backfill_port_status.sh +++ b/scripts/hellocodenameone/conformance/backfill_port_status.sh @@ -405,7 +405,8 @@ echo "Port status sweep: published ${published} report(s), ${skipped} already cu stale_days="$(jq -r '.stale_after_days' "${MANIFEST}")" problems=() published_dir="${tmp_dir}/published" -mkdir -p "${published_dir}" +contracts_dir="${tmp_dir}/contracts" +mkdir -p "${published_dir}" "${contracts_dir}" while IFS= read -r port; do if ! gh api "repos/${GITHUB_REPOSITORY}/contents/ports/${port}.json?ref=${DATA_BRANCH}" \ --jq '.content' 2>/dev/null | decode_base64 > "${tmp_dir}/check.json" 2>/dev/null; then @@ -413,6 +414,19 @@ while IFS= read -r port; do continue fi cp "${tmp_dir}/check.json" "${published_dir}/${port}.json" + # The contract this report's own run was built against. Comparing reports to + # each other cannot see a test missing from ALL of them -- there is no older + # report left to prove it existed -- and a test nothing runs anywhere is the + # worst version of what this gate is for. Reading the manifest at the report's + # commit answers it directly: a run whose own contract listed the test has no + # excuse for reporting nothing. A fetch that fails simply leaves the port out, + # which keeps the weaker comparison rather than inventing an obligation. + report_commit="$(jq -r '.commit // empty' "${tmp_dir}/check.json" 2>/dev/null || true)" + if [ -n "${report_commit}" ]; then + gh api "repos/${GITHUB_REPOSITORY}/contents/docs/website/data/port_status.json?ref=${report_commit}" \ + --jq '.content' 2>/dev/null | decode_base64 > "${contracts_dir}/${port}.json" 2>/dev/null \ + || rm -f "${contracts_dir}/${port}.json" + fi # Freshness alone is not enough: a published report the website rejects # leaves the column on its checked-in fallback, which is the state this # sweep exists to detect. @@ -488,4 +502,5 @@ echo "Every port in the contract has a report inside the ${stale_days}-day windo # writes in a pull request can reach it. A registered test runs on every port, # and the only permitted exception is a skip the suite itself emits with an # erratum explaining it. -python3 "${SCRIPT_DIR}/port_status.py" coverage --reports "${published_dir}" +python3 "${SCRIPT_DIR}/port_status.py" coverage \ + --reports "${published_dir}" --contracts "${contracts_dir}" diff --git a/scripts/hellocodenameone/conformance/port_status.py b/scripts/hellocodenameone/conformance/port_status.py index 4770afa3d45..96ddb53f390 100755 --- a/scripts/hellocodenameone/conformance/port_status.py +++ b/scripts/hellocodenameone/conformance/port_status.py @@ -462,7 +462,11 @@ def skip_is_documented(supplement: dict, port_id: str, test: str, reasons: list) return False -def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: +def coverage_problems( + manifest: dict, + reports: dict[str, dict], + contracts: dict[str, set[str]] | None = None, +) -> list[str]: """Hold the *published* reports to "every registered test runs on every port". This is the gate that used to live, badly, in the checked-in snapshots. Two @@ -478,6 +482,14 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: test the port has dropped rather than one it has not reached. No history lookup and no grace period to tune; the reports date themselves. + That comparison is blind to a test missing from *every* report, because then + no report is the older one that proves it existed -- and a test nothing runs + anywhere is the worst version of the failure this gate is for, not a + tolerable one. ``contracts`` closes it: the set of tests each report's own + commit defined, which the caller reads at that commit. A report whose + contract already listed the test has no excuse for omitting it, whatever the + other ports did. Offline callers pass None and keep the weaker comparison. + A ``skip`` is the one permitted exception, and only with an erratum that accounts for the reason the run actually gave. Reading skips out of the checked-in fallbacks instead -- which is all validate() can see -- would let @@ -541,9 +553,11 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: if stamp is None: problems.append(f"{port}: report has no usable generated_at") continue + own_contract = (contracts or {}).get(port) + absent = set(mapped) - set(tests) dropped = sorted( name - for name in set(mapped) - set(tests) + for name in absent if name in known_since and known_since[name] < stamp ) if dropped: @@ -552,6 +566,13 @@ def coverage_problems(manifest: dict, reports: dict[str, dict]) -> list[str]: + ", ".join(dropped) + ", which an earlier run on another port already covered" ) + if own_contract is not None: + unreported = sorted(name for name in absent - set(dropped) if name in own_contract) + if unreported: + problems.append( + f"{port}: ran at {report.get('commit') or 'an unknown commit'}, which " + "defines " + ", ".join(unreported) + ", and reported nothing for them" + ) return problems @@ -1289,6 +1310,14 @@ def build_parser() -> argparse.ArgumentParser: type=Path, help="directory of published .json reports", ) + coverage_parser.add_argument( + "--contracts", + type=Path, + help=( + "directory of .json manifests read at each report's own commit; " + "omit when they cannot be fetched" + ), + ) provenance_parser = subparsers.add_parser( "provenance", @@ -1369,7 +1398,21 @@ def main() -> int: print(f"port-status: no published report for {port_id}", file=sys.stderr) return 1 reports[port_id] = read_json(path) - problems = coverage_problems(manifest, reports) + contracts = None + if args.contracts is not None: + contracts = {} + for port_id in reports: + path = args.contracts / f"{port_id}.json" + if not path.is_file(): + continue + try: + contracts[port_id] = set(test_to_feature(read_json(path))) + except ContractError: + # A manifest we cannot read proves nothing. Leaving the + # port out keeps the weaker comparison rather than + # inventing an obligation or excusing one. + continue + problems = coverage_problems(manifest, reports, contracts) for problem in problems: print(f"port-status coverage: {problem}", file=sys.stderr) if problems: diff --git a/scripts/hellocodenameone/conformance/test_port_status.py b/scripts/hellocodenameone/conformance/test_port_status.py index 8b32c3fa25a..654d5644244 100755 --- a/scripts/hellocodenameone/conformance/test_port_status.py +++ b/scripts/hellocodenameone/conformance/test_port_status.py @@ -573,6 +573,59 @@ def test_coverage_rejects_a_skip_carrying_no_reason(self): problems, ) + def test_coverage_catches_a_test_absent_from_every_report(self): + # Comparing reports to each other cannot see this: with the test missing everywhere, + # there is no older report left to prove it existed, so every port is excused and the + # gate prints success over a test nothing runs anywhere -- the worst version of the + # failure this gate is for. Each report's own contract answers it directly. + reports = self.stored_reports() + victim = "CryptoApiTest" + for report in reports.values(): + report["tests"].pop(victim, None) + self.assertEqual([], port_status.coverage_problems(self.manifest, reports)) + + contract = set(port_status.test_to_feature(self.manifest)) + problems = port_status.coverage_problems( + self.manifest, reports, {port: contract for port in reports} + ) + self.assertEqual(len(reports), len(problems), problems) + self.assertTrue(all(victim in problem for problem in problems), problems) + + def test_coverage_tolerates_a_report_whose_own_contract_predates_the_test(self): + # The state every port is in for a few hours after a test is registered, and the one + # this must never fail: the run happened against a manifest that did not define the + # test, so there was nothing to report. + reports = self.stored_reports() + victim = "CryptoApiTest" + for report in reports.values(): + report["tests"].pop(victim, None) + # Each port's own contract is exactly what its run reported, which is what "the run + # predates the test" means. Handing every port the CURRENT contract minus one test + # would instead accuse the five Apple ports of dropping LogSubclassCaptureTest, which + # their reports really do predate -- and the gate would be right to say so. + self.assertEqual( + [], + port_status.coverage_problems( + self.manifest, + reports, + {port: set(report["tests"]) for port, report in reports.items()}, + ), + ) + + def test_coverage_leaves_a_port_alone_when_its_contract_is_unknown(self): + # A manifest the sweep could not fetch proves nothing, so that port keeps the weaker + # report-to-report comparison rather than being excused or accused. + reports = self.stored_reports() + victim = "CryptoApiTest" + for report in reports.values(): + report["tests"].pop(victim, None) + contract = set(port_status.test_to_feature(self.manifest)) + problems = port_status.coverage_problems( + self.manifest, reports, {"android": contract} + ) + self.assertEqual(1, len(problems), problems) + self.assertIn("android", problems[0]) + def test_coverage_accepts_a_report_older_than_the_test(self): # The state every port is in between the commit that registers a test and that port's # next master run. Failing here would put the old ritual straight back: the only way to From cdb1433cb54247fce707e4bf78ad4b3cd66ebb74 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Mon, 24 Aug 2026 19:54:37 +0300 Subject: [PATCH 14/14] Document the contract-at-commit half of the coverage gate The README described only the report-to-report comparison, which is the half that cannot see a test missing from every report. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/hellocodenameone/README.adoc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/hellocodenameone/README.adoc b/scripts/hellocodenameone/README.adoc index be3d8f909b7..baccc10cd37 100644 --- a/scripts/hellocodenameone/README.adoc +++ b/scripts/hellocodenameone/README.adoc @@ -169,7 +169,11 @@ manifest and the check no longer looks at it. the nightly sweep holds the *published* reports to it. A test left at `not-run` fails the sweep, and so does a test missing from one port's report when an earlier run on another port already covered it -- that port dropped the test -rather than merely predating it. The single permitted exception is a `skip` the +rather than merely predating it. Comparing reports to each other cannot see a +test missing from *all* of them, so the sweep also reads the manifest at each +report's own commit: a run whose own contract listed the test and reported +nothing for it fails, however the other ports behaved. The single permitted +exception is a `skip` the suite itself emits, and the sweep checks it against `port_status_supplement.json` the way the page does: the erratum has to name the test *and*, where it lists reason codes, account for the reason that run gave