From b129e5df4abe73ad7883cdca801ca6ff7db4f359 Mon Sep 17 00:00:00 2001
From: Powei Feng <powei@google.com>
Date: Tue, 22 Jul 2025 11:47:18 -0700
Subject: [PATCH] renderdiff: fix upload issue second try (#8972)

 - Move upload step to later so that comparison result is also
    included in the upload
 - Fix the wrong relative paths in the comparison_result.json
 - Fix viewer logic to pick only the latest run for a PR.
---
 .github/workflows/presubmit.yml | 33 +++++++++++++++++++++++----------
 test/renderdiff/src/compare.py  |  4 ++--
 test/renderdiff/src/viewer.py   |  8 ++++++--
 3 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml
index 029d8622a5..b3814f82a1 100644
--- a/.github/workflows/presubmit.yml
+++ b/.github/workflows/presubmit.yml
@@ -130,7 +130,8 @@ jobs:
         run: |
           bash build/common/get-mesa.sh
           pip install tifffile numpy
-      - name: Render
+      - name: Render and compare
+        id: render_compare
         run: |
           TEST_DIR=test/renderdiff
           source ${TEST_DIR}/src/preamble.sh
@@ -140,20 +141,32 @@ jobs:
               python3 ${TEST_DIR}/src/golden_manager.py \
                   --branch=${GOLDEN_BRANCH} \
                   --output=${GOLDEN_OUTPUT_DIR}
-          # Note that we need to upload the output even if comparison fails
+
+          # Note that we need to upload the output even if comparison fails, so we undo `set -ex`
+          end_
+
+          python3 ${TEST_DIR}/src/compare.py \
+              --src=${GOLDEN_OUTPUT_DIR} \
+              --dest=${RENDER_OUTPUT_DIR} \
+              --out=${DIFF_OUTPUT_DIR} 2>&1 | tee compare_output.txt
+
+          if grep "Failed" compare_output.txt > /dev/null; then
+             DELIMITER="EOF_FILE_CONTENT_$(date +%s)" # Using timestamp to make it more unique
+             echo "err<<$DELIMITER" >> "$GITHUB_OUTPUT"
+             cat compare_output.txt >> "$GITHUB_OUTPUT"
+             echo "$DELIMITER" >> "$GITHUB_OUTPUT"
+          fi
       - uses: actions/upload-artifact@v4
         with:
           name: presubmit-renderdiff-result
           path: ./out/renderdiff
-      - name: Compare output
+      - name: Compare result
         run: |
-          TEST_DIR=test/renderdiff
-          source ${TEST_DIR}/src/preamble.sh
-          python3 ${TEST_DIR}/src/compare.py \
-              --src=${GOLDEN_OUTPUT_DIR} \
-              --dest=${RENDER_OUTPUT_DIR} \
-              --out=${DIFF_OUTPUT_DIR}
-          end_
+          ERROR_STR="${{ steps.render_compare.outputs.err }}"
+          if [ -n "${ERROR_STR}" ]; then
+              echo "${ERROR_STR}"
+              exit 1
+          fi
 
   validate-wgsl-webgpu:
     name: validate-wgsl-webgpu
diff --git a/test/renderdiff/src/compare.py b/test/renderdiff/src/compare.py
index da7d2da54e..4f083f45e6 100644
--- a/test/renderdiff/src/compare.py
+++ b/test/renderdiff/src/compare.py
@@ -45,8 +45,8 @@ def _compare_goldens(base_dir, comparison_dir, out_dir=None):
       output_fname = os.path.join(output_test_dir, "compare_results.json")
       results_meta = {
         'results': results,
-        'base_dir': os.path.relpath(output_test_dir, base_test_dir),
-        'comparison_dir': os.path.relpath(output_test_dir, comp_test_dir),
+        'base_dir': os.path.relpath(base_test_dir, output_test_dir),
+        'comparison_dir': os.path.relpath(comp_test_dir, output_test_dir)
       }
       with open(output_fname, 'w') as f:
         f.write(json.dumps(results_meta, indent=2))
diff --git a/test/renderdiff/src/viewer.py b/test/renderdiff/src/viewer.py
index ec1d743e26..2e22d8fe3f 100644
--- a/test/renderdiff/src/viewer.py
+++ b/test/renderdiff/src/viewer.py
@@ -95,8 +95,8 @@ def _download_github_artifacts(pr_number, github_token, output_dir= ".") -> None
       print("Consider checking GitHub Actions runs manually for this PR's branch on GitHub.")
       return None
 
-    # Filter for runs that completed successfully
-    successful_runs = [run for run in workflow_runs if run.get("conclusion") == "success" and run.get("status") == "completed"]
+    # Do not filter for runs that completed successfully
+    successful_runs = sorted(workflow_runs, key=lambda run: run['id'], reverse=True)
     if not successful_runs:
       print(f"No *successful and completed* workflow runs found for PR #{pr_number} with commit SHA {commit_sha}. Exiting.")
       return None
@@ -161,6 +161,10 @@ def _download_github_artifacts(pr_number, github_token, output_dir= ".") -> None
             print(f"  Error: Downloaded file for '{artifact_name}' is not a valid zip file. Skipping extraction.")
           except Exception as e:
             print(f"  An error occurred during extraction of '{artifact_name}': {e}")
+
+      # Once we find the lastest run with artifacts, we just quit
+      if len(artifacts) > 0:
+        break
     except requests.exceptions.HTTPError as e:
       print(f"  An HTTP error occurred while fetching artifacts for run {run_id}: {e}")
       if e.response.status_code == 403: