From 896c0e0ccdddc9d36df1e9f21af4aef937bd4ab7 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 15 Oct 2025 04:29:52 +0300 Subject: [PATCH 1/3] Add BrowserComponent instrumentation screenshot regression test --- scripts/android/screenshots/README.md | 13 + .../HelloCodenameOneInstrumentedTest.java | 214 +++++++--- scripts/android/tests/cn1ss_chunk_tools.py | 60 ++- scripts/android/tests/process_screenshots.py | 240 +++++++++++ scripts/run-android-instrumentation-tests.sh | 384 +++++++++++++++--- 5 files changed, 776 insertions(+), 135 deletions(-) create mode 100644 scripts/android/screenshots/README.md create mode 100644 scripts/android/tests/process_screenshots.py diff --git a/scripts/android/screenshots/README.md b/scripts/android/screenshots/README.md new file mode 100644 index 0000000000..84d922730f --- /dev/null +++ b/scripts/android/screenshots/README.md @@ -0,0 +1,13 @@ +# Android Instrumentation Test Screenshots + +This directory stores reference screenshots for Android native instrumentation tests. + +Each PNG file should be named after the test stream that emits the screenshot +(e.g. `MainActivity.png` or `BrowserComponent.png`). The automation in +`scripts/run-android-instrumentation-tests.sh` compares the screenshots emitted +by the emulator with the files stored here. If the pixels differ (ignoring PNG +metadata) or if a reference image is missing, the workflow posts a pull request +comment that includes the updated screenshot. + +When the comparison passes, no screenshot artifacts are published and no +comment is created. diff --git a/scripts/android/tests/HelloCodenameOneInstrumentedTest.java b/scripts/android/tests/HelloCodenameOneInstrumentedTest.java index 92de85de8c..d684b85b5e 100644 --- a/scripts/android/tests/HelloCodenameOneInstrumentedTest.java +++ b/scripts/android/tests/HelloCodenameOneInstrumentedTest.java @@ -13,25 +13,42 @@ import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.codename1.ui.BrowserComponent; +import com.codename1.ui.Display; +import com.codename1.ui.Form; +import com.codename1.ui.layouts.BorderLayout; + import org.junit.Assert; +import org.junit.Assume; import org.junit.Test; import org.junit.runner.RunWith; import java.io.ByteArrayOutputStream; +import java.util.Locale; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; @RunWith(AndroidJUnit4.class) public class HelloCodenameOneInstrumentedTest { + private static final int CHUNK_SIZE = 2000; + private static final String MAIN_SCREEN_TEST = "MainActivity"; + private static final String BROWSER_TEST = "BrowserComponent"; + private static void println(String s) { System.out.println(s); } - @Test - public void testUseAppContext_andEmitScreenshot() throws Exception { - Context ctx = ApplicationProvider.getApplicationContext(); - String pkg = "@PACKAGE@"; - Assert.assertEquals("Package mismatch", pkg, ctx.getPackageName()); + private static void settle(long millis) { + try { + Thread.sleep(millis); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } + } + private static ActivityScenario launchMainActivity(Context ctx) { + String pkg = "@PACKAGE@"; Intent launch = ctx.getPackageManager().getLaunchIntentForPackage(pkg); if (launch == null) { Intent q = new Intent(Intent.ACTION_MAIN); @@ -40,73 +57,148 @@ public void testUseAppContext_andEmitScreenshot() throws Exception { launch = q; } launch.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); + println("CN1SS:INFO: launching activity for test"); + return ActivityScenario.launch(launch); + } - println("CN1SS:INFO: about to launch Activity"); - byte[] pngBytes = null; - - try (ActivityScenario scenario = ActivityScenario.launch(launch)) { - Thread.sleep(750); - - println("CN1SS:INFO: activity launched"); - - final byte[][] holder = new byte[1][]; - scenario.onActivity(activity -> { - try { - View root = activity.getWindow().getDecorView().getRootView(); - int w = root.getWidth(); - int h = root.getHeight(); - if (w <= 0 || h <= 0) { - DisplayMetrics dm = activity.getResources().getDisplayMetrics(); - w = Math.max(1, dm.widthPixels); - h = Math.max(1, dm.heightPixels); - int sw = View.MeasureSpec.makeMeasureSpec(w, View.MeasureSpec.EXACTLY); - int sh = View.MeasureSpec.makeMeasureSpec(h, View.MeasureSpec.EXACTLY); - root.measure(sw, sh); - root.layout(0, 0, w, h); - println("CN1SS:INFO: forced layout to " + w + "x" + h); - } else { - println("CN1SS:INFO: natural layout " + w + "x" + h); - } - - Bitmap bmp = Bitmap.createBitmap(w, h, Bitmap.Config.ARGB_8888); - Canvas c = new Canvas(bmp); - root.draw(c); - - ByteArrayOutputStream baos = new ByteArrayOutputStream(Math.max(1024, w * h / 2)); - boolean ok = bmp.compress(Bitmap.CompressFormat.PNG, 100, baos); - if (!ok) { - throw new RuntimeException("Bitmap.compress returned false"); - } - holder[0] = baos.toByteArray(); - println("CN1SS:INFO: png_bytes=" + holder[0].length); - } catch (Throwable t) { - println("CN1SS:ERR: onActivity " + t); - t.printStackTrace(System.out); + private static byte[] captureScreenshot(ActivityScenario scenario, String testName) { + final byte[][] holder = new byte[1][]; + scenario.onActivity(activity -> { + try { + View root = activity.getWindow().getDecorView().getRootView(); + int w = root.getWidth(); + int h = root.getHeight(); + if (w <= 0 || h <= 0) { + DisplayMetrics dm = activity.getResources().getDisplayMetrics(); + w = Math.max(1, dm.widthPixels); + h = Math.max(1, dm.heightPixels); + int sw = View.MeasureSpec.makeMeasureSpec(w, View.MeasureSpec.EXACTLY); + int sh = View.MeasureSpec.makeMeasureSpec(h, View.MeasureSpec.EXACTLY); + root.measure(sw, sh); + root.layout(0, 0, w, h); + println("CN1SS:INFO:test=" + testName + " forced layout to " + w + "x" + h); + } else { + println("CN1SS:INFO:test=" + testName + " natural layout " + w + "x" + h); } - }); - pngBytes = holder[0]; - } catch (Throwable t) { - println("CN1SS:ERR: launch " + t); - t.printStackTrace(System.out); - } + Bitmap bmp = Bitmap.createBitmap(w, h, Bitmap.Config.ARGB_8888); + Canvas c = new Canvas(bmp); + root.draw(c); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(Math.max(1024, w * h / 2)); + if (!bmp.compress(Bitmap.CompressFormat.PNG, 100, baos)) { + throw new RuntimeException("Bitmap.compress returned false"); + } + holder[0] = baos.toByteArray(); + println("CN1SS:INFO:test=" + testName + " png_bytes=" + holder[0].length); + } catch (Throwable t) { + println("CN1SS:ERR:test=" + testName + " " + t); + t.printStackTrace(System.out); + } + }); + return holder[0]; + } + + private static String sanitizeTestName(String testName) { + return testName.replaceAll("[^A-Za-z0-9_.-]", "_"); + } + private static void emitScreenshot(byte[] pngBytes, String testName) { + String safeName = sanitizeTestName(testName); if (pngBytes == null || pngBytes.length == 0) { - println("CN1SS:END"); - Assert.fail("Screenshot capture produced 0 bytes"); + println("CN1SS:END:" + safeName); + Assert.fail("Screenshot capture produced 0 bytes for " + testName); return; } - String b64 = Base64.encodeToString(pngBytes, Base64.NO_WRAP); - final int chunkSize = 2000; int count = 0; - for (int pos = 0; pos < b64.length(); pos += chunkSize) { - int end = Math.min(pos + chunkSize, b64.length()); - System.out.println("CN1SS:" + String.format("%06d", pos) + ":" + b64.substring(pos, end)); + for (int pos = 0; pos < b64.length(); pos += CHUNK_SIZE) { + int end = Math.min(pos + CHUNK_SIZE, b64.length()); + String chunk = b64.substring(pos, end); + System.out.println("CN1SS:" + safeName + ":" + String.format(Locale.US, "%06d", pos) + ":" + chunk); count++; } - println("CN1SS:INFO: chunks=" + count + " total_b64_len=" + b64.length()); - System.out.println("CN1SS:END"); + println("CN1SS:INFO:test=" + safeName + " chunks=" + count + " total_b64_len=" + b64.length()); + System.out.println("CN1SS:END:" + safeName); System.out.flush(); } + + private static void prepareBrowserComponentContent(ActivityScenario scenario) throws InterruptedException { + final CountDownLatch supportLatch = new CountDownLatch(1); + final boolean[] supported = new boolean[1]; + + scenario.onActivity(activity -> Display.getInstance().callSerially(() -> { + try { + supported[0] = BrowserComponent.isNativeBrowserSupported(); + } finally { + supportLatch.countDown(); + } + })); + + if (!supportLatch.await(5, TimeUnit.SECONDS)) { + Assert.fail("Timed out while verifying BrowserComponent support"); + } + + Assume.assumeTrue("BrowserComponent native support required for this test", supported[0]); + + final CountDownLatch loadLatch = new CountDownLatch(1); + final String html = "" + + "" + + "

Codename One

" + + "

BrowserComponent instrumentation test content.

"; + + scenario.onActivity(activity -> Display.getInstance().callSerially(() -> { + Form current = Display.getInstance().getCurrent(); + if (current == null) { + current = new Form("Browser Test", new BorderLayout()); + current.show(); + } else { + current.setLayout(new BorderLayout()); + current.setTitle("Browser Test"); + current.removeAll(); + } + + BrowserComponent browser = new BrowserComponent(); + browser.addWebEventListener(BrowserComponent.onLoad, evt -> loadLatch.countDown()); + browser.setPage(html, null); + current.add(BorderLayout.CENTER, browser); + current.revalidate(); + })); + + if (!loadLatch.await(10, TimeUnit.SECONDS)) { + Assert.fail("Timed out waiting for BrowserComponent to load content"); + } + } + + @Test + public void testUseAppContext_andEmitScreenshot() throws Exception { + Context ctx = ApplicationProvider.getApplicationContext(); + String pkg = "@PACKAGE@"; + Assert.assertEquals("Package mismatch", pkg, ctx.getPackageName()); + + byte[] pngBytes; + try (ActivityScenario scenario = launchMainActivity(ctx)) { + settle(750); + pngBytes = captureScreenshot(scenario, MAIN_SCREEN_TEST); + } + + emitScreenshot(pngBytes, MAIN_SCREEN_TEST); + } + + @Test + public void testBrowserComponentScreenshot() throws Exception { + Context ctx = ApplicationProvider.getApplicationContext(); + byte[] pngBytes; + + try (ActivityScenario scenario = launchMainActivity(ctx)) { + settle(750); + prepareBrowserComponentContent(scenario); + settle(500); + pngBytes = captureScreenshot(scenario, BROWSER_TEST); + } + + emitScreenshot(pngBytes, BROWSER_TEST); + } } diff --git a/scripts/android/tests/cn1ss_chunk_tools.py b/scripts/android/tests/cn1ss_chunk_tools.py index 026bae0192..f40b2dfa65 100755 --- a/scripts/android/tests/cn1ss_chunk_tools.py +++ b/scripts/android/tests/cn1ss_chunk_tools.py @@ -1,40 +1,40 @@ -#!/usr/bin/env python3 -"""Helpers for extracting CN1SS chunked screenshot payloads.""" -from __future__ import annotations - import argparse import base64 import pathlib import re import sys -from typing import Iterable, List, Tuple +from typing import Iterable, List, Optional, Tuple -CHUNK_PATTERN = re.compile(r"CN1SS:(\d{6}):(.*)") +DEFAULT_TEST_NAME = "default" +CHUNK_PATTERN = re.compile(r"CN1SS:(?:(?P[A-Za-z0-9_.-]+):)?(?P\d{6}):(.*)") -def _iter_chunk_lines(path: pathlib.Path) -> Iterable[Tuple[int, str]]: +def _iter_chunk_lines(path: pathlib.Path, test_filter: Optional[str] = None) -> Iterable[Tuple[str, int, str]]: text = path.read_text(encoding="utf-8", errors="ignore") for line in text.splitlines(): match = CHUNK_PATTERN.search(line) if not match: continue - index = int(match.group(1)) - payload = re.sub(r"[^A-Za-z0-9+/=]", "", match.group(2)) + test_name = match.group("test") or DEFAULT_TEST_NAME + if test_filter is not None and test_name != test_filter: + continue + index = int(match.group("index")) + payload = re.sub(r"[^A-Za-z0-9+/=]", "", match.group(3)) if payload: - yield index, payload + yield test_name, index, payload -def count_chunks(path: pathlib.Path) -> int: - return sum(1 for _ in _iter_chunk_lines(path)) +def count_chunks(path: pathlib.Path, test: Optional[str] = None) -> int: + return sum(1 for _ in _iter_chunk_lines(path, test_filter=test)) -def concatenate_chunks(path: pathlib.Path) -> str: - ordered = sorted(_iter_chunk_lines(path), key=lambda item: item[0]) - return "".join(payload for _, payload in ordered) +def concatenate_chunks(path: pathlib.Path, test: Optional[str] = None) -> str: + ordered = sorted(_iter_chunk_lines(path, test_filter=test), key=lambda item: item[1]) + return "".join(payload for _, _, payload in ordered) -def decode_chunks(path: pathlib.Path) -> bytes: - data = concatenate_chunks(path) +def decode_chunks(path: pathlib.Path, test: Optional[str] = None) -> bytes: + data = concatenate_chunks(path, test=test) if not data: return b"" try: @@ -43,28 +43,48 @@ def decode_chunks(path: pathlib.Path) -> bytes: return b"" +def list_tests(path: pathlib.Path) -> List[str]: + seen = {test for test, _, _ in _iter_chunk_lines(path)} + return sorted(seen) + + def main(argv: List[str] | None = None) -> int: parser = argparse.ArgumentParser(description=__doc__) subparsers = parser.add_subparsers(dest="command", required=True) p_count = subparsers.add_parser("count", help="Count CN1SS chunks in a file") p_count.add_argument("path", type=pathlib.Path) + p_count.add_argument("--test", dest="test", default=None, help="Optional test name filter") p_extract = subparsers.add_parser("extract", help="Concatenate CN1SS payload chunks") p_extract.add_argument("path", type=pathlib.Path) p_extract.add_argument("--decode", action="store_true", help="Decode payload to binary PNG") + p_extract.add_argument("--test", dest="test", default=None, help="Test name to extract (default=unnamed)") + + p_tests = subparsers.add_parser("tests", help="List distinct test names found in CN1SS chunks") + p_tests.add_argument("path", type=pathlib.Path) args = parser.parse_args(argv) if args.command == "count": - print(count_chunks(args.path)) + print(count_chunks(args.path, args.test)) return 0 if args.command == "extract": + target_test: Optional[str] + if args.test is None: + target_test = DEFAULT_TEST_NAME + else: + target_test = args.test if args.decode: - sys.stdout.buffer.write(decode_chunks(args.path)) + sys.stdout.buffer.write(decode_chunks(args.path, target_test)) else: - sys.stdout.write(concatenate_chunks(args.path)) + sys.stdout.write(concatenate_chunks(args.path, target_test)) + return 0 + + if args.command == "tests": + for name in list_tests(args.path): + print(name) return 0 return 1 diff --git a/scripts/android/tests/process_screenshots.py b/scripts/android/tests/process_screenshots.py new file mode 100644 index 0000000000..fac8877443 --- /dev/null +++ b/scripts/android/tests/process_screenshots.py @@ -0,0 +1,240 @@ +#!/usr/bin/env python3 +"""Compare CN1 screenshot outputs against stored references.""" + +from __future__ import annotations + +import argparse +import base64 +import json +import pathlib +import sys +import zlib +from dataclasses import dataclass +from typing import Dict, List, Tuple + +PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n" + + +class PNGError(Exception): + """Raised when a PNG cannot be parsed.""" + + +@dataclass +class PNGImage: + width: int + height: int + bit_depth: int + color_type: int + pixels: bytes + bytes_per_pixel: int + + +def _read_chunks(path: pathlib.Path) -> Iterable[Tuple[bytes, bytes]]: + data = path.read_bytes() + if not data.startswith(PNG_SIGNATURE): + raise PNGError(f"{path} is not a PNG file (missing signature)") + offset = len(PNG_SIGNATURE) + length = len(data) + while offset + 8 <= length: + chunk_len = int.from_bytes(data[offset : offset + 4], "big") + chunk_type = data[offset + 4 : offset + 8] + offset += 8 + if offset + chunk_len + 4 > length: + raise PNGError("PNG chunk truncated before CRC") + chunk_data = data[offset : offset + chunk_len] + offset += chunk_len + 4 # skip data + CRC + yield chunk_type, chunk_data + if chunk_type == b"IEND": + break + + +def _bytes_per_pixel(bit_depth: int, color_type: int) -> int: + if bit_depth != 8: + raise PNGError(f"Unsupported bit depth: {bit_depth}") + if color_type == 0: # greyscale + return 1 + if color_type == 2: # RGB + return 3 + if color_type == 4: # greyscale + alpha + return 2 + if color_type == 6: # RGBA + return 4 + raise PNGError(f"Unsupported color type: {color_type}") + + +def _paeth_predict(a: int, b: int, c: int) -> int: + p = a + b - c + pa = abs(p - a) + pb = abs(p - b) + pc = abs(p - c) + if pa <= pb and pa <= pc: + return a + if pb <= pc: + return b + return c + + +def _unfilter(width: int, height: int, bpp: int, raw: bytes) -> bytes: + stride = width * bpp + expected = height * (stride + 1) + if len(raw) != expected: + raise PNGError("PNG IDAT payload has unexpected length") + result = bytearray(height * stride) + in_offset = 0 + out_offset = 0 + for row in range(height): + filter_type = raw[in_offset] + in_offset += 1 + row_data = bytearray(raw[in_offset : in_offset + stride]) + in_offset += stride + if filter_type == 0: # None + pass + elif filter_type == 1: # Sub + for i in range(stride): + left = row_data[i - bpp] if i >= bpp else 0 + row_data[i] = (row_data[i] + left) & 0xFF + elif filter_type == 2: # Up + for i in range(stride): + up = result[out_offset - stride + i] if row > 0 else 0 + row_data[i] = (row_data[i] + up) & 0xFF + elif filter_type == 3: # Average + for i in range(stride): + left = row_data[i - bpp] if i >= bpp else 0 + up = result[out_offset - stride + i] if row > 0 else 0 + row_data[i] = (row_data[i] + ((left + up) // 2)) & 0xFF + elif filter_type == 4: # Paeth + for i in range(stride): + left = row_data[i - bpp] if i >= bpp else 0 + up = result[out_offset - stride + i] if row > 0 else 0 + up_left = result[out_offset - stride + i - bpp] if (row > 0 and i >= bpp) else 0 + row_data[i] = (row_data[i] + _paeth_predict(left, up, up_left)) & 0xFF + else: + raise PNGError(f"Unsupported PNG filter type: {filter_type}") + result[out_offset : out_offset + stride] = row_data + out_offset += stride + return bytes(result) + + +def load_png(path: pathlib.Path) -> PNGImage: + ihdr = None + idat_chunks: List[bytes] = [] + for chunk_type, chunk_data in _read_chunks(path): + if chunk_type == b"IHDR": + if ihdr is not None: + raise PNGError("Duplicate IHDR chunk") + if len(chunk_data) != 13: + raise PNGError("Invalid IHDR length") + width = int.from_bytes(chunk_data[0:4], "big") + height = int.from_bytes(chunk_data[4:8], "big") + bit_depth = chunk_data[8] + color_type = chunk_data[9] + # compression (10), filter (11), interlace (12) must be default values + if chunk_data[10] != 0 or chunk_data[11] != 0: + raise PNGError("Unsupported PNG compression or filter method") + if chunk_data[12] not in (0, 1): + raise PNGError("Unsupported PNG interlace method") + ihdr = (width, height, bit_depth, color_type, chunk_data[12]) + elif chunk_type == b"IDAT": + idat_chunks.append(chunk_data) + elif chunk_type == b"IEND": + break + else: + # Ancillary chunks are ignored (metadata) + continue + + if ihdr is None: + raise PNGError("Missing IHDR chunk") + if not idat_chunks: + raise PNGError("Missing IDAT data") + + width, height, bit_depth, color_type, interlace = ihdr + if interlace != 0: + raise PNGError("Interlaced PNGs are not supported") + + bpp = _bytes_per_pixel(bit_depth, color_type) + compressed = b"".join(idat_chunks) + try: + raw = zlib.decompress(compressed) + except Exception as exc: # pragma: no cover - defensive + raise PNGError(f"Failed to decompress IDAT data: {exc}") from exc + + pixels = _unfilter(width, height, bpp, raw) + return PNGImage(width, height, bit_depth, color_type, pixels, bpp) + + +def compare_images(expected_path: pathlib.Path, actual_path: pathlib.Path) -> Dict[str, bool]: + expected = load_png(expected_path) + actual = load_png(actual_path) + equal = ( + expected.width == actual.width + and expected.height == actual.height + and expected.bit_depth == actual.bit_depth + and expected.color_type == actual.color_type + and expected.pixels == actual.pixels + ) + return { + "equal": equal, + "width": actual.width, + "height": actual.height, + "bit_depth": actual.bit_depth, + "color_type": actual.color_type, + } + + +def build_results(reference_dir: pathlib.Path, actual_entries: List[Tuple[str, pathlib.Path]], emit_base64: bool) -> Dict[str, List[Dict[str, object]]]: + results: List[Dict[str, object]] = [] + for test_name, actual_path in actual_entries: + expected_path = reference_dir / f"{test_name}.png" + record: Dict[str, object] = { + "test": test_name, + "actual_path": str(actual_path), + "expected_path": str(expected_path), + } + if not actual_path.exists(): + record.update({"status": "missing_actual", "message": "Actual screenshot not found"}) + elif not expected_path.exists(): + record.update({"status": "missing_expected"}) + if emit_base64: + record["base64"] = base64.b64encode(actual_path.read_bytes()).decode("ascii") + else: + try: + outcome = compare_images(expected_path, actual_path) + except Exception as exc: + record.update({"status": "error", "message": str(exc)}) + else: + if outcome["equal"]: + record.update({"status": "equal"}) + else: + record.update({"status": "different", "details": outcome}) + if emit_base64: + record["base64"] = base64.b64encode(actual_path.read_bytes()).decode("ascii") + results.append(record) + return {"results": results} + + +def parse_args(argv: List[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--reference-dir", required=True, type=pathlib.Path) + parser.add_argument("--emit-base64", action="store_true", help="Include base64 payloads for updated screenshots") + parser.add_argument("--actual", action="append", default=[], help="Mapping of test=path to evaluate") + return parser.parse_args(argv) + + +def main(argv: List[str] | None = None) -> int: + args = parse_args(argv) + reference_dir: pathlib.Path = args.reference_dir + actual_entries: List[Tuple[str, pathlib.Path]] = [] + for item in args.actual: + if "=" not in item: + print(f"Invalid --actual value: {item}", file=sys.stderr) + return 2 + name, path_str = item.split("=", 1) + actual_entries.append((name, pathlib.Path(path_str))) + + payload = build_results(reference_dir, actual_entries, bool(args.emit_base64)) + json.dump(payload, sys.stdout) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/run-android-instrumentation-tests.sh b/scripts/run-android-instrumentation-tests.sh index 1bac3c92cc..7c2beacd63 100755 --- a/scripts/run-android-instrumentation-tests.sh +++ b/scripts/run-android-instrumentation-tests.sh @@ -13,6 +13,7 @@ CN1SS_TOOL="" count_chunks() { local f="${1:-}" + local test="${2:-}" if [ -z "$CN1SS_TOOL" ] || [ ! -x "$CN1SS_TOOL" ]; then echo 0 return @@ -21,29 +22,168 @@ count_chunks() { echo 0 return fi - python3 "$CN1SS_TOOL" count "$f" 2>/dev/null || echo 0 + local args=("count" "$f") + if [ -n "$test" ]; then + args+=("--test" "$test") + fi + python3 "$CN1SS_TOOL" "${args[@]}" 2>/dev/null || echo 0 } extract_cn1ss_base64() { local f="${1:-}" + local test="${2:-}" if [ -z "$CN1SS_TOOL" ] || [ ! -x "$CN1SS_TOOL" ]; then return 1 fi if [ -z "$f" ] || [ ! -r "$f" ]; then return 1 fi - python3 "$CN1SS_TOOL" extract "$f" + local args=("extract" "$f") + if [ -n "$test" ]; then + args+=("--test" "$test") + fi + python3 "$CN1SS_TOOL" "${args[@]}" } decode_cn1ss_png() { local f="${1:-}" + local test="${2:-}" if [ -z "$CN1SS_TOOL" ] || [ ! -x "$CN1SS_TOOL" ]; then return 1 fi if [ -z "$f" ] || [ ! -r "$f" ]; then return 1 fi - python3 "$CN1SS_TOOL" extract "$f" --decode + local args=("extract" "$f" "--decode") + if [ -n "$test" ]; then + args+=("--test" "$test") + fi + python3 "$CN1SS_TOOL" "${args[@]}" +} + +list_cn1ss_tests() { + local f="${1:-}" + if [ -z "$CN1SS_TOOL" ] || [ ! -x "$CN1SS_TOOL" ]; then + return 1 + fi + if [ -z "$f" ] || [ ! -r "$f" ]; then + return 1 + fi + python3 "$CN1SS_TOOL" tests "$f" +} + +post_pr_comment() { + local body_file="${1:-}" + if [ -z "$body_file" ] || [ ! -s "$body_file" ]; then + return 0 + fi + if [ -z "${GITHUB_TOKEN:-}" ]; then + ra_log "PR comment skipped (GITHUB_TOKEN not set)" + return 0 + fi + if [ -z "${GITHUB_EVENT_PATH:-}" ] || [ ! -f "$GITHUB_EVENT_PATH" ]; then + ra_log "PR comment skipped (GITHUB_EVENT_PATH unavailable)" + return 0 + fi + python3 - "$body_file" <<'PY' +import json +import os +import pathlib +import sys +from urllib.error import HTTPError +from urllib.request import Request, urlopen + +body_path = pathlib.Path(sys.argv[1]) +body = body_path.read_text(encoding="utf-8").strip() +if not body: + sys.exit(0) + +event_path = os.environ.get("GITHUB_EVENT_PATH") +repo = os.environ.get("GITHUB_REPOSITORY") +token = os.environ.get("GITHUB_TOKEN") +if not event_path or not repo or not token: + sys.exit(0) + +with open(event_path, "r", encoding="utf-8") as fh: + event = json.load(fh) + +pr_number = None +if "pull_request" in event: + pr_number = event["pull_request"].get("number") +elif event.get("issue") and event["issue"].get("pull_request"): + pr_number = event["issue"].get("number") + +if not pr_number: + sys.exit(0) + +payload = json.dumps({"body": body}).encode("utf-8") +req = Request( + f"https://api.github.com/repos/{repo}/issues/{pr_number}/comments", + data=payload, + headers={ + "Authorization": f"token {token}", + "Accept": "application/vnd.github+json", + "Content-Type": "application/json", + }, + method="POST", +) + +try: + with urlopen(req) as resp: + resp.read() +except HTTPError as exc: # pragma: no cover - diagnostics only + print(f"Failed to post PR comment: {exc}", file=sys.stderr) + sys.exit(0) +PY + if [ $? -eq 0 ]; then + ra_log "Posted screenshot comparison comment to PR" + fi + return 0 +} + +decode_test_png() { + local test_name="${1:-}" + local dest="${2:-}" + local source="" + local count="0" + + if [ "${#XMLS[@]}" -gt 0 ]; then + for x in "${XMLS[@]}"; do + count="$(count_chunks "$x" "$test_name")"; count="${count//[^0-9]/}"; : "${count:=0}" + [ "$count" -gt 0 ] || continue + ra_log "Reassembling test '$test_name' from XML: $x (chunks=$count)" + if decode_cn1ss_png "$x" "$test_name" > "$dest" 2>/dev/null; then + if verify_png "$dest"; then source="XML"; break; fi + fi + done + fi + + if [ -z "$source" ] && [ -s "${LOGCAT_FILE:-}" ]; then + count="$(count_chunks "$LOGCAT_FILE" "$test_name")"; count="${count//[^0-9]/}"; : "${count:=0}" + if [ "$count" -gt 0 ]; then + ra_log "Reassembling test '$test_name' from logcat: $LOGCAT_FILE (chunks=$count)" + if decode_cn1ss_png "$LOGCAT_FILE" "$test_name" > "$dest" 2>/dev/null; then + if verify_png "$dest"; then source="LOGCAT"; fi + fi + fi + fi + + if [ -z "$source" ] && [ -n "${TEST_EXEC_LOG:-}" ] && [ -s "$TEST_EXEC_LOG" ]; then + count="$(count_chunks "$TEST_EXEC_LOG" "$test_name")"; count="${count//[^0-9]/}"; : "${count:=0}" + if [ "$count" -gt 0 ]; then + ra_log "Reassembling test '$test_name' from test-results.log: $TEST_EXEC_LOG (chunks=$count)" + if decode_cn1ss_png "$TEST_EXEC_LOG" "$test_name" > "$dest" 2>/dev/null; then + if verify_png "$dest"; then source="EXECLOG"; fi + fi + fi + fi + + if [ -n "$source" ]; then + printf '%s' "$source" + return 0 + fi + + return 1 } # Verify PNG signature + non-zero size @@ -79,7 +219,9 @@ ENV_FILE="$ENV_DIR/env.sh" ARTIFACTS_DIR="${ARTIFACTS_DIR:-${GITHUB_WORKSPACE:-$REPO_ROOT}/artifacts}" ensure_dir "$ARTIFACTS_DIR" TEST_LOG="$ARTIFACTS_DIR/connectedAndroidTest.log" -SCREENSHOT_OUT="$ARTIFACTS_DIR/emulator-screenshot.png" +SCREENSHOT_REF_DIR="$SCRIPT_DIR/android/screenshots" +SCREENSHOT_TMP_DIR="$(mktemp -d "${TMPDIR}/cn1ss-XXXXXX" 2>/dev/null || echo "${TMPDIR}/cn1ss-tmp")" +ensure_dir "$SCREENSHOT_TMP_DIR" ra_log "Loading workspace environment from $ENV_FILE" [ -f "$ENV_FILE" ] || { ra_log "Missing env file: $ENV_FILE"; exit 3; } @@ -168,72 +310,206 @@ if [ "${LOGCAT_CHUNKS:-0}" = "0" ] && [ "${XML_CHUNKS_TOTAL:-0}" = "0" ] && [ "$ exit 12 fi -# ---- Reassemble (prefer XML → logcat → exec log) -------------------------- +# ---- Identify CN1SS test streams ----------------------------------------- -: > "$SCREENSHOT_OUT" -SOURCE="" +declare -A TEST_NAME_SET=() -if [ "${#XMLS[@]}" -gt 0 ] && [ "${XML_CHUNKS_TOTAL:-0}" -gt 0 ]; then +if [ "${#XMLS[@]}" -gt 0 ]; then for x in "${XMLS[@]}"; do - c="$(count_chunks "$x")"; c="${c//[^0-9]/}"; : "${c:=0}" - [ "$c" -gt 0 ] || continue - ra_log "Reassembling from XML: $x (chunks=$c)" - if decode_cn1ss_png "$x" > "$SCREENSHOT_OUT" 2>/dev/null; then - if verify_png "$SCREENSHOT_OUT"; then SOURCE="XML"; break; fi - fi + while IFS= read -r name; do + [ -n "$name" ] || continue + TEST_NAME_SET["$name"]=1 + done < <(list_cn1ss_tests "$x" 2>/dev/null || true) done fi -if [ -z "$SOURCE" ] && [ "${LOGCAT_CHUNKS:-0}" -gt 0 ]; then - ra_log "Reassembling from logcat: $LOGCAT_FILE (chunks=$LOGCAT_CHUNKS)" - if decode_cn1ss_png "$LOGCAT_FILE" > "$SCREENSHOT_OUT" 2>/dev/null; then - if verify_png "$SCREENSHOT_OUT"; then SOURCE="LOGCAT"; fi - fi +if [ -s "${LOGCAT_FILE:-}" ]; then + while IFS= read -r name; do + [ -n "$name" ] || continue + TEST_NAME_SET["$name"]=1 + done < <(list_cn1ss_tests "$LOGCAT_FILE" 2>/dev/null || true) fi -if [ -z "$SOURCE" ] && [ -n "${TEST_EXEC_LOG:-}" ] && [ "${EXECLOG_CHUNKS:-0}" -gt 0 ]; then - ra_log "Reassembling from test-results.log: $TEST_EXEC_LOG (chunks=$EXECLOG_CHUNKS)" - if decode_cn1ss_png "$TEST_EXEC_LOG" > "$SCREENSHOT_OUT" 2>/dev/null; then - if verify_png "$SCREENSHOT_OUT"; then SOURCE="EXECLOG"; fi - fi +if [ -n "${TEST_EXEC_LOG:-}" ] && [ -s "$TEST_EXEC_LOG" ]; then + while IFS= read -r name; do + [ -n "$name" ] || continue + TEST_NAME_SET["$name"]=1 + done < <(list_cn1ss_tests "$TEST_EXEC_LOG" 2>/dev/null || true) fi -# ---- Final validation / failure paths ------------------------------------- - -if [ -z "$SOURCE" ]; then - ra_log "FATAL: Failed to extract/decode CN1SS payload from any source" - # Keep partial for debugging - RAW_B64_OUT="${SCREENSHOT_OUT}.raw.b64" - { - # Try to emit concatenated base64 from whichever had chunks (priority logcat, then XML, then exec) - if [ "${LOGCAT_CHUNKS:-0}" -gt 0 ]; then extract_cn1ss_base64 "$LOGCAT_FILE"; fi - if [ "${XML_CHUNKS_TOTAL:-0}" -gt 0 ] && [ "${LOGCAT_CHUNKS:-0}" -eq 0 ]; then - # concatenate all XMLs - for x in "${XMLS[@]}"; do - if [ "$(count_chunks "$x")" -gt 0 ]; then extract_cn1ss_base64 "$x"; fi - done - fi - if [ -n "${TEST_EXEC_LOG:-}" ] && [ "${EXECLOG_CHUNKS:-0}" -gt 0 ] && [ "${LOGCAT_CHUNKS:-0}" -eq 0 ] && [ "${XML_CHUNKS_TOTAL:-0}" -eq 0 ]; then - extract_cn1ss_base64 "$TEST_EXEC_LOG" +if [ "${#TEST_NAME_SET[@]}" -eq 0 ] && { [ "${LOGCAT_CHUNKS:-0}" -gt 0 ] || [ "${XML_CHUNKS_TOTAL:-0}" -gt 0 ] || [ "${EXECLOG_CHUNKS:-0}" -gt 0 ]; }; then + TEST_NAME_SET["default"]=1 +fi + +if [ "${#TEST_NAME_SET[@]}" -eq 0 ]; then + ra_log "FATAL: Could not determine any CN1SS test streams" + exit 12 +fi + +declare -a TEST_NAMES=() +for name in "${!TEST_NAME_SET[@]}"; do + TEST_NAMES+=("$name") +done +IFS=$'\n' TEST_NAMES=($(printf '%s\n' "${TEST_NAMES[@]}" | sort)) +unset IFS +ra_log "Detected CN1SS test streams: ${TEST_NAMES[*]}" + +declare -A TEST_OUTPUTS=() +declare -A TEST_SOURCES=() + +for test in "${TEST_NAMES[@]}"; do + dest="$SCREENSHOT_TMP_DIR/${test}.png" + if source_label="$(decode_test_png "$test" "$dest")"; then + TEST_OUTPUTS["$test"]="$dest" + TEST_SOURCES["$test"]="$source_label" + ra_log "Decoded screenshot for '$test' (source=${source_label}, size: $(stat -c '%s' "$dest") bytes)" + else + ra_log "FATAL: Failed to extract/decode CN1SS payload for test '$test'" + RAW_B64_OUT="$SCREENSHOT_TMP_DIR/${test}.raw.b64" + { + local count + count="$(count_chunks "$LOGCAT_FILE" "$test")"; count="${count//[^0-9]/}"; : "${count:=0}" + if [ "$count" -gt 0 ]; then extract_cn1ss_base64 "$LOGCAT_FILE" "$test"; fi + if [ "${#XMLS[@]}" -gt 0 ]; then + for x in "${XMLS[@]}"; do + count="$(count_chunks "$x" "$test")"; count="${count//[^0-9]/}"; : "${count:=0}" + if [ "$count" -gt 0 ]; then extract_cn1ss_base64 "$x" "$test"; fi + done + fi + if [ -n "${TEST_EXEC_LOG:-}" ] && [ -s "$TEST_EXEC_LOG" ]; then + count="$(count_chunks "$TEST_EXEC_LOG" "$test")"; count="${count//[^0-9]/}"; : "${count:=0}" + if [ "$count" -gt 0 ]; then extract_cn1ss_base64 "$TEST_EXEC_LOG" "$test"; fi + fi + } > "$RAW_B64_OUT" 2>/dev/null || true + if [ -s "$RAW_B64_OUT" ]; then + head -c 64 "$RAW_B64_OUT" | sed 's/^/[CN1SS-B64-HEAD] /' + ra_log "Partial base64 saved at: $RAW_B64_OUT" fi - } > "$RAW_B64_OUT" 2>/dev/null || true - if [ -s "$RAW_B64_OUT" ]; then - head -c 64 "$RAW_B64_OUT" | sed 's/^/[CN1SS-B64-HEAD] /' - ra_log "Partial base64 saved at: $RAW_B64_OUT" + exit 12 fi - # Emit contextual INFO lines - grep -n 'CN1SS:INFO' "$LOGCAT_FILE" 2>/dev/null || true - exit 12 +done + +# ---- Compare against stored references ------------------------------------ + +COMPARE_ARGS=() +for test in "${TEST_NAMES[@]}"; do + dest="${TEST_OUTPUTS[$test]:-}" + [ -n "$dest" ] || continue + COMPARE_ARGS+=("--actual" "${test}=${dest}") +done + +COMPARE_JSON="$SCREENSHOT_TMP_DIR/screenshot-compare.json" +python3 "$SCRIPT_DIR/android/tests/process_screenshots.py" \ + --reference-dir "$SCREENSHOT_REF_DIR" \ + --emit-base64 \ + "${COMPARE_ARGS[@]}" > "$COMPARE_JSON" + +SUMMARY_FILE="$SCREENSHOT_TMP_DIR/screenshot-summary.txt" +COMMENT_FILE="$SCREENSHOT_TMP_DIR/screenshot-comment.md" + +python3 <<'PY' "$COMPARE_JSON" "$COMMENT_FILE" "$SUMMARY_FILE" +import json +import pathlib +import sys + +compare_path = pathlib.Path(sys.argv[1]) +comment_path = pathlib.Path(sys.argv[2]) +summary_path = pathlib.Path(sys.argv[3]) + +data = json.loads(compare_path.read_text(encoding="utf-8")) +summary_lines = [] +comment_entries = [] + +for result in data.get("results", []): + test = result.get("test", "unknown") + status = result.get("status", "unknown") + expected_path = result.get("expected_path") + actual_path = result.get("actual_path", "") + details = result.get("details") or {} + base64_data = result.get("base64") + message = "" + copy_flag = "0" + + if status == "equal": + message = "Matches stored reference." + elif status == "missing_expected": + message = f"Reference screenshot missing at {expected_path}." + copy_flag = "1" + comment_entries.append({ + "test": test, + "status": "missing reference", + "message": message, + "base64": base64_data, + }) + elif status == "different": + dims = "" + if details: + dims = f" ({details.get('width')}x{details.get('height')} px, bit depth {details.get('bit_depth')})" + message = f"Screenshot differs{dims}." + copy_flag = "1" + comment_entries.append({ + "test": test, + "status": "updated screenshot", + "message": message, + "base64": base64_data, + }) + elif status == "error": + message = f"Comparison error: {result.get('message', 'unknown error')}" + copy_flag = "1" + comment_entries.append({ + "test": test, + "status": "comparison error", + "message": message, + "base64": None, + }) + elif status == "missing_actual": + message = "Actual screenshot missing (test did not produce output)." + copy_flag = "1" + comment_entries.append({ + "test": test, + "status": "missing actual screenshot", + "message": message, + "base64": None, + }) + else: + message = f"Status: {status}." + + summary_lines.append("|".join([status, test, message, copy_flag, actual_path])) + +summary_path.write_text("\n".join(summary_lines) + ("\n" if summary_lines else ""), encoding="utf-8") + +if comment_entries: + lines = ["### Android screenshot updates", ""] + for entry in comment_entries: + lines.append(f"- **{entry['test']}** — {entry['status']}. {entry['message']}") + if entry.get("base64"): + lines.append("") + lines.append(f" ![{entry['test']}](data:image/png;base64,{entry['base64']})") + lines.append("") + comment_path.write_text("\n".join(lines).rstrip() + "\n", encoding="utf-8") +else: + comment_path.write_text("", encoding="utf-8") +PY + +if [ -s "$SUMMARY_FILE" ]; then + while IFS='|' read -r status test message copy_flag path; do + [ -n "${test:-}" ] || continue + ra_log "Test '${test}': ${message}" + if [ "$copy_flag" = "1" ] && [ -n "${path:-}" ] && [ -f "$path" ]; then + cp -f "$path" "$ARTIFACTS_DIR/${test}.png" 2>/dev/null || true + fi + if [ "$status" = "equal" ] && [ -n "${path:-}" ]; then + rm -f "$path" 2>/dev/null || true + fi + done < "$SUMMARY_FILE" fi -# Size & signature check (belt & suspenders) -if ! verify_png "$SCREENSHOT_OUT"; then - ra_log "STAGE:BAD_PNG_SIGNATURE -> Not a PNG" - file "$SCREENSHOT_OUT" || true - exit 14 +cp -f "$COMPARE_JSON" "$ARTIFACTS_DIR/screenshot-compare.json" 2>/dev/null || true +if [ -s "$COMMENT_FILE" ]; then + cp -f "$COMMENT_FILE" "$ARTIFACTS_DIR/screenshot-comment.md" 2>/dev/null || true fi -ra_log "SUCCESS -> screenshot saved (${SOURCE}), size: $(stat -c '%s' "$SCREENSHOT_OUT") bytes at $SCREENSHOT_OUT" +post_pr_comment "$COMMENT_FILE" # Copy useful artifacts for GH Actions cp -f "$LOGCAT_FILE" "$ARTIFACTS_DIR/$(basename "$LOGCAT_FILE")" 2>/dev/null || true From 91e5b63d062df0c7323054ed244fe1934bec8c57 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 15 Oct 2025 04:49:34 +0300 Subject: [PATCH 2/3] Compress screenshot previews in PR comments --- scripts/android/tests/process_screenshots.py | 102 +++++++++++++++++-- scripts/run-android-instrumentation-tests.sh | 1 + 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/scripts/android/tests/process_screenshots.py b/scripts/android/tests/process_screenshots.py index fac8877443..83a2ad909d 100644 --- a/scripts/android/tests/process_screenshots.py +++ b/scripts/android/tests/process_screenshots.py @@ -7,10 +7,13 @@ import base64 import json import pathlib +import struct import sys import zlib from dataclasses import dataclass -from typing import Dict, List, Tuple +from typing import Dict, Iterable, List, Tuple + +MAX_COMMENT_BASE64 = 40_000 PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n" @@ -162,9 +165,7 @@ def load_png(path: pathlib.Path) -> PNGImage: return PNGImage(width, height, bit_depth, color_type, pixels, bpp) -def compare_images(expected_path: pathlib.Path, actual_path: pathlib.Path) -> Dict[str, bool]: - expected = load_png(expected_path) - actual = load_png(actual_path) +def compare_images(expected: PNGImage, actual: PNGImage) -> Dict[str, bool]: equal = ( expected.width == actual.width and expected.height == actual.height @@ -181,6 +182,85 @@ def compare_images(expected_path: pathlib.Path, actual_path: pathlib.Path) -> Di } +def _encode_png(width: int, height: int, bit_depth: int, color_type: int, bpp: int, pixels: bytes) -> bytes: + import zlib as _zlib + + if len(pixels) != width * height * bpp: + raise PNGError("Pixel buffer length does not match dimensions") + + def chunk(tag: bytes, payload: bytes) -> bytes: + crc = _zlib.crc32(tag + payload) & 0xFFFFFFFF + return ( + len(payload).to_bytes(4, "big") + + tag + + payload + + crc.to_bytes(4, "big") + ) + + raw = bytearray() + stride = width * bpp + for row in range(height): + raw.append(0) + start = row * stride + raw.extend(pixels[start : start + stride]) + + ihdr = struct.pack( + ">IIBBBBB", + width, + height, + bit_depth, + color_type, + 0, + 0, + 0, + ) + + compressed = _zlib.compress(bytes(raw)) + return b"".join( + [PNG_SIGNATURE, chunk(b"IHDR", ihdr), chunk(b"IDAT", compressed), chunk(b"IEND", b"")] + ) + + +def _downscale_half(width: int, height: int, bpp: int, pixels: bytes) -> Tuple[int, int, bytes]: + new_width = max(1, (width + 1) // 2) + new_height = max(1, (height + 1) // 2) + new_pixels = bytearray(new_width * new_height * bpp) + + for ny in range(new_height): + for nx in range(new_width): + accum = [0] * bpp + samples = 0 + for dy in (0, 1): + sy = min(height - 1, ny * 2 + dy) + for dx in (0, 1): + sx = min(width - 1, nx * 2 + dx) + src_index = (sy * width + sx) * bpp + for channel in range(bpp): + accum[channel] += pixels[src_index + channel] + samples += 1 + dst_index = (ny * new_width + nx) * bpp + for channel in range(bpp): + new_pixels[dst_index + channel] = accum[channel] // samples + + return new_width, new_height, bytes(new_pixels) + + +def build_preview_base64(image: PNGImage, max_length: int = MAX_COMMENT_BASE64) -> str: + width = image.width + height = image.height + bpp = image.bytes_per_pixel + pixels = image.pixels + + while True: + png_bytes = _encode_png(width, height, image.bit_depth, image.color_type, bpp, pixels) + encoded = base64.b64encode(png_bytes).decode("ascii") + if len(encoded) <= max_length or width <= 1 or height <= 1: + return encoded + if image.color_type not in {0, 2, 4, 6}: + return encoded + width, height, pixels = _downscale_half(width, height, bpp, pixels) + + def build_results(reference_dir: pathlib.Path, actual_entries: List[Tuple[str, pathlib.Path]], emit_base64: bool) -> Dict[str, List[Dict[str, object]]]: results: List[Dict[str, object]] = [] for test_name, actual_path in actual_entries: @@ -195,10 +275,15 @@ def build_results(reference_dir: pathlib.Path, actual_entries: List[Tuple[str, p elif not expected_path.exists(): record.update({"status": "missing_expected"}) if emit_base64: - record["base64"] = base64.b64encode(actual_path.read_bytes()).decode("ascii") + try: + record["base64"] = build_preview_base64(load_png(actual_path)) + except Exception: + record["base64"] = base64.b64encode(actual_path.read_bytes()).decode("ascii") else: try: - outcome = compare_images(expected_path, actual_path) + actual_img = load_png(actual_path) + expected_img = load_png(expected_path) + outcome = compare_images(expected_img, actual_img) except Exception as exc: record.update({"status": "error", "message": str(exc)}) else: @@ -207,7 +292,10 @@ def build_results(reference_dir: pathlib.Path, actual_entries: List[Tuple[str, p else: record.update({"status": "different", "details": outcome}) if emit_base64: - record["base64"] = base64.b64encode(actual_path.read_bytes()).decode("ascii") + try: + record["base64"] = build_preview_base64(actual_img) + except Exception: + record["base64"] = base64.b64encode(actual_path.read_bytes()).decode("ascii") results.append(record) return {"results": results} diff --git a/scripts/run-android-instrumentation-tests.sh b/scripts/run-android-instrumentation-tests.sh index 7c2beacd63..1f1c4c29e0 100755 --- a/scripts/run-android-instrumentation-tests.sh +++ b/scripts/run-android-instrumentation-tests.sh @@ -485,6 +485,7 @@ if comment_entries: if entry.get("base64"): lines.append("") lines.append(f" ![{entry['test']}](data:image/png;base64,{entry['base64']})") + lines.append(" _(Preview image scaled for comment delivery.)_") lines.append("") comment_path.write_text("\n".join(lines).rstrip() + "\n", encoding="utf-8") else: From ad247aa48971066af5d6f8dde16cb4bc0f4315b8 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 15 Oct 2025 05:19:54 +0300 Subject: [PATCH 3/3] Preserve full-resolution screenshot previews --- scripts/android/tests/process_screenshots.py | 79 +++++++++----------- scripts/run-android-instrumentation-tests.sh | 28 ++++++- 2 files changed, 63 insertions(+), 44 deletions(-) diff --git a/scripts/android/tests/process_screenshots.py b/scripts/android/tests/process_screenshots.py index 83a2ad909d..36a2b87f6b 100644 --- a/scripts/android/tests/process_screenshots.py +++ b/scripts/android/tests/process_screenshots.py @@ -13,7 +13,7 @@ from dataclasses import dataclass from typing import Dict, Iterable, List, Tuple -MAX_COMMENT_BASE64 = 40_000 +MAX_COMMENT_BASE64 = 60_000 PNG_SIGNATURE = b"\x89PNG\r\n\x1a\n" @@ -221,44 +221,19 @@ def chunk(tag: bytes, payload: bytes) -> bytes: ) -def _downscale_half(width: int, height: int, bpp: int, pixels: bytes) -> Tuple[int, int, bytes]: - new_width = max(1, (width + 1) // 2) - new_height = max(1, (height + 1) // 2) - new_pixels = bytearray(new_width * new_height * bpp) - - for ny in range(new_height): - for nx in range(new_width): - accum = [0] * bpp - samples = 0 - for dy in (0, 1): - sy = min(height - 1, ny * 2 + dy) - for dx in (0, 1): - sx = min(width - 1, nx * 2 + dx) - src_index = (sy * width + sx) * bpp - for channel in range(bpp): - accum[channel] += pixels[src_index + channel] - samples += 1 - dst_index = (ny * new_width + nx) * bpp - for channel in range(bpp): - new_pixels[dst_index + channel] = accum[channel] // samples - - return new_width, new_height, bytes(new_pixels) - - -def build_preview_base64(image: PNGImage, max_length: int = MAX_COMMENT_BASE64) -> str: - width = image.width - height = image.height - bpp = image.bytes_per_pixel - pixels = image.pixels - - while True: - png_bytes = _encode_png(width, height, image.bit_depth, image.color_type, bpp, pixels) - encoded = base64.b64encode(png_bytes).decode("ascii") - if len(encoded) <= max_length or width <= 1 or height <= 1: - return encoded - if image.color_type not in {0, 2, 4, 6}: - return encoded - width, height, pixels = _downscale_half(width, height, bpp, pixels) +def build_base64_payload(image: PNGImage, max_length: int = MAX_COMMENT_BASE64) -> Tuple[str | None, int]: + png_bytes = _encode_png( + image.width, + image.height, + image.bit_depth, + image.color_type, + image.bytes_per_pixel, + image.pixels, + ) + encoded = base64.b64encode(png_bytes).decode("ascii") + if len(encoded) <= max_length: + return encoded, len(encoded) + return None, len(encoded) def build_results(reference_dir: pathlib.Path, actual_entries: List[Tuple[str, pathlib.Path]], emit_base64: bool) -> Dict[str, List[Dict[str, object]]]: @@ -275,10 +250,19 @@ def build_results(reference_dir: pathlib.Path, actual_entries: List[Tuple[str, p elif not expected_path.exists(): record.update({"status": "missing_expected"}) if emit_base64: + payload = None + length = 0 try: - record["base64"] = build_preview_base64(load_png(actual_path)) + payload, length = build_base64_payload(load_png(actual_path)) except Exception: - record["base64"] = base64.b64encode(actual_path.read_bytes()).decode("ascii") + raw = base64.b64encode(actual_path.read_bytes()).decode("ascii") + length = len(raw) + if length <= MAX_COMMENT_BASE64: + payload = raw + if payload is not None: + record["base64"] = payload + elif length: + record.update({"base64_omitted": "too_large", "base64_length": length}) else: try: actual_img = load_png(actual_path) @@ -292,10 +276,19 @@ def build_results(reference_dir: pathlib.Path, actual_entries: List[Tuple[str, p else: record.update({"status": "different", "details": outcome}) if emit_base64: + payload = None + length = 0 try: - record["base64"] = build_preview_base64(actual_img) + payload, length = build_base64_payload(actual_img) except Exception: - record["base64"] = base64.b64encode(actual_path.read_bytes()).decode("ascii") + raw = base64.b64encode(actual_path.read_bytes()).decode("ascii") + length = len(raw) + if length <= MAX_COMMENT_BASE64: + payload = raw + if payload is not None: + record["base64"] = payload + elif length: + record.update({"base64_omitted": "too_large", "base64_length": length}) results.append(record) return {"results": results} diff --git a/scripts/run-android-instrumentation-tests.sh b/scripts/run-android-instrumentation-tests.sh index 1f1c4c29e0..21ef272a15 100755 --- a/scripts/run-android-instrumentation-tests.sh +++ b/scripts/run-android-instrumentation-tests.sh @@ -427,6 +427,8 @@ for result in data.get("results", []): actual_path = result.get("actual_path", "") details = result.get("details") or {} base64_data = result.get("base64") + base64_omitted = result.get("base64_omitted") + base64_length = result.get("base64_length") message = "" copy_flag = "0" @@ -440,6 +442,9 @@ for result in data.get("results", []): "status": "missing reference", "message": message, "base64": base64_data, + "base64_omitted": base64_omitted, + "base64_length": base64_length, + "artifact_name": f"{test}.png", }) elif status == "different": dims = "" @@ -452,6 +457,9 @@ for result in data.get("results", []): "status": "updated screenshot", "message": message, "base64": base64_data, + "base64_omitted": base64_omitted, + "base64_length": base64_length, + "artifact_name": f"{test}.png", }) elif status == "error": message = f"Comparison error: {result.get('message', 'unknown error')}" @@ -461,6 +469,9 @@ for result in data.get("results", []): "status": "comparison error", "message": message, "base64": None, + "base64_omitted": base64_omitted, + "base64_length": base64_length, + "artifact_name": f"{test}.png", }) elif status == "missing_actual": message = "Actual screenshot missing (test did not produce output)." @@ -470,6 +481,9 @@ for result in data.get("results", []): "status": "missing actual screenshot", "message": message, "base64": None, + "base64_omitted": base64_omitted, + "base64_length": base64_length, + "artifact_name": None, }) else: message = f"Status: {status}." @@ -485,7 +499,19 @@ if comment_entries: if entry.get("base64"): lines.append("") lines.append(f" ![{entry['test']}](data:image/png;base64,{entry['base64']})") - lines.append(" _(Preview image scaled for comment delivery.)_") + artifact_name = entry.get("artifact_name") + if artifact_name: + lines.append(f" _Full-resolution PNG saved as `{artifact_name}` in workflow artifacts._") + lines.append("") + elif entry.get("base64_omitted") == "too_large": + artifact_name = entry.get("artifact_name") + size_note = "" + if entry.get("base64_length"): + size_note = f" (base64 length ≈ {entry['base64_length']:,} chars)" + lines.append("") + lines.append(" _Screenshot omitted from comment because the encoded payload exceeded GitHub's size limits" + size_note + "._") + if artifact_name: + lines.append(f" _Full-resolution PNG saved as `{artifact_name}` in workflow artifacts._") lines.append("") comment_path.write_text("\n".join(lines).rstrip() + "\n", encoding="utf-8") else: