From 59f50350c7ffe5d6bdd8ee6e6981bd5ca24d3109 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Mon, 13 Apr 2026 10:52:59 +0200 Subject: [PATCH 1/4] =?UTF-8?q?refactor:=20ADR-006=20T3+T4=20=E2=80=94=20h?= =?UTF-8?q?oist=20setup,=20consolidate=20assertion=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T3: Hoist duplicated setup (base_path, new_path, driver, comparison) from 3 nested classes to parent DifferenceFinderTest. Nested classes inherit setup and only add their own (@finder). T4: Extract assert_driver_check_called(driver, check_type, times) parameterized helper. Old methods kept as one-line aliases. T6: Skipped — param order already matches Minitest convention. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/support/test_helpers.rb | 29 ++++++++++------------------- test/unit/difference_finder_test.rb | 21 +++++++-------------- 2 files changed, 17 insertions(+), 33 deletions(-) diff --git a/test/support/test_helpers.rb b/test/support/test_helpers.rb index 030e718d..e0a41c12 100644 --- a/test/support/test_helpers.rb +++ b/test/support/test_helpers.rb @@ -7,29 +7,20 @@ module TestHelpers # Common assertions for image comparison tests module Assertions - # Asserts that a dimension check was called a specific number of times + # Asserts that a driver check was called a specific number of times # @param driver [Object] The test driver object + # @param check_type [Symbol] :dimension_check, :pixel_check, or :difference_region # @param times [Integer] The expected number of calls (default: 1) - def assert_dimension_check_called(driver, times = 1) - assert_equal times, driver.dimension_check_calls.size, - "Expected dimension check to be called #{times} time(s)" + def assert_driver_check_called(driver, check_type, times = 1) + calls = driver.public_send(:"#{check_type}_calls") + assert_equal times, calls.size, + "Expected #{check_type} to be called #{times} time(s), was called #{calls.size}" end - # Asserts that a pixel check was called a specific number of times - # @param driver [Object] The test driver object - # @param times [Integer] The expected number of calls (default: 1) - def assert_pixel_check_called(driver, times = 1) - assert_equal times, driver.pixel_check_calls.size, - "Expected pixel check to be called #{times} time(s)" - end - - # Asserts that a difference region check was called a specific number of times - # @param driver [Object] The test driver object - # @param times [Integer] The expected number of calls (default: 1) - def assert_difference_region_called(driver, times = 1) - assert_equal times, driver.difference_region_calls.size, - "Expected difference region check to be called #{times} time(s)" - end + # Convenience aliases for backward compatibility + def assert_dimension_check_called(driver, times = 1) = assert_driver_check_called(driver, :dimension_check, times) + def assert_pixel_check_called(driver, times = 1) = assert_driver_check_called(driver, :pixel_check, times) + def assert_difference_region_called(driver, times = 1) = assert_driver_check_called(driver, :difference_region, times) end # Common setup methods for test drivers diff --git a/test/unit/difference_finder_test.rb b/test/unit/difference_finder_test.rb index 92557f77..512d761c 100644 --- a/test/unit/difference_finder_test.rb +++ b/test/unit/difference_finder_test.rb @@ -11,13 +11,14 @@ class DifferenceFinderTest < ActiveSupport::TestCase include CapybaraScreenshotDiff::DSLStub include TestDoubles + setup do + @base_path = TestDoubles::TestPath.new(12345) + @new_path = TestDoubles::TestPath.new(54321) + @driver = TestDoubles::TestDriver.new(false) + setup_test_comparison + end + class InitializationTest < self - setup do - @base_path = TestDoubles::TestPath.new(12345) - @new_path = TestDoubles::TestPath.new(54321) - @driver = TestDoubles::TestDriver.new(false) - setup_test_comparison - end test "#initialize sets driver and options correctly" do driver = TestDoubles::TestDriver.new @@ -32,10 +33,6 @@ class InitializationTest < self class QuickModeTest < self setup do - @base_path = TestDoubles::TestPath.new(12345) - @new_path = TestDoubles::TestPath.new(54321) - @driver = TestDoubles::TestDriver.new(false) - setup_test_comparison @finder = create_finder end @@ -76,10 +73,6 @@ class QuickModeTest < self class FullModeTest < self setup do - @base_path = TestDoubles::TestPath.new(12345) - @new_path = TestDoubles::TestPath.new(54321) - @driver = TestDoubles::TestDriver.new(false) - setup_test_comparison @finder = create_finder end From b6a50cc490a163572f49bd295452346850522c3f Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Mon, 13 Apr 2026 10:55:38 +0200 Subject: [PATCH 2/4] fix: remove extra empty line in InitializationTest (standardrb) Co-Authored-By: Claude Opus 4.6 (1M context) --- test/unit/difference_finder_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/difference_finder_test.rb b/test/unit/difference_finder_test.rb index 512d761c..fd8e6cb5 100644 --- a/test/unit/difference_finder_test.rb +++ b/test/unit/difference_finder_test.rb @@ -19,7 +19,6 @@ class DifferenceFinderTest < ActiveSupport::TestCase end class InitializationTest < self - test "#initialize sets driver and options correctly" do driver = TestDoubles::TestDriver.new options = {tolerance: 0.05} From 2fd834f01f83020f95be3b7ac76542870d2e9731 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Mon, 13 Apr 2026 11:21:14 +0200 Subject: [PATCH 3/4] refactor: move PR comment + job summary into upload-screenshots action Add pr-comment input to upload-screenshots composite action. When enabled, it finds/creates a PR comment with report links automatically. Also adds job summary with artifact links. Removes ~40 lines of duplicated YAML from test.yml. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/actions/upload-screenshots/action.yml | 43 ++++++++++++- .github/workflows/test.yml | 63 +------------------ 2 files changed, 43 insertions(+), 63 deletions(-) diff --git a/.github/actions/upload-screenshots/action.yml b/.github/actions/upload-screenshots/action.yml index 595720a7..7a937a8e 100644 --- a/.github/actions/upload-screenshots/action.yml +++ b/.github/actions/upload-screenshots/action.yml @@ -1,6 +1,6 @@ --- name: 'Upload SnapDiff screenshots' -description: 'Upload screenshot diffs and HTML report as CI artifacts' +description: 'Upload screenshot diffs, HTML report, and optionally comment on PR' inputs: name: description: 'Artifact name prefix' @@ -11,6 +11,9 @@ inputs: retention-days: description: 'Number of days to retain artifacts' default: '2' + pr-comment: + description: 'Post PR comment with report link (requires pull-requests: write)' + default: 'false' outputs: report-url: description: 'Direct URL to the inline HTML report artifact' @@ -70,3 +73,41 @@ runs: name: ${{ inputs.name }}-report-full retention-days: ${{ inputs.retention-days }} path: ${{ inputs.report-path }}/ + + - name: Job summary + if: steps.check-report.outputs.exists == 'true' + shell: bash + run: | + cat >> "$GITHUB_STEP_SUMMARY" <> "$GITHUB_STEP_SUMMARY" <> "$GITHUB_STEP_SUMMARY" <<'EOF' - ### SnapDiff Report - - | Artifact | Link | - |----------|------| - | HTML report (inline) | ${{ steps.upload-screenshots.outputs.report-url || 'N/A' }} | - | Full report with images | ${{ steps.upload-screenshots.outputs.report-full-url || 'N/A' }} | - EOF From b944dc6f02e55e323b52b8e3341e1cb31a0b7c04 Mon Sep 17 00:00:00 2001 From: Paul Keen <125715+pftg@users.noreply.github.com> Date: Mon, 13 Apr 2026 11:23:07 +0200 Subject: [PATCH 4/4] docs: update CI integration for new composite action API - Document pr-comment input for upload-screenshots action - Document setup-ruby-and-dependencies action with inputs table - Add full example combining both actions - Move manual setup and update-baselines to collapsible sections - Update README CI snippet to show pr-comment: 'true' Co-Authored-By: Claude Opus 4.6 (1M context) --- README.md | 5 +- docs/ci-integration.md | 173 +++++++++++++++++++++++++---------------- 2 files changed, 109 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index bd2249a3..e7e71fa8 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ After tests run, open `doc/screenshots/snap_diff_report.html`: Review all visual changes in one place — no need to hunt through `.diff.png` files. 4 view modes (both/base/new/heatmap), per-image zoom, annotation toggle, keyboard navigation, and search. -**In GitHub Actions**, the report renders inline as a CI artifact — no download needed. Add a PR comment with a link to the report automatically: +**In GitHub Actions**, one step uploads the report, posts a PR comment with the link, and adds a job summary: ```yaml - name: Upload screenshot report @@ -126,9 +126,10 @@ Review all visual changes in one place — no need to hunt through `.diff.png` f uses: snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@master with: name: screenshots + pr-comment: 'true' ``` -See [CI Integration](docs/ci-integration.md) for the full GitHub Actions setup with PR commenting. +See [CI Integration](docs/ci-integration.md) for full setup including Ruby + libvips action and baseline update workflow. ## Compare Any Two Images diff --git a/docs/ci-integration.md b/docs/ci-integration.md index 8e3274cd..3f8f3ec5 100644 --- a/docs/ci-integration.md +++ b/docs/ci-integration.md @@ -36,15 +36,19 @@ Add to your test helper: require 'capybara_screenshot_diff/reporters/html' ``` -### 2. Upload artifacts (manual setup) +### 2. Reusable composite action (recommended) -This is the full YAML so you understand what each step does: +The simplest way — one step handles artifact upload, job summary, and PR comments: ```yaml # .github/workflows/test.yml jobs: test: runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write # Required for PR comments + steps: - uses: actions/checkout@v6 @@ -52,38 +56,6 @@ jobs: with: bundler-cache: true - # Install libvips for the :vips driver (optional — skip if using :chunky_png) - - name: Install libvips - run: sudo apt-get install -y libvips-dev - - - name: Run tests - run: bundle exec rake test - - # Upload HTML report — renders inline in Actions UI (no download needed) - - name: Upload screenshot report - if: failure() - uses: actions/upload-artifact@v7 - with: - name: screenshot-report - path: doc/screenshots/snap_diff_report.html - archive: false - retention-days: 2 - - # Upload full report with images (for offline review) - - name: Upload full screenshot report - if: failure() - uses: actions/upload-artifact@v7 - with: - name: screenshot-report-full - path: doc/screenshots/ - retention-days: 2 -``` - -### 3. Or use the reusable action (one line) - -Instead of the manual upload steps above, reference our composite action directly: - -```yaml - name: Run tests run: bundle exec rake test @@ -92,54 +64,123 @@ Instead of the manual upload steps above, reference our composite action directl uses: snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@master with: name: screenshots + pr-comment: 'true' ``` -This uploads diffs, Capybara failure screenshots, and the HTML report (inline + full) in one step. +That's it. On failure, this will: +- Upload diff images + HTML report as artifacts +- Post a PR comment with links to the inline report and full artifact download +- Add a job summary with report links (visible in the Actions UI) -**Inputs:** +#### Inputs | Input | Default | Description | |-------|---------|-------------| | `name` | (required) | Artifact name prefix | | `report-path` | `doc/screenshots` | Path to HTML report directory | | `retention-days` | `2` | Days to retain artifacts | +| `pr-comment` | `false` | Post PR comment with report link (requires `pull-requests: write`) | -### 4. PR comment with link to report (optional) +#### Outputs -Automatically comment on the PR pointing to the artifact. Uses `find-comment` to update existing comments instead of creating duplicates: +| Output | Description | +|--------|-------------| +| `report-url` | Direct URL to the inline HTML report artifact | +| `report-full-url` | Direct URL to the full report artifact (with images) | -```yaml - - name: Find existing comment - if: failure() && github.event_name == 'pull_request' - uses: peter-evans/find-comment@v3 - id: find-comment - with: - issue-number: ${{ github.event.pull_request.number }} - comment-author: 'github-actions[bot]' - body-includes: 'Screenshot diffs detected' +### 3. Ruby + libvips setup action + +For consistent CI environments (libvips, font antialiasing disabled), use the setup action: - - name: Comment PR with report link - if: failure() && github.event_name == 'pull_request' - uses: peter-evans/create-or-update-comment@v5 +```yaml + - uses: snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@master with: - comment-id: ${{ steps.find-comment.outputs.comment-id }} - issue-number: ${{ github.event.pull_request.number }} - edit-mode: replace - body: | - ### Screenshot diffs detected - [View report](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}#artifacts) + ruby-version: '4.0' + cache-apt-packages: true ``` -**Required:** Add permissions to the job for PR commenting to work: +This installs Ruby, libvips (with apt caching), and disables font antialiasing for consistent rendering across CI runs. + +#### Inputs + +| Input | Default | Description | +|-------|---------|-------------| +| `ruby-version` | (required) | Ruby version to install | +| `cache-apt-packages` | `false` | Cache libvips apt packages for faster runs | +| `ruby-cache-version` | — | Bundler cache version key | + +### 4. Full example with both actions ```yaml jobs: test: + runs-on: ubuntu-latest + timeout-minutes: 10 permissions: contents: read pull-requests: write + + steps: + - uses: actions/checkout@v6 + + - uses: snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@master + with: + ruby-version: '4.0' + cache-apt-packages: true + + - run: bundle exec rake test + + - uses: snap-diff/snap_diff-capybara/.github/actions/upload-screenshots@master + if: failure() + with: + name: screenshots + pr-comment: 'true' ``` +### 5. Manual setup (without composite actions) + +If you prefer full control, here's the expanded YAML: + +
+Expand manual setup + +```yaml +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + + - name: Install libvips + run: sudo apt-get install -y libvips-dev + + - name: Run tests + run: bundle exec rake test + + - name: Upload screenshot report + if: failure() + uses: actions/upload-artifact@v7 + with: + name: screenshot-report + path: doc/screenshots/snap_diff_report.html + archive: false + retention-days: 2 + + - name: Upload full report with images + if: failure() + uses: actions/upload-artifact@v7 + with: + name: screenshot-report-full + path: doc/screenshots/ + retention-days: 2 +``` + +
+ ## Update Baselines in CI When intentional UI changes are made, baselines need to be re-recorded. You can do this locally: @@ -152,6 +193,9 @@ git commit -m "chore: update screenshot baselines" Or add a workflow that maintainers can trigger manually: +
+Expand update-baselines workflow + ```yaml # .github/workflows/update-baselines.yml name: Update Screenshot Baselines @@ -175,12 +219,10 @@ jobs: with: ref: ${{ inputs.branch }} - - uses: ruby/setup-ruby@v1 + - uses: snap-diff/snap_diff-capybara/.github/actions/setup-ruby-and-dependencies@master with: - bundler-cache: true - - - name: Install libvips - run: sudo apt-get install -y libvips-dev + ruby-version: '4.0' + cache-apt-packages: true - name: Record new baselines run: RECORD_SCREENSHOTS=1 bundle exec rake test @@ -195,14 +237,11 @@ jobs: git push ``` +
+ **How it works:** 1. Go to Actions → "Update Screenshot Baselines" → "Run workflow" 2. Enter the branch name (e.g. your PR branch) 3. The workflow records new baselines, commits, and pushes -**Safety:** -- Only maintainers with write access can trigger `workflow_dispatch` -- The commit uses `git diff --staged --quiet ||` to skip empty commits -- `GITHUB_TOKEN` pushes don't trigger subsequent CI runs ([by design](https://docs.github.com/actions/using-workflows/events-that-trigger-workflows)) - [← Back to README](../README.md)