From 215437e29b134f2df523955660f7efe5c31ef048 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 31 Jul 2026 22:44:56 +0200 Subject: [PATCH 1/2] Add a tool that opens the core's whole test corpus on a device The instrumented tests open nine files. OpenDocument.core's input corpus has a couple of hundred, across formats no test here touches, and since #548 the app hands almost all of them to CoreLoader. Nothing was looking at what comes out the other side. This walks that corpus one document at a time and records what the app made of each: a screenshot, the text the WebView ended up showing, the crash buffer, and whether the process was still alive afterwards. It asserts nothing and fails no build - the output is a table of signals plus an image per document, and a human decides what broken means. Diffing against the core's reference output is deliberately not attempted; that comparison already exists in the core's own suite, and the question here is what the app shows, WebView and chrome included. Three of the mechanics are not obvious and the comments say why at each site. Files go in through run-as, not adb push: the app declares only INTERNET, so a file on shared storage is unreadable to it - even under its own /sdcard/Android/data/, which returns EACCES when shell owns it. The intent carries no mime type, which keeps MetadataLoader's libmagic detection in the path instead of taking the caller's word for it. And uiautomator dump runs twice per document, because a WebView only builds its accessibility tree once something asks for one - the first dump after a load has no text in it. The signals match the app's strings in full rather than a keyword. A keyword matches the document too: the app's own about.odt and changelog fixtures contain "upload" and "password-protected", and an earlier version reported them as failures they were not. The crash check likewise requires a fatal that names our process, because uiautomator's own launcher logs "D AndroidRuntime" on every iteration - matching that alone reported all 225 documents as crashes. Slow is not broken, and a fixed shutter cannot tell them apart: a 5MB .doc was still showing "Loading..." at 11s and a 284KB .csv was still blank at 6s, and both render fine given a minute. So each document is shot repeatedly until two frames agree in size to within 1%, and a row that never settles is marked still-rendering rather than being read as a blank page. It never taps. When a load fails the app offers to upload the document to the conversion service, and that offer is a dialog with a positive button - photographing it sends nothing, accepting it would send someone's test document to a third party. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Qis7KBzd1YHZqPa9ZEd6WV --- tools/render-sweep/README.md | 115 ++++++++++++++ tools/render-sweep/render-sweep.sh | 233 +++++++++++++++++++++++++++++ 2 files changed, 348 insertions(+) create mode 100644 tools/render-sweep/README.md create mode 100755 tools/render-sweep/render-sweep.sh diff --git a/tools/render-sweep/README.md b/tools/render-sweep/README.md new file mode 100644 index 000000000000..eebaa398c9a6 --- /dev/null +++ b/tools/render-sweep/README.md @@ -0,0 +1,115 @@ +# render-sweep + +Opens every document of a corpus on a connected device, one at a time, and screenshots +what the app made of it. + +The instrumented tests open nine files. `OpenDocument.core`'s own input corpus has a couple +of hundred, across formats no test here touches, and the app now hands almost all of them to +`CoreLoader` (see the supported-file-types section of `CLAUDE.md`). This walks that corpus so +a format that renders blank, renders half, or takes the process down with it is something you +can look at rather than something a user reports. + +It is a **looking** tool, not a test: nothing here asserts, and nothing here fails a build. +The output is a table of signals plus a screenshot per document, and a human (or a model) +decides what "broken" means. There is deliberately no attempt to diff against the core's +reference output — that comparison already exists in the core's own suite, and the question +here is what the *app* shows, WebView, chrome and all. + +## Running it + +You need a device or emulator on `adb`, the pro debug build installed on it, and a checkout +of `OpenDocument.core` with its test submodules initialised. + +```sh +./gradlew assembleProDebug +tools/render-sweep/render-sweep.sh --install +``` + +`--install` pushes the apk it just built; drop it on later runs. By default the corpus is the +sibling checkout `../OpenDocument.core/test/data/input` and results land in +`build/render-sweep` (gitignored, like the rest of `build/`). + +```sh +# somewhere else, or just one format, or just a few +tools/render-sweep/render-sweep.sh --corpus ~/corpora/docs +tools/render-sweep/render-sweep.sh --filter '\.ods$' +tools/render-sweep/render-sweep.sh --filter 'odr-public' --limit 20 +``` + +A full run over ~225 documents takes about 80 minutes, most of it the fixed wait after each +launch. The screen is on and rendering the whole time, so put the device on a charger. + +## What you get + +``` +build/render-sweep/ + results.tsv one row per document: launch status, png size, text nodes, signal + shots/ full resolution screenshot per document + small/ the same, downscaled, for flipping through quickly + ui/ uiautomator dump per document - the text the WebView actually showed + logs/ crash buffer and error lines per document +``` + +The `signal` column is a triage hint, not a verdict: + +| signal | what it means | +| --- | --- | +| `ok` | launched, survived, nothing obviously wrong — still worth a look | +| `CRASH` / `CRASH-died` | a fatal naming our process, or the process was gone afterwards | +| `notfound` | the app could not read the file at all | +| `encrypted` | the password dialog came up (expected for the encrypted fixtures) | +| `unsupported` | the app put up its "try opening it in another app" snackbar | +| `upload-offer` | the core declined it and the app offered to convert it online | +| `still-rendering` | the screen was still changing when the shutter gave up — see below | +| `launch-*` | `am start` itself did not report `ok` | + +Each of these matches one of the app's strings in full rather than a keyword, because a +keyword matches the *document* too: the app's own `about.odt` and changelog fixtures contain +the words "upload" and "password-protected", and an earlier version of this reported them as +failures they were not. Anything matching on document text will do the same. + +**A blank render has no signal of its own** — that is the one thing the device will not tell +you. The run ends by printing the documents with the fewest text nodes and the smallest +screenshots, which is where blank and near-blank pages sort to. Start there, then look at the +rest. + +Slow is not broken, and the shutter cannot tell them apart on its own. A 5 MB `.doc` was +still showing "Loading…" after 11 seconds and a 284 KB `.csv` was still a blank white page +after 6, and both render fine given a minute. So each document is shot repeatedly until two +frames agree in size to within 1%; if that never happens the row is `still-rendering` and its +screenshot says nothing about the document. Do not read a `still-rendering` blank as a bug. + +Screenshots of the private corpus stay on your machine. Do not paste them into an issue +without checking what is in them. + +## Why it works the way it does + +Three of the mechanics look arbitrary and are not. The script says the same thing at each +site, in more detail: + +- **Files go in through `run-as`, not `adb push`.** The app declares only `INTERNET`, so a + file pushed to shared storage is unreadable to it — even under its own + `/sdcard/Android/data/`, which comes back `EACCES` when shell owns the file. Piping + into internal storage via `run-as` is the route that works, and it is why this needs the + debug build rather than the store one. +- **The intent carries no mime type.** That leaves `MetadataLoader`'s libmagic detection in + the path instead of taking a caller's word for the type, which is the half of the app worth + exercising. +- **`uiautomator dump` runs twice per document.** A WebView only builds its accessibility + tree once something asks for one, so the first dump after a load has no text in it and the + second has the document. + +And one about the signals: the crash check requires a fatal that *names our process*. The +first version of this matched `AndroidRuntime` anywhere in logcat and reported all 225 +documents as crashes, because `uiautomator`'s own launcher logs that line on every iteration. + +## What it does not do + +It never taps. When a load fails the app offers to upload the document to the conversion +service, and that offer is a dialog with a positive button — photographing it sends nothing, +accepting it would send someone's test document to a third party. There is no code path here +that sends a tap, and adding one would change what running this means. + +It also skips formats the app does not claim (`ttf`, `otf`, `svm`, `pages`, `wpd`, `sxw`, and +so on). Those reach the app only if a user picks one deliberately; checking that they are +declined gracefully is a different sweep from this one. diff --git a/tools/render-sweep/render-sweep.sh b/tools/render-sweep/render-sweep.sh new file mode 100755 index 000000000000..d29308de0c3e --- /dev/null +++ b/tools/render-sweep/render-sweep.sh @@ -0,0 +1,233 @@ +#!/usr/bin/env bash +# +# Opens every document of a corpus on a connected device, one at a time, and +# records what the app made of it: a screenshot, the text the WebView ended up +# showing, and whatever logcat had to say. Meant for the input corpus of +# OpenDocument.core (test/data/input), which is far larger than the handful of +# files in app/src/androidTest/assets and covers formats no test opens. +# +# It only ever looks. Nothing is tapped, which matters: when a load fails the app +# offers to upload the document to the conversion service, and that offer is a +# dialog with a positive button. Photographing it sends nothing. Accepting it +# would send someone's test document to a third party, so this script has no code +# path that taps anything at all. +# +# Usage: tools/render-sweep/render-sweep.sh [--corpus DIR] [--filter REGEX] ... +# See README.md next to this file. + +set -u + +PKG=at.tomtasche.reader.pro +# the activity-alias, not the relocated class - see the package names section of +# CLAUDE.md for why the component names still read at.tomtasche.reader.* +ALIAS_SUFFIX=at.tomtasche.reader.ui.activity.MainActivity.STRICT_CATCH + +REPO_ROOT=$(cd "$(dirname "$0")/../.." && pwd) +# the sibling checkout this repo is normally developed next to +CORPUS=$REPO_ROOT/../OpenDocument.core/test/data/input +OUT=$REPO_ROOT/build/render-sweep +FILTER= +LIMIT=0 +INSTALL=0 + +# every extension the app claims. Kept as a plain list rather than derived, +# because the tables live in libodr_jni and reading them needs a device - the +# same reason SupportedDocumentTypesTest is an instrumented test. A format added +# upstream and missing here just means this sweep skips it. +CLAIMED='csv|doc|docm|docx|dot|dotm|dotx|fodg|fodp|fods|fodt|odg|odm|odp|ods|odt|otg|otm|otp|ots|ott|pdf|pot|potm|potx|pps|ppsm|ppsx|ppt|pptm|pptx|text|txt|xlm|xls|xlsm|xlsx|xlt|xltm|xltx|zip' + +usage() { + cat <<'EOF' +Opens every document of a corpus on a connected device, one at a time, and records +what the app made of it: a screenshot, the text the WebView showed, and logcat. + +It only ever looks - nothing is tapped, so the app's offer to upload a document it +could not open is photographed rather than accepted. See README.md next to this file. + +Options: + --corpus DIR directory to walk (default: ../OpenDocument.core/test/data/input) + --out DIR where to write results (default: build/render-sweep) + --package ID application id to drive (default: at.tomtasche.reader.pro) + --filter REGEX only paths matching this (case-insensitive), e.g. '\.odt$' + --limit N stop after N documents + --install adb install the pro debug apk before starting + --help +EOF +} + +while [ $# -gt 0 ]; do + case "$1" in + --corpus) CORPUS=$2; shift 2 ;; + --out) OUT=$2; shift 2 ;; + --package) PKG=$2; shift 2 ;; + --filter) FILTER=$2; shift 2 ;; + --limit) LIMIT=$2; shift 2 ;; + --install) INSTALL=1; shift ;; + --help|-h) usage; exit 0 ;; + *) echo "unknown option: $1" >&2; usage >&2; exit 2 ;; + esac +done + +ADB=${ADB:-} +if [ -z "$ADB" ]; then + if [ -n "${ANDROID_HOME:-}" ] && [ -x "$ANDROID_HOME/platform-tools/adb" ]; then + ADB=$ANDROID_HOME/platform-tools/adb + else + ADB=$(command -v adb || true) + fi +fi +[ -n "$ADB" ] || { echo "adb not found - set ANDROID_HOME or ADB" >&2; exit 1; } + +[ -d "$CORPUS" ] || { echo "corpus not found: $CORPUS" >&2; exit 1; } +"$ADB" get-state >/dev/null 2>&1 || { echo "no device (adb get-state failed)" >&2; exit 1; } + +if [ "$INSTALL" = 1 ]; then + APK=$REPO_ROOT/app/build/outputs/apk/pro/debug/app-pro-debug.apk + [ -f "$APK" ] || { echo "no apk at $APK - run ./gradlew assembleProDebug" >&2; exit 1; } + "$ADB" install -r -d "$APK" || exit 1 +fi +"$ADB" shell pm path "$PKG" >/dev/null 2>&1 || { + echo "$PKG is not installed - pass --install or install it yourself" >&2; exit 1; } + +# wc -c rather than stat, whose flags differ between macos and linux +filesize() { wc -c < "$1" | tr -d ' '; } + +mkdir -p "$OUT/shots" "$OUT/logs" "$OUT/ui" +TSV=$OUT/results.tsv +printf 'idx\trepo\trelpath\text\tbytes\tlaunch\tpngbytes\ttextnodes\talive\tsettled\tsignal\n' > "$TSV" + +LIST=$OUT/filelist.txt +(cd "$CORPUS" && find . -type f ! -path '*/.git*' | sed 's|^\./||' | sort) > "$LIST" + +"$ADB" shell run-as "$PKG" mkdir -p files/sweep >/dev/null 2>&1 + +i=0 +# the list is read on fd 3 because the adb calls in the body would otherwise eat it +while IFS= read -r rel <&3; do + ext=$(printf '%s' "${rel##*.}" | tr 'A-Z' 'a-z') + printf '%s' "$ext" | grep -qiE "^($CLAIMED)$" || continue + [ -n "$FILTER" ] && { printf '%s' "$rel" | grep -qiE "$FILTER" || continue; } + i=$((i+1)) + [ "$LIMIT" -gt 0 ] && [ "$i" -gt "$LIMIT" ] && { i=$((i-1)); break; } + + idx=$(printf '%03d' $i) + src=$CORPUS/$rel + repo=${rel%%/*} + bytes=$(filesize "$src") + safe=$(printf '%s' "$rel" | tr '/ $+' '____') + + "$ADB" shell am force-stop "$PKG" >/dev/null 2>&1 + "$ADB" logcat -c -b all >/dev/null 2>&1 + + # The app declares only INTERNET, so it cannot read a file pushed anywhere on + # shared storage - even its own /sdcard/Android/data/ comes back EACCES + # when the file is owned by shell. Piping it into the app's internal storage + # through run-as (which the debug build allows) is the one route that works. + # + # It lands as doc. rather than under its own name so that names with + # spaces, '$' or '+' need no quoting here and no percent-encoding in the URI + # below. Only the extension carries information the app might use. + "$ADB" shell run-as "$PKG" sh -c "rm -f files/sweep/doc.*" >/dev/null 2>&1 + "$ADB" shell "run-as $PKG sh -c 'cat > files/sweep/doc.$ext'" < "$src" >/dev/null 2>&1 + + # No -t: leaving the mime type off is what a real opener rarely does, but it + # puts MetadataLoader's libmagic detection in the path instead of taking the + # caller's word for it, which is the more interesting half of the app. + launch=$("$ADB" shell am start -W -a android.intent.action.VIEW \ + -d "file:///data/data/$PKG/files/sweep/doc.$ext" \ + -n "$PKG/$ALIAS_SUFFIX" 2>&1 | grep -E '^Status:' | head -1 | awk '{print $2}') + [ -z "$launch" ] && launch=nostart + + # Rendering is not something the app signals from outside, so this waits - and a + # fixed wait is not enough. A 5MB .doc was still showing "Loading..." at 11s and a + # 284KB .csv was still a blank white page at 6s; both render fine given a minute, + # so a fixed shutter reports slow documents as broken ones. + # + # Instead: shoot, wait, shoot again, and keep going until two frames agree in size + # to within 1% (the clock and battery icon change, so they are never byte-equal). + wait=$(( 4 + bytes / 2000000 )) + [ $wait -gt 12 ] && wait=12 + sleep $wait + + shot=$OUT/shots/$idx-$safe.png + "$ADB" exec-out screencap -p > "$shot" 2>/dev/null + pngbytes=$(filesize "$shot" 2>/dev/null || echo 0) + settled=no + for _ in 1 2 3 4 5 6 7 8; do + sleep 6 + "$ADB" exec-out screencap -p > "$shot.next" 2>/dev/null + nextbytes=$(filesize "$shot.next" 2>/dev/null || echo 0) + mv -f "$shot.next" "$shot" + delta=$(( pngbytes > nextbytes ? pngbytes - nextbytes : nextbytes - pngbytes )) + prev=$pngbytes + pngbytes=$nextbytes + [ "$prev" -gt 0 ] && [ $(( delta * 100 )) -le "$prev" ] && { settled=yes; break; } + done + + # Twice on purpose. A WebView only builds its accessibility tree once + # something asks for one, so the first dump after a load comes back with no + # text at all and the second has the document in it. + "$ADB" shell uiautomator dump /sdcard/render-sweep.xml >/dev/null 2>&1 + "$ADB" shell uiautomator dump /sdcard/render-sweep.xml >/dev/null 2>&1 + "$ADB" shell cat /sdcard/render-sweep.xml 2>/dev/null > "$OUT/ui/$idx.xml" + textnodes=$(grep -oE 'text="[^"]+"' "$OUT/ui/$idx.xml" 2>/dev/null | wc -l | tr -d ' ') + + alive=$("$ADB" shell pidof "$PKG" >/dev/null 2>&1 && echo yes || echo no) + + { echo "=== crash buffer ==="; "$ADB" logcat -d -b crash 2>/dev/null | tail -60 + echo "=== app errors ===" + "$ADB" logcat -d 2>/dev/null \ + | grep -iE "System.err|FATAL|OdrException|Fatal signal|DEBUG.*$PKG" | tail -40 + } > "$OUT/logs/$idx.log" + + signal=ok + # A fatal only counts if it names our process. uiautomator's own launcher logs + # "D AndroidRuntime" every single iteration, and matching that alone reports + # every document in the corpus as a crash. + if grep -qiE "FATAL EXCEPTION|Fatal signal|SIGSEGV" "$OUT/logs/$idx.log" \ + && grep -qi "$PKG" "$OUT/logs/$idx.log"; then signal=CRASH; fi + # covers a native crash in the core that never reached the java handler + [ "$alive" = "no" ] && signal=CRASH-died + # These match the app's own strings in full, not a keyword. Matching "upload" or + # "password" anywhere in the dump instead flags every *document* that happens to + # contain the word - the app's own about.odt and changelog fixtures did exactly + # that, and reported themselves as failures they were not. + grep -q "Couldn't find file" "$OUT/ui/$idx.xml" && signal=notfound + grep -q "This document is password-protected" "$OUT/ui/$idx.xml" && signal=encrypted + grep -q "doesn't seem to be a supported file format" "$OUT/ui/$idx.xml" && signal=upload-offer + grep -q "Unsupported file format" "$OUT/ui/$idx.xml" && signal=unsupported + [ "$launch" != "ok" ] && signal=launch-$launch + + # settled=no means the shutter gave up before the screen stopped changing, so a + # blank or half-drawn page in that row says nothing about the document + [ "$settled" = "no" ] && [ "$signal" = "ok" ] && signal=still-rendering + + printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ + "$idx" "$repo" "$rel" "$ext" "$bytes" "$launch" "$pngbytes" "$textnodes" "$alive" \ + "$settled" "$signal" >> "$TSV" + echo "[$idx] $signal ($textnodes texts) $rel" +done 3< "$LIST" + +"$ADB" shell run-as "$PKG" sh -c "rm -f files/sweep/doc.*" >/dev/null 2>&1 + +# Downscaled copies, purely so a reviewer (or a model) can flip through them +# cheaply. Optional - the full resolution shots are the record. +if command -v sips >/dev/null 2>&1; then + mkdir -p "$OUT/small" && cp "$OUT/shots/"*.png "$OUT/small/" 2>/dev/null + sips -Z 640 "$OUT/small/"*.png >/dev/null 2>&1 +elif command -v magick >/dev/null 2>&1; then + mkdir -p "$OUT/small" + for f in "$OUT/shots/"*.png; do magick "$f" -resize x640 "$OUT/small/$(basename "$f")"; done +fi + +echo +echo "$i documents, results in $OUT" +echo "signals:" +awk -F'\t' 'NR>1 {c[$11]++} END {for (s in c) printf " %-14s %d\n", s, c[s]}' "$TSV" | sort -k2 -rn + +# A blank page is not a signal the device reports, so it is left to the reviewer: +# sort by pngbytes and textnodes and look at the small ones first. +echo +echo "least content (look at these first):" +awk -F'\t' 'NR>1 {printf " %-6s %-8s %s\n", $8" txt", $7" B", $3}' "$TSV" \ + | sort -k1 -n | head -10 From ae4bde9b25b96adb01d29c3a422977886024aa8c Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Fri, 31 Jul 2026 23:02:26 +0200 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tools/render-sweep/render-sweep.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/render-sweep/render-sweep.sh b/tools/render-sweep/render-sweep.sh index d29308de0c3e..c2db38d034ab 100755 --- a/tools/render-sweep/render-sweep.sh +++ b/tools/render-sweep/render-sweep.sh @@ -177,7 +177,8 @@ while IFS= read -r rel <&3; do { echo "=== crash buffer ==="; "$ADB" logcat -d -b crash 2>/dev/null | tail -60 echo "=== app errors ===" "$ADB" logcat -d 2>/dev/null \ - | grep -iE "System.err|FATAL|OdrException|Fatal signal|DEBUG.*$PKG" | tail -40 + | awk -v pkg="$PKG" 'BEGIN{IGNORECASE=1} /System\.err|FATAL|OdrException|Fatal signal/ || (/DEBUG/ && index($0, pkg))' \ + | tail -40 } > "$OUT/logs/$idx.log" signal=ok