Skip to content

Commit d071e7b

Browse files
JoeMattJoeMatt
authored
Merge pull request #94 from Provenance-Emu/feature/visual-regression-diffs
Add visual diff comparison to regression tests
2 parents 8daa730 + 129ed66 commit d071e7b

4 files changed

Lines changed: 154 additions & 24 deletions

File tree

‎.github/workflows/regression-test.yml‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ on:
66
pull_request:
77
branches: [ master ]
88

9+
permissions:
10+
pull-requests: write
11+
912
jobs:
1013
regression:
1114
strategy:
@@ -16,23 +19,34 @@ jobs:
1619
cc: gcc
1720
cxx: g++
1821
core: virtualjaguar_libretro.so
22+
platform: linux-x64
1923

2024
- os: ubuntu-24.04-arm
2125
cc: gcc
2226
cxx: g++
2327
core: virtualjaguar_libretro.so
28+
platform: linux-arm64
2429

2530
- os: macos-latest
2631
cc: clang
2732
cxx: clang++
2833
core: virtualjaguar_libretro.dylib
34+
platform: macos-arm64
2935

30-
name: regression-${{ matrix.config.os }}
36+
name: regression-${{ matrix.config.platform }}
3137
runs-on: ${{ matrix.config.os }}
3238

3339
steps:
3440
- uses: actions/checkout@v4
3541

42+
- name: Install ImageMagick
43+
run: |
44+
if [ "$(uname -s)" = "Darwin" ]; then
45+
brew install imagemagick
46+
else
47+
sudo apt-get update -qq && sudo apt-get install -y -qq imagemagick
48+
fi
49+
3650
- name: Cache miniretro
3751
id: cache-miniretro
3852
uses: actions/cache@v4
@@ -59,5 +73,63 @@ jobs:
5973
- name: Run regression tests
6074
run: |
6175
export MINIRETRO_BIN="$(pwd)/miniretro-bin"
76+
export DIFF_DIR="$(pwd)/regression-diffs"
6277
chmod +x "${MINIRETRO_BIN}"
6378
./test/regression_test.sh ./${{ matrix.config.core }}
79+
80+
- name: Upload diff artifacts
81+
if: failure()
82+
uses: actions/upload-artifact@v4
83+
with:
84+
name: regression-diffs-${{ matrix.config.platform }}
85+
path: regression-diffs/
86+
if-no-files-found: ignore
87+
88+
- name: Comment on PR with results
89+
if: always() && github.event_name == 'pull_request'
90+
continue-on-error: true
91+
uses: actions/github-script@v7
92+
with:
93+
script: |
94+
const fs = require('fs');
95+
const path = 'regression-diffs/summary.md';
96+
if (!fs.existsSync(path)) return;
97+
98+
const summary = fs.readFileSync(path, 'utf8');
99+
const platform = '${{ matrix.config.platform }}';
100+
101+
// Paginate to find existing comment even on busy PRs
102+
const comments = await github.paginate(
103+
github.rest.issues.listComments,
104+
{
105+
owner: context.repo.owner,
106+
repo: context.repo.repo,
107+
issue_number: context.issue.number,
108+
per_page: 100,
109+
}
110+
);
111+
const marker = `<!-- regression-${platform} -->`;
112+
const existing = comments.find(c => c.body.includes(marker));
113+
114+
const hasDiffs = fs.readdirSync('regression-diffs').some(f => f.endsWith('_diff.png'));
115+
const body = `${marker}\n### Regression: \`${platform}\`\n\n${summary}\n\n` +
116+
(hasDiffs
117+
? `> :warning: Visual diffs uploaded as artifacts — download \`regression-diffs-${platform}\` to inspect.\n`
118+
: '') +
119+
`\n<sub>Updated by CI at ${new Date().toISOString()}</sub>`;
120+
121+
if (existing) {
122+
await github.rest.issues.updateComment({
123+
owner: context.repo.owner,
124+
repo: context.repo.repo,
125+
comment_id: existing.id,
126+
body,
127+
});
128+
} else {
129+
await github.rest.issues.createComment({
130+
owner: context.repo.owner,
131+
repo: context.repo.repo,
132+
issue_number: context.issue.number,
133+
body,
134+
});
135+
}

‎test/baselines/jagniccc.md5‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

‎test/baselines/jagniccc.png‎

7.5 KB
Loading

‎test/regression_test.sh‎

Lines changed: 81 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22
#
33
# Headless regression test for virtualjaguar-libretro
44
#
5-
# Builds miniretro, runs test ROMs for N frames, dumps screenshots,
6-
# and compares the last screenshot's checksum against a known baseline.
5+
# Runs test ROMs via miniretro, compares screenshots against reference
6+
# images, and generates visual diffs on failure.
77
#
88
# Usage: ./test/regression_test.sh <core_path>
99
# Example: ./test/regression_test.sh ./virtualjaguar_libretro.so
1010
#
1111
# Set MINIRETRO_BIN env var to skip building miniretro from source.
12+
# Set DIFF_DIR env var to specify where diff images are saved.
1213
#
1314
set -euo pipefail
1415

@@ -17,6 +18,7 @@ SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
1718
WORK_DIR="$(mktemp -d)"
1819
BASELINE_DIR="${SCRIPT_DIR}/baselines"
1920
ROM_DIR="${SCRIPT_DIR}/roms"
21+
DIFF_DIR="${DIFF_DIR:-${WORK_DIR}/diffs}"
2022
# 600 frames (~10 seconds at 60fps) to get past BIOS boot
2123
FRAMES=600
2224
DUMP_EVERY=100
@@ -55,6 +57,8 @@ else
5557
fi
5658

5759
echo "==> Baselines: ${BASELINE_DIR}"
60+
echo "==> Diff output: ${DIFF_DIR}"
61+
mkdir -p "${DIFF_DIR}"
5862

5963
# --- Resolve core to absolute path ---
6064
CORE="$(cd "$(dirname "${CORE}")" && pwd)/$(basename "${CORE}")"
@@ -63,6 +67,7 @@ CORE="$(cd "$(dirname "${CORE}")" && pwd)/$(basename "${CORE}")"
6367
PASS=0
6468
FAIL=0
6569
NEW=0
70+
SUMMARY=""
6671

6772
for rom in "${ROM_DIR}"/*.j64 "${ROM_DIR}"/*.rom; do
6873
[ -f "${rom}" ] || continue
@@ -86,41 +91,95 @@ for rom in "${ROM_DIR}"/*.j64 "${ROM_DIR}"/*.rom; do
8691
if [ -z "${frame_file}" ]; then
8792
echo " WARNING: No frame dumped for ${rom_name}"
8893
FAIL=$((FAIL + 1))
94+
SUMMARY="${SUMMARY}| ${rom_name} | :x: FAIL | No frame output | - |\n"
8995
continue
9096
fi
9197

9298
echo " Using frame: $(basename "${frame_file}")"
9399

94-
# Compute checksum
95-
if command -v md5sum &>/dev/null; then
96-
hash=$(md5sum "${frame_file}" | awk '{print $1}')
97-
else
98-
hash=$(md5 -q "${frame_file}")
99-
fi
100-
101-
baseline_file="${BASELINE_DIR}/${rom_name}.md5"
102-
103-
if [ -f "${baseline_file}" ]; then
104-
expected=$(cat "${baseline_file}")
105-
if [ "${hash}" = "${expected}" ]; then
106-
echo " PASS: ${rom_name} (${hash})"
107-
PASS=$((PASS + 1))
100+
# Copy current screenshot to diff dir for reference
101+
cp "${frame_file}" "${DIFF_DIR}/${rom_name}_current.png"
102+
103+
baseline_png="${BASELINE_DIR}/${rom_name}.png"
104+
105+
if [ -f "${baseline_png}" ]; then
106+
# Compare against reference screenshot
107+
if command -v compare &>/dev/null; then
108+
# ImageMagick compare: generate diff image and get metric
109+
set +e
110+
metric_raw=$(compare -metric AE "${baseline_png}" "${frame_file}" \
111+
"${DIFF_DIR}/${rom_name}_diff.png" 2>&1)
112+
compare_status=$?
113+
set -e
114+
115+
if [ "${compare_status}" -le 1 ]; then
116+
# Extract just the integer pixel count (ImageMagick may output "0 (0)")
117+
metric=$(printf '%s\n' "${metric_raw}" | awk 'NR==1 {print $1}')
118+
119+
if [[ "${metric}" =~ ^[0-9]+$ ]] && [ "${metric}" = "0" ]; then
120+
echo " PASS: ${rom_name} (0 pixels differ)"
121+
PASS=$((PASS + 1))
122+
SUMMARY="${SUMMARY}| ${rom_name} | :white_check_mark: PASS | 0 pixels differ | - |\n"
123+
# Clean up diff artifacts on pass
124+
rm -f "${DIFF_DIR}/${rom_name}_diff.png" "${DIFF_DIR}/${rom_name}_current.png"
125+
elif [[ "${metric}" =~ ^[0-9]+$ ]]; then
126+
echo " FAIL: ${rom_name} (${metric} pixels differ)"
127+
# Also generate a side-by-side comparison
128+
if command -v montage &>/dev/null; then
129+
montage "${baseline_png}" "${frame_file}" "${DIFF_DIR}/${rom_name}_diff.png" \
130+
-tile 3x1 -geometry +4+4 -label '%f' \
131+
"${DIFF_DIR}/${rom_name}_sidebyside.png" 2>/dev/null || true
132+
fi
133+
cp "${baseline_png}" "${DIFF_DIR}/${rom_name}_expected.png"
134+
FAIL=$((FAIL + 1))
135+
SUMMARY="${SUMMARY}| ${rom_name} | :x: FAIL | ${metric} pixels differ | See artifacts |\n"
136+
else
137+
echo " FAIL: ${rom_name} (compare error: ${metric_raw})"
138+
cp "${baseline_png}" "${DIFF_DIR}/${rom_name}_expected.png"
139+
FAIL=$((FAIL + 1))
140+
SUMMARY="${SUMMARY}| ${rom_name} | :x: FAIL | compare error | See artifacts |\n"
141+
fi
142+
else
143+
echo " FAIL: ${rom_name} (compare failed: ${metric_raw})"
144+
cp "${baseline_png}" "${DIFF_DIR}/${rom_name}_expected.png"
145+
FAIL=$((FAIL + 1))
146+
SUMMARY="${SUMMARY}| ${rom_name} | :x: FAIL | compare error | See artifacts |\n"
147+
fi
108148
else
109-
echo " FAIL: ${rom_name}"
110-
echo " expected: ${expected}"
111-
echo " got: ${hash}"
112-
FAIL=$((FAIL + 1))
149+
# Fallback: byte-level comparison
150+
if cmp -s "${baseline_png}" "${frame_file}"; then
151+
echo " PASS: ${rom_name} (identical)"
152+
PASS=$((PASS + 1))
153+
SUMMARY="${SUMMARY}| ${rom_name} | :white_check_mark: PASS | identical | - |\n"
154+
else
155+
echo " FAIL: ${rom_name} (screenshots differ)"
156+
cp "${baseline_png}" "${DIFF_DIR}/${rom_name}_expected.png"
157+
FAIL=$((FAIL + 1))
158+
SUMMARY="${SUMMARY}| ${rom_name} | :x: FAIL | screenshots differ | See artifacts |\n"
159+
fi
113160
fi
114161
else
115-
echo " NEW: ${rom_name} — no baseline yet (${hash})"
116-
echo " Run: echo '${hash}' > ${baseline_file}"
162+
echo " NEW: ${rom_name} — no baseline yet"
163+
echo " To create: cp ${frame_file} ${baseline_png}"
117164
NEW=$((NEW + 1))
165+
SUMMARY="${SUMMARY}| ${rom_name} | :new: NEW | no baseline | - |\n"
118166
fi
119167
done
120168

121169
echo ""
122170
echo "==> Results: ${PASS} passed, ${FAIL} failed, ${NEW} new (no baseline)"
123171

172+
# Write summary for CI to pick up
173+
cat > "${DIFF_DIR}/summary.md" <<EOSUMMARY
174+
## Regression Test Results
175+
176+
| ROM | Status | Details | Diff |
177+
|-----|--------|---------|------|
178+
$(echo -e "${SUMMARY}")
179+
180+
**Platform:** $(uname -s) $(uname -m)
181+
EOSUMMARY
182+
124183
if [ "${FAIL}" -gt 0 ]; then
125184
exit 1
126185
fi

0 commit comments

Comments
 (0)