Skip to content

Commit a5585aa

Browse files
google-labs-jules[bot]google-labs-jules[bot]
authored
Simplify CI screenshot scripts and fix truncated PNG error (#4209)
* Refactor CI screenshot processing to shared scripts and improve robustness against truncated PNGs. - Moved Java helper classes (ProcessScreenshots.java, etc.) from `scripts/android/tests` to `scripts/common/java`. - Updated `ProcessScreenshots.java` to include a retry mechanism for `loadPng` to handle "PNG chunk truncated before CRC" errors, which were causing flaky iOS builds. - Extracted common screenshot reporting logic into `scripts/lib/cn1ss.sh` as `cn1ss_process_and_report`. - Updated `scripts/run-android-instrumentation-tests.sh` and `scripts/run-ios-ui-tests.sh` to use the shared Java source location and the new shared reporting function, significantly reducing code duplication. - Default class names for Java helpers are now managed in `cn1ss.sh`. * Refactor CI screenshot processing to shared scripts and improve robustness against truncated PNGs. - Moved Java helper classes (ProcessScreenshots.java, etc.) from `scripts/android/tests` to `scripts/common/java`. - Updated `ProcessScreenshots.java` to include a retry mechanism for `loadPng` to handle "PNG chunk truncated before CRC" errors, which were causing flaky iOS builds. - Extracted common screenshot reporting logic into `scripts/lib/cn1ss.sh` as `cn1ss_process_and_report`. - Updated `scripts/run-android-instrumentation-tests.sh` and `scripts/run-ios-ui-tests.sh` to use the shared Java source location and the new shared reporting function, significantly reducing code duplication. - Default class names for Java helpers are now managed in `cn1ss.sh`. * Refactor CI screenshot processing to shared scripts and improve robustness against truncated PNGs. - Moved Java helper classes (ProcessScreenshots.java, etc.) from `scripts/android/tests` to `scripts/common/java`. - Updated `ProcessScreenshots.java` to include a retry mechanism for `loadPng` to handle "PNG chunk truncated before CRC" errors, which were causing flaky iOS builds. - Extracted common screenshot reporting logic into `scripts/lib/cn1ss.sh` as `cn1ss_process_and_report`. - Updated `scripts/run-android-instrumentation-tests.sh` and `scripts/run-ios-ui-tests.sh` to use the shared Java source location and the new shared reporting function, significantly reducing code duplication. - Default class names for Java helpers are now managed in `cn1ss.sh`. --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
1 parent d58d4b9 commit a5585aa

7 files changed

Lines changed: 196 additions & 147 deletions

File tree

scripts/android/tests/Cn1ssChunkTools.java renamed to scripts/common/java/Cn1ssChunkTools.java

File renamed without changes.

scripts/android/tests/PostPrComment.java renamed to scripts/common/java/PostPrComment.java

File renamed without changes.

scripts/android/tests/ProcessScreenshots.java renamed to scripts/common/java/ProcessScreenshots.java

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ public class ProcessScreenshots {
2828
private static final byte[] PNG_SIGNATURE = new byte[]{
2929
(byte) 0x89, 'P', 'N', 'G', 0x0D, 0x0A, 0x1A, 0x0A
3030
};
31+
private static final int MAX_RETRIES = 5;
32+
private static final long RETRY_DELAY_MS = 2000;
3133

3234
public static void main(String[] args) throws Exception {
3335
Arguments arguments = Arguments.parse(args);
@@ -71,8 +73,8 @@ static Map<String, Object> buildResults(
7173
}
7274
} else {
7375
try {
74-
PNGImage actual = loadPng(actualPath);
75-
PNGImage expected = loadPng(expectedPath);
76+
PNGImage actual = loadPngWithRetry(actualPath);
77+
PNGImage expected = loadPngWithRetry(expectedPath);
7678
Map<String, Object> outcome = compareImages(expected, actual);
7779
if (Boolean.TRUE.equals(outcome.get("equal"))) {
7880
record.put("status", "equal");
@@ -107,7 +109,7 @@ private static CommentPayload loadPreviewOrBuild(String testName, Path actualPat
107109
return external;
108110
}
109111
}
110-
PNGImage image = cached != null ? cached : loadPng(actualPath);
112+
PNGImage image = cached != null ? cached : loadPngWithRetry(actualPath);
111113
return buildCommentPayload(image, MAX_COMMENT_BASE64);
112114
}
113115

@@ -331,6 +333,58 @@ private static Map<String, Object> compareImages(PNGImage expected, PNGImage act
331333
return result;
332334
}
333335

336+
private static PNGImage loadPngWithRetry(Path path) throws IOException {
337+
int attempt = 0;
338+
long lastSize = -1;
339+
while (true) {
340+
try {
341+
// Stabilize check: if file size is changing, wait
342+
if (Files.exists(path)) {
343+
long size = Files.size(path);
344+
if (size != lastSize) {
345+
lastSize = size;
346+
if (attempt > 0) {
347+
// If size changed, we should wait and retry
348+
Thread.sleep(RETRY_DELAY_MS);
349+
attempt++;
350+
if (attempt >= MAX_RETRIES) {
351+
break; // fall through to try loading anyway, will likely fail
352+
}
353+
continue;
354+
}
355+
}
356+
}
357+
358+
return loadPng(path);
359+
} catch (IOException e) {
360+
// Only retry on truncated chunk or premature end of file
361+
if (e.getMessage() != null &&
362+
(e.getMessage().contains("PNG chunk truncated") ||
363+
e.getMessage().contains("Premature end of file") ||
364+
e.getMessage().contains("Missing IHDR"))) {
365+
366+
attempt++;
367+
if (attempt >= MAX_RETRIES) {
368+
throw e;
369+
}
370+
try {
371+
System.err.println("Retrying load of " + path + " (attempt " + (attempt + 1) + "/" + MAX_RETRIES + "): " + e.getMessage());
372+
Thread.sleep(RETRY_DELAY_MS);
373+
} catch (InterruptedException ie) {
374+
Thread.currentThread().interrupt();
375+
throw new IOException("Interrupted while waiting to retry load of " + path, ie);
376+
}
377+
} else {
378+
throw e;
379+
}
380+
} catch (InterruptedException e) {
381+
Thread.currentThread().interrupt();
382+
throw new IOException("Interrupted while loading " + path, e);
383+
}
384+
}
385+
return loadPng(path); // Final attempt
386+
}
387+
334388
private static PNGImage loadPng(Path path) throws IOException {
335389
byte[] data = Files.readAllBytes(path);
336390
for (int i = 0; i < PNG_SIGNATURE.length; i++) {
@@ -984,4 +1038,3 @@ private void skipWhitespace() {
9841038
}
9851039
}
9861040
}
987-

scripts/android/tests/RenderScreenshotReport.java renamed to scripts/common/java/RenderScreenshotReport.java

File renamed without changes.

‎scripts/lib/cn1ss.sh‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,18 @@ cn1ss_setup() {
2828
CN1SS_SOURCE_PATH="$2"
2929
local cache_override="${3:-}" tmp_root
3030

31+
if [ -z "$CN1SS_SOURCE_PATH" ]; then
32+
# Default to common/java if not provided or empty
33+
local script_dir
34+
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
35+
CN1SS_SOURCE_PATH="$script_dir/common/java"
36+
fi
37+
3138
if [ -z "$CN1SS_JAVA_BIN" ] || [ ! -x "$CN1SS_JAVA_BIN" ]; then
3239
cn1ss_log "CN1SS setup failed: java binary not executable ($CN1SS_JAVA_BIN)"
3340
return 1
3441
fi
35-
if [ -z "$CN1SS_SOURCE_PATH" ] || [ ! -d "$CN1SS_SOURCE_PATH" ]; then
42+
if [ ! -d "$CN1SS_SOURCE_PATH" ]; then
3643
cn1ss_log "CN1SS setup failed: source directory missing ($CN1SS_SOURCE_PATH)"
3744
return 1
3845
fi
@@ -305,3 +312,94 @@ cn1ss_post_pr_comment() {
305312
fi
306313
return $rc
307314
}
315+
316+
# Shared function to generate report, compare screenshots, and post PR comment
317+
cn1ss_process_and_report() {
318+
local platform_title="$1"
319+
local compare_json_out="$2"
320+
local summary_out="$3"
321+
local comment_out="$4"
322+
local ref_dir="$5"
323+
local preview_dir="$6"
324+
local artifacts_dir="$7"
325+
# Optional: array of actual entries in format "testName=path"
326+
shift 7
327+
local actual_entries=("$@")
328+
329+
local rc=0
330+
331+
# Run ProcessScreenshots
332+
local -a compare_args=("--reference-dir" "$ref_dir" "--emit-base64" "--preview-dir" "$preview_dir")
333+
for entry in "${actual_entries[@]}"; do
334+
compare_args+=("--actual" "$entry")
335+
done
336+
337+
cn1ss_log "STAGE:COMPARE -> Evaluating screenshots against stored references"
338+
if ! cn1ss_java_run "$CN1SS_PROCESS_CLASS" "${compare_args[@]}" > "$compare_json_out"; then
339+
cn1ss_log "FATAL: Screenshot comparison helper failed"
340+
return 13
341+
fi
342+
343+
# Run RenderScreenshotReport
344+
cn1ss_log "STAGE:COMMENT_BUILD -> Rendering summary and PR comment markdown"
345+
local -a render_args=(
346+
--title "$platform_title"
347+
--compare-json "$compare_json_out"
348+
--comment-out "$comment_out"
349+
--summary-out "$summary_out"
350+
)
351+
if [ -n "${CN1SS_COVERAGE_SUMMARY:-}" ]; then
352+
render_args+=(--coverage-summary "$CN1SS_COVERAGE_SUMMARY")
353+
fi
354+
if [ -n "${CN1SS_COVERAGE_HTML_URL:-}" ]; then
355+
render_args+=(--coverage-html-url "$CN1SS_COVERAGE_HTML_URL")
356+
fi
357+
358+
if ! cn1ss_java_run "$CN1SS_RENDER_CLASS" "${render_args[@]}"; then
359+
cn1ss_log "FATAL: Failed to render screenshot summary/comment"
360+
return 14
361+
fi
362+
363+
if [ -s "$summary_out" ]; then
364+
cn1ss_log " -> Wrote summary entries to $summary_out ($(wc -l < "$summary_out" 2>/dev/null || echo 0) line(s))"
365+
else
366+
cn1ss_log " -> No summary entries generated (all screenshots matched stored baselines)"
367+
fi
368+
369+
if [ -s "$comment_out" ]; then
370+
cn1ss_log " -> Prepared PR comment payload at $comment_out (bytes=$(wc -c < "$comment_out" 2>/dev/null || echo 0))"
371+
else
372+
cn1ss_log " -> No PR comment content produced"
373+
fi
374+
375+
# Process summary entries (copy artifacts, clean up)
376+
if [ -s "$summary_out" ]; then
377+
while IFS='|' read -r status test message copy_flag path preview_note; do
378+
[ -n "${test:-}" ] || continue
379+
cn1ss_log "Test '${test}': ${message}"
380+
if [ "$copy_flag" = "1" ] && [ -n "${path:-}" ] && [ -f "$path" ]; then
381+
cp -f "$path" "$artifacts_dir/${test}.png" 2>/dev/null || true
382+
cn1ss_log " -> Stored PNG artifact copy at $artifacts_dir/${test}.png"
383+
fi
384+
if [ "$status" = "equal" ] && [ -n "${path:-}" ]; then
385+
rm -f "$path" 2>/dev/null || true
386+
fi
387+
if [ -n "${preview_note:-}" ]; then
388+
cn1ss_log " Preview note: ${preview_note}"
389+
fi
390+
done < "$summary_out"
391+
fi
392+
393+
cp -f "$compare_json_out" "$artifacts_dir/screenshot-compare.json" 2>/dev/null || true
394+
if [ -s "$comment_out" ]; then
395+
cp -f "$comment_out" "$artifacts_dir/screenshot-comment.md" 2>/dev/null || true
396+
fi
397+
398+
cn1ss_log "STAGE:COMMENT_POST -> Submitting PR feedback"
399+
local comment_rc=0
400+
if ! cn1ss_post_pr_comment "$comment_out" "$preview_dir"; then
401+
comment_rc=$?
402+
fi
403+
404+
return $comment_rc
405+
}

‎scripts/run-android-instrumentation-tests.sh‎

Lines changed: 21 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ ra_log() { echo "[run-android-instrumentation-tests] $1"; }
99
ensure_dir() { mkdir -p "$1" 2>/dev/null || true; }
1010

1111
# CN1SS helpers are implemented in Java for easier maintenance
12-
CN1SS_MAIN_CLASS="Cn1ssChunkTools"
13-
PROCESS_SCREENSHOTS_CLASS="ProcessScreenshots"
14-
RENDER_SCREENSHOT_REPORT_CLASS="RenderScreenshotReport"
12+
# (Defaults for class names are provided by cn1ss.sh)
1513

1614
# ---- Args & environment ----------------------------------------------------
1715

@@ -25,13 +23,13 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
2523
REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
2624
cd "$REPO_ROOT"
2725

28-
CN1SS_HELPER_SOURCE_DIR="$SCRIPT_DIR/android/tests"
26+
CN1SS_HELPER_SOURCE_DIR="$SCRIPT_DIR/common/java"
27+
source "$SCRIPT_DIR/lib/cn1ss.sh"
28+
2929
if [ ! -f "$CN1SS_HELPER_SOURCE_DIR/$CN1SS_MAIN_CLASS.java" ]; then
3030
ra_log "Missing CN1SS helper: $CN1SS_HELPER_SOURCE_DIR/$CN1SS_MAIN_CLASS.java" >&2
3131
exit 3
3232
fi
33-
34-
source "$SCRIPT_DIR/lib/cn1ss.sh"
3533
cn1ss_log() { ra_log "$1"; }
3634

3735
TMPDIR="${TMPDIR:-/tmp}"; TMPDIR="${TMPDIR%/}"
@@ -220,86 +218,37 @@ done
220218

221219
# ---- Compare against stored references ------------------------------------
222220

223-
COMPARE_ARGS=()
221+
COMPARE_ENTRIES=()
224222
for test in "${TEST_NAMES[@]}"; do
225223
dest="${TEST_OUTPUTS[$test]:-}"
226224
[ -n "$dest" ] || continue
227-
COMPARE_ARGS+=("--actual" "${test}=${dest}")
225+
COMPARE_ENTRIES+=("${test}=${dest}")
228226
done
229227

230228
COMPARE_JSON="$SCREENSHOT_TMP_DIR/screenshot-compare.json"
231-
export CN1SS_PREVIEW_DIR="$SCREENSHOT_PREVIEW_DIR"
232-
ra_log "STAGE:COMPARE -> Evaluating screenshots against stored references"
233-
if ! cn1ss_java_run "$PROCESS_SCREENSHOTS_CLASS" \
234-
--reference-dir "$SCREENSHOT_REF_DIR" \
235-
--emit-base64 \
236-
--preview-dir "$SCREENSHOT_PREVIEW_DIR" \
237-
"${COMPARE_ARGS[@]}" > "$COMPARE_JSON"; then
238-
ra_log "FATAL: Screenshot comparison helper failed"
239-
exit 13
240-
fi
241-
242229
SUMMARY_FILE="$SCREENSHOT_TMP_DIR/screenshot-summary.txt"
243230
COMMENT_FILE="$SCREENSHOT_TMP_DIR/screenshot-comment.md"
244231

245-
ra_log "STAGE:COMMENT_BUILD -> Rendering summary and PR comment markdown"
246-
render_args=(
247-
--compare-json "$COMPARE_JSON"
248-
--comment-out "$COMMENT_FILE"
249-
--summary-out "$SUMMARY_FILE"
250-
--coverage-summary "$COVERAGE_SUMMARY"
251-
)
252-
if [ -n "${ANDROID_COVERAGE_HTML_URL:-}" ]; then
253-
render_args+=(--coverage-html-url "${ANDROID_COVERAGE_HTML_URL}")
254-
fi
255-
if ! cn1ss_java_run "$RENDER_SCREENSHOT_REPORT_CLASS" "${render_args[@]}"; then
256-
ra_log "FATAL: Failed to render screenshot summary/comment"
257-
exit 14
258-
fi
259-
260-
if [ -s "$SUMMARY_FILE" ]; then
261-
ra_log " -> Wrote summary entries to $SUMMARY_FILE ($(wc -l < "$SUMMARY_FILE" 2>/dev/null || echo 0) line(s))"
262-
else
263-
ra_log " -> No summary entries generated (all screenshots matched stored baselines)"
264-
fi
265-
266-
if [ -s "$COMMENT_FILE" ]; then
267-
ra_log " -> Prepared PR comment payload at $COMMENT_FILE (bytes=$(wc -c < "$COMMENT_FILE" 2>/dev/null || echo 0))"
268-
else
269-
ra_log " -> No PR comment content produced"
270-
fi
271-
272-
if [ -s "$SUMMARY_FILE" ]; then
273-
while IFS='|' read -r status test message copy_flag path preview_note; do
274-
[ -n "${test:-}" ] || continue
275-
ra_log "Test '${test}': ${message}"
276-
if [ "$copy_flag" = "1" ] && [ -n "${path:-}" ] && [ -f "$path" ]; then
277-
cp -f "$path" "$ARTIFACTS_DIR/${test}.png" 2>/dev/null || true
278-
ra_log " -> Stored PNG artifact copy at $ARTIFACTS_DIR/${test}.png"
279-
fi
280-
if [ "$status" = "equal" ] && [ -n "${path:-}" ]; then
281-
rm -f "$path" 2>/dev/null || true
282-
fi
283-
if [ -n "${preview_note:-}" ]; then
284-
ra_log " Preview note: ${preview_note}"
285-
fi
286-
done < "$SUMMARY_FILE"
287-
fi
288-
289-
cp -f "$COMPARE_JSON" "$ARTIFACTS_DIR/screenshot-compare.json" 2>/dev/null || true
290-
if [ -s "$COMMENT_FILE" ]; then
291-
cp -f "$COMMENT_FILE" "$ARTIFACTS_DIR/screenshot-comment.md" 2>/dev/null || true
292-
fi
293-
294-
ra_log "STAGE:COMMENT_POST -> Submitting PR feedback"
295-
comment_rc=0
232+
export CN1SS_PREVIEW_DIR="$SCREENSHOT_PREVIEW_DIR"
296233
export CN1SS_COMMENT_MARKER="<!-- CN1SS_ANDROID_COMMENT -->"
297234
export CN1SS_COMMENT_LOG_PREFIX="[run-android-device-tests]"
298235
export CN1SS_PREVIEW_SUBDIR="android"
299-
if ! cn1ss_post_pr_comment "$COMMENT_FILE" "$SCREENSHOT_PREVIEW_DIR"; then
300-
comment_rc=$?
236+
export CN1SS_COVERAGE_SUMMARY="$COVERAGE_SUMMARY"
237+
if [ -n "${ANDROID_COVERAGE_HTML_URL:-}" ]; then
238+
export CN1SS_COVERAGE_HTML_URL="${ANDROID_COVERAGE_HTML_URL}"
301239
fi
302240

241+
cn1ss_process_and_report \
242+
"Android screenshot updates" \
243+
"$COMPARE_JSON" \
244+
"$SUMMARY_FILE" \
245+
"$COMMENT_FILE" \
246+
"$SCREENSHOT_REF_DIR" \
247+
"$SCREENSHOT_PREVIEW_DIR" \
248+
"$ARTIFACTS_DIR" \
249+
"${COMPARE_ENTRIES[@]}"
250+
comment_rc=$?
251+
303252
# Copy useful artifacts for GH Actions
304253
cp -f "$TEST_LOG" "$ARTIFACTS_DIR/device-runner-logcat.txt" 2>/dev/null || true
305254
[ -n "${TEST_EXEC_LOG:-}" ] && cp -f "$TEST_EXEC_LOG" "$ARTIFACTS_DIR/test-results.log" 2>/dev/null || true

0 commit comments

Comments
 (0)