From 69fab2ef8886eb6e739b388de431804b7fe98ebe Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 5 Sep 2026 10:13:57 +0200 Subject: [PATCH 1/3] Hand the screenshot run its locales in something gradle keeps AGP 9.4.0 cuts a `-Pandroid.testInstrumentationRunnerArguments.` value at the first comma. The lane joined the fifteen locales with one, so the runner was handed `-e locales cs-CZ` and each device photographed a single language: 9.3.2 args_map { key: "locales" value: "de-DE,en-US" } 9.4.0 am instrument ... -e locales de-DE It is AGP's bug rather than ours - the same truncation drops every class but the first from the documented `class=TestA,TestB` - so the lane joins with a plus, which arrives whole, and the test splits on either. A run driving `am instrument` by hand is unaffected and can still spell it with a comma. What made this cost a release is that it passed. `ScreenshotTests` photographs the locales it is handed, so one language is a full run of a short list, and `screenshots_narrowed?` skips the full-set check on a release runner holding half a set. The lane now checks that every language it asked for came out, which holds for half a set as well. Found on the 4.19.0 release: both devices reported success with cs-CZ alone, and `screenshot-set` refused the other fourteen. The store kept 4.18.0's pictures, which is what that job is split out to make survivable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017zSYiEKUuTFhMy3jpee99C --- .../droid/test/ScreenshotTests.kt | 10 +++++++-- fastlane/Fastfile | 22 ++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt index 2cadd02cb36d..015a0b7d9292 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt @@ -599,7 +599,13 @@ class ScreenshotTests { // --- what the run was asked for ----------------------------------------- - /** The locales to photograph: every one the listing is written in, unless fewer were named. */ + /** + * The locales to photograph: every one the listing is written in, unless fewer were named. + * + * Either separator, because gradle cannot be trusted with a comma: AGP 9.4.0 cuts a + * `-Pandroid.testInstrumentationRunnerArguments.=a,b` at the first one, so the lane hands + * this a `+` and only a run driving `am instrument` itself still spells it with a comma. + */ private fun locales(spoken: JSONObject): List { val known = spoken.keys().asSequence().sorted().toList() @@ -608,7 +614,7 @@ class ScreenshotTests { return known } - val wanted = given.split(",").map { it.trim() }.filter { it.isNotEmpty() } + val wanted = given.split(',', '+').map { it.trim() }.filter { it.isNotEmpty() } val unknown = wanted.filterNot { it in known } Assert.assertTrue( "no such locale: ${unknown.joinToString()}. One of ${known.joinToString()}", diff --git a/fastlane/Fastfile b/fastlane/Fastfile index 28204ca10a56..2f3d9f7b08fa 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -330,7 +330,13 @@ platform :android do "android.testInstrumentationRunnerArguments.class" => "app.opendocument.droid.test.ScreenshotTests", "android.testInstrumentationRunnerArguments.device" => device, - "android.testInstrumentationRunnerArguments.locales" => languages.join(",") + # joined with a plus rather than a comma: AGP 9.4.0 cuts one of these + # properties at the first comma, so `cs-CZ,de-DE,...` reached the runner + # as `cs-CZ` and 4.19.0 went to the store with one locale photographed. + # The same bug drops every class but the first from the documented + # `class=TestA,TestB`, so it is AGP's rather than ours - and the check + # below is here because that run still passed. + "android.testInstrumentationRunnerArguments.locales" => languages.join("+") } ) @@ -342,6 +348,20 @@ platform :android do ) end + # Every language this run asked for, or the run did less than it was told and + # said nothing: the test photographs the locales it was handed and passes on + # whatever that turned out to be, so a list mangled on the way in - which is + # what the plus above is for - looks exactly like a run that was asked for + # one language. Checked per language rather than as a full set, so it holds + # for the release's half a set too. + missing = languages - taken.map { |path| File.basename(File.dirname(path)) }.uniq + unless missing.empty? + UI.user_error!( + "the run photographed #{missing.length} of #{languages.length} languages: " \ + "nothing came out for #{missing.join(', ')}" + ) + end + FileUtils.mkdir_p(SCREENSHOT_DIR) taken.each do |path| locale = File.basename(File.dirname(path)) From bd08dd9e0c31bae85ff0a4cf6e28fbb313d3226a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 5 Sep 2026 10:14:55 +0200 Subject: [PATCH 2/3] Spell the by-hand invocation the way that works too Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017zSYiEKUuTFhMy3jpee99C --- .../java/app/opendocument/droid/test/ScreenshotTests.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt index 015a0b7d9292..803e6ce540c4 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt @@ -86,9 +86,11 @@ import org.junit.runner.RunWith * ./gradlew connectedProDebugAndroidTest \ * -Pandroid.testInstrumentationRunnerArguments.class=app.opendocument.droid.test.ScreenshotTests \ * -Pandroid.testInstrumentationRunnerArguments.device=phone \ - * -Pandroid.testInstrumentationRunnerArguments.locales=en-US,de-DE + * -Pandroid.testInstrumentationRunnerArguments.locales=en-US+de-DE * ``` * + * A plus rather than a comma, for the reason [locales] gives. + * * Wants android 15 or newer, and says so rather than photographing what an older one draws. */ @LargeTest From 36cd51d20cc0b720a3009b7247c6aa9ed019bc04 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 5 Sep 2026 10:24:34 +0200 Subject: [PATCH 3/3] Trim the comments to what the code cannot say Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017zSYiEKUuTFhMy3jpee99C --- .../opendocument/droid/test/ScreenshotTests.kt | 6 +----- fastlane/Fastfile | 15 +++------------ 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt b/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt index 803e6ce540c4..7b70c9d54f41 100644 --- a/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt +++ b/app/src/androidTest/java/app/opendocument/droid/test/ScreenshotTests.kt @@ -89,8 +89,6 @@ import org.junit.runner.RunWith * -Pandroid.testInstrumentationRunnerArguments.locales=en-US+de-DE * ``` * - * A plus rather than a comma, for the reason [locales] gives. - * * Wants android 15 or newer, and says so rather than photographing what an older one draws. */ @LargeTest @@ -604,9 +602,7 @@ class ScreenshotTests { /** * The locales to photograph: every one the listing is written in, unless fewer were named. * - * Either separator, because gradle cannot be trusted with a comma: AGP 9.4.0 cuts a - * `-Pandroid.testInstrumentationRunnerArguments.=a,b` at the first one, so the lane hands - * this a `+` and only a run driving `am instrument` itself still spells it with a comma. + * A plus separates them as well as a comma, which AGP 9.4.0 cuts such a property at. */ private fun locales(spoken: JSONObject): List { val known = spoken.keys().asSequence().sorted().toList() diff --git a/fastlane/Fastfile b/fastlane/Fastfile index 2f3d9f7b08fa..0efd4f19f6fe 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -330,12 +330,7 @@ platform :android do "android.testInstrumentationRunnerArguments.class" => "app.opendocument.droid.test.ScreenshotTests", "android.testInstrumentationRunnerArguments.device" => device, - # joined with a plus rather than a comma: AGP 9.4.0 cuts one of these - # properties at the first comma, so `cs-CZ,de-DE,...` reached the runner - # as `cs-CZ` and 4.19.0 went to the store with one locale photographed. - # The same bug drops every class but the first from the documented - # `class=TestA,TestB`, so it is AGP's rather than ours - and the check - # below is here because that run still passed. + # a plus, not a comma: AGP 9.4.0 cuts one of these at the first comma "android.testInstrumentationRunnerArguments.locales" => languages.join("+") } ) @@ -348,12 +343,8 @@ platform :android do ) end - # Every language this run asked for, or the run did less than it was told and - # said nothing: the test photographs the locales it was handed and passes on - # whatever that turned out to be, so a list mangled on the way in - which is - # what the plus above is for - looks exactly like a run that was asked for - # one language. Checked per language rather than as a full set, so it holds - # for the release's half a set too. + # a short list photographs as a complete run, so the count is what catches it. + # Per language rather than as a set, which a release runner does not have. missing = languages - taken.map { |path| File.basename(File.dirname(path)) }.uniq unless missing.empty? UI.user_error!(