Adding Visual Test Report in Github Actions #7653

Vaivaswat2244 · 2025-03-21T14:28:10Z

Resolves #7618

This PR adds support for generating an HTML report to compare visual test results more effectively. The goal is to improve accessibility and readability of visual regression test outputs.

Changes:

Added a script to generate an HTML report summarizing visual test results.
The report includes expected screenshots and actual screenshots
Updated the workflow to run the added script after completion of tests
Edited the VisualTest.js to save the recorded screenshots to a folder actual-screenshots.
Then the generated html report is uploaded as an artifact.

Screenshots of the change:
This is the screenshot of the html page generated. It will show expected and Actual visuals of this.

@davepagurek @ksen0 Can you have a look at this?
You can go ahead in the Actions under Summary and download the report from artifacts section.

PR Checklist

npm run lint passes
Inline reference is included / updated
Unit tests are included / updated

davepagurek · 2025-03-25T13:09:24Z

Nice work @Vaivaswat2244! I'm a little less familiar with github actions myself, @ksen0 and @limzykenneth, does this look OK to you?

Vaivaswat2244 · 2025-04-24T19:40:25Z

Hello @davepagurek @limzykenneth @ksen0

Just following up on this PR - I'm wondering if there's any additional feedback or if we should move forward with the implementation as is? If you'd prefer to explore alternative approaches first or if we are waiting for the pr05 project, I'd be happy to convert this to a draft PR in the meantime. Otherwise, I'm ready to address any feedback to get this merged.

ksen0 · 2025-04-24T20:16:16Z

Thanks for your patience @Vaivaswat2244 ! I will have time next week (when the GSoC reviews are done:) to review this, though if someone else reviews this before their feedback is welcome. Sorry for the wait.

Vaivaswat2244 · 2025-05-14T05:01:20Z

Hey @ksen0 , can this be reviewed now?

ksen0 · 2025-05-16T06:21:46Z

Hi @Vaivaswat2244 thanks for all the patience on this and for the reminder. It's really nicely done! I've added notes on 3 points below, let me know what you think about all that, if you have any other ideas (or any questions).

The report looks great, especially the filter for "show only failed."

(1) There are false positives for FAIL - these tests passed but are missing images - could you investigate why this happens please?

(2) Very, very minor nice-to-haves - for consistency, use lowercase step names ("upload visual test report"). If it's easy to add, include link to PR or the # in the report html. Feel free to ignore this feedback.

(3) Major - execution conditions: if there is a failing case, then npm test fails and report is not generated, preventing it from being used. This is the desired outcome when tests fail:

In the example above I'm using:

name: Testing

on:
  push:
    branches:
      - main
      - dev-2.0
  pull_request:
    branches:
      - '*'

jobs:
  test:
    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v1
      - name: Use Node.js 20.x
        uses: actions/setup-node@v1
        with:
          node-version: 20.x
      - name: Get node modules
        run: npm ci
        env:
          CI: true
      - name: build and test
        id: test
        run: npm test
        continue-on-error: true
        env:
          CI: true
      - name: Generate Visual Test Report
        if: always()
        run: node visual-report.js
        env:
          CI: true
      - name: Upload Visual Test Report
        if: always()
        uses: actions/upload-artifact@v4
        with:
          name: visual-test-report
          path: test/unit/visual/visual-report.html
          retention-days: 14
      - name: report test coverage
        if: steps.test.outcome == 'success'
        run: bash <(curl -s https://codecov.io/bash) -f coverage/coverage-final.json
        env:
          CI: true
      - name: fail job if tests failed
        if: steps.test.outcome != 'success'
        run: exit 1

I was able to test this by adding an artificial problem into the test suite:

    visualTest("with the default font", function (p5, screenshot) {
      p5.createCanvas(50, 50);
      p5.textSize(20);
      p5.textAlign(p5.LEFT, p5.TOP);
      p5.text("wrong text", 0, 0); // should be 'text' so should defintiely fail
      screenshot();
    });

This failure does show up in the report, but without actual image:

Looks like writeImageFile(actualFilename, toBase64(actual[i])); only runs on success, so in addition to updating the ci-test.yml, please check the logic of the visualTest.js. The goal is to ensure that report should help users see the visual test failures.

(4) [Edited to add:] Additional improvements - could be a separate PR, or can include in this one - could include a diff image, mirroring what visualTest.js provides:

If problem (3) is addressed, happy to merge!

Vaivaswat2244 · 2025-05-17T18:10:47Z

Got it! I reviewed the code and yes currently the report is not generated in case of test failure. This is a logical misunderstanding from my side which I'll fix. Although regarding the false positives I'm not sure, I'll test the new logic and most probably that'll fix the (1) and (3) points. Also I'll update the script for adding diff as well. Thanks for the detailed review on the pr...

Vaivaswat2244 · 2025-05-20T20:53:00Z

hey everyone,
I've tried to fix the above mentioned issues. I've checked the logic of visualTest.js and it now writes actual-screenshots no matter what...
Also I've updated the ci-test.yml file as @ksen0 recommended.
Although I'm stuck on the false fail issue...
This one test "Drawing triangles" passes on all the tests. Still the actual screenshot is not getting written in the actual-screenshots folder, when I've changed the logic and it is working for "intentionally failing test". Would be really helpful if someone helped @ksen0 @davepagurek
I'm continuing to fix that. In the meantime I've commited fix for (3) and (4) for review.

ksen0

Added a few thoughts, thanks for working on it!

ksen0 · 2025-05-26T06:05:09Z

.gitignore

+__screenshots__/
+actual-screenshots/


Minor, didn't notice it before: for consistency with existing __screenshots__ could you rename actual-screnshots to something like __screenshots_actual, please?

Well, the __screenshots__ here are not of the visual tests as they are not in the .gitignore file and are present in the repo.
Though I will change the name of the folder to screenshots_actual if you wish.

No you're right it's not comparable, no worries, please disregard

ksen0 · 2025-05-26T06:10:38Z

test/unit/visual/visualTest.js

@@ -452,6 +458,7 @@ export function visualTest(
          }
        } else {
          writeImageFile(expectedFilenames[i], toBase64(actual[i]));
+          writeImageFile(actualFilename, toBase64(actual[i]));


Also minor: this looks like it might be an accidental duplicate since it's also on line 449

this was written because earlier the command was written in both if and else conditions. Now, I've moved writeImageFile(actualFilename, toBase64(actual[i])); outside the conditions

ksen0 · 2025-05-26T06:18:26Z

test/unit/visual/visualTest.js

@@ -441,9 +441,15 @@ export function visualTest(
        : [];

      for (let i = 0; i < actual.length; i++) {
+        const flatName = name.replace(/\//g, '-');
+        const actualFilename = `../actual-screenshots/${flatName}-${i.toString().padStart(3, '0')}.png`;
        if (expected[i]) {
          const result = await checkMatch(actual[i], expected[i], myp5);


Just an idea: Could the missing actual screenshots be potentially explained by some surprising behavior here? Ie, if you move writeImageFile(actualFilename, toBase64(actual[i])); to above 446, does the issue persist?

you mean moving it out of the if condition... am I right?
If so, tried that as well. Still the same issue.

Vaivaswat2244 added 3 commits March 21, 2025 19:35

Adding Visual Test Report in Github Actions

b10edd3

Updated file name and upload-artifact version

8679a7a

moved the script from test folder to root

4a674e3

ksen0 added Unit Testing p5.js 2.0 labels Mar 25, 2025

report genration fixes and actual-screenshot fixes

a229893

ksen0 reviewed May 26, 2025

View reviewed changes

		__screenshots__/
		actual-screenshots/

Uh oh!

Adding Visual Test Report in Github Actions #7653

Are you sure you want to change the base?

Adding Visual Test Report in Github Actions #7653

Uh oh!

Conversation

Vaivaswat2244 commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

davepagurek commented Mar 25, 2025

Uh oh!

Vaivaswat2244 commented Apr 24, 2025

Uh oh!

ksen0 commented Apr 24, 2025

Uh oh!

Vaivaswat2244 commented May 14, 2025

Uh oh!

ksen0 commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vaivaswat2244 commented May 17, 2025

Uh oh!

Vaivaswat2244 commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ksen0 left a comment

Choose a reason for hiding this comment

Uh oh!

ksen0 May 26, 2025

Choose a reason for hiding this comment

Uh oh!

Vaivaswat2244 May 27, 2025

Choose a reason for hiding this comment

Uh oh!

ksen0 May 27, 2025

Choose a reason for hiding this comment

Uh oh!

ksen0 May 26, 2025

Choose a reason for hiding this comment

Uh oh!

Vaivaswat2244 May 27, 2025

Choose a reason for hiding this comment

Uh oh!

ksen0 May 26, 2025

Choose a reason for hiding this comment

Uh oh!

Vaivaswat2244 May 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Vaivaswat2244 commented Mar 21, 2025 •

edited

Loading

ksen0 commented May 16, 2025 •

edited

Loading

Vaivaswat2244 commented May 20, 2025 •

edited

Loading