Skip to content

Commit

Permalink
Update tests, use a diff library
Browse files Browse the repository at this point in the history
  • Loading branch information
davepagurek committed Sep 7, 2024
1 parent 2f7052a commit 190d516
Show file tree
Hide file tree
Showing 18 changed files with 514 additions and 69 deletions.
458 changes: 458 additions & 0 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"i18next": "^19.0.2",
"i18next-browser-languagedetector": "^4.0.1",
"lint-staged": "^15.1.0",
"resemblejs": "^5.0.0",
"rollup": "^4.9.6",
"rollup-plugin-string": "^3.0.0",
"rollup-plugin-visualizer": "^5.12.0",
Expand Down
6 changes: 0 additions & 6 deletions src/image/loading_displaying.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,13 @@ p5.prototype.loadImage = async function(
successCallback,
failureCallback,
(pImg => {
self._decrementPreload();
resolve(pImg);
}).bind(self)
);
}catch(e){
console.error(e.toString(), e.stack);
if (typeof failureCallback === 'function') {
failureCallback(e);
self._decrementPreload();
} else {
console.error(e);
}
Expand All @@ -160,7 +158,6 @@ p5.prototype.loadImage = async function(
e => {
if (typeof failureCallback === 'function') {
failureCallback(e);
self._decrementPreload();
} else {
console.error(e);
}
Expand All @@ -182,14 +179,12 @@ p5.prototype.loadImage = async function(
successCallback(pImg);
}
resolve();
self._decrementPreload();
};

img.onerror = e => {
p5._friendlyFileLoadError(0, img.src);
if (typeof failureCallback === 'function') {
failureCallback(e);
self._decrementPreload();
} else {
console.error(e);
}
Expand All @@ -215,7 +210,6 @@ p5.prototype.loadImage = async function(
p5._friendlyFileLoadError(0, path);
if (typeof failureCallback === 'function') {
failureCallback(e);
self._decrementPreload();
} else {
console.error(e);
}
Expand Down
10 changes: 4 additions & 6 deletions src/typography/loading_displaying.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,11 @@ import '../core/friendly_errors/fes_core';
* </code>
* </div>
*/
p5.prototype.loadFont = function(path, onSuccess, onError) {
p5.prototype.loadFont = async function(path, onSuccess, onError) {
p5._validateParameters('loadFont', arguments);
const p5Font = new p5.Font(this);

const self = this;
opentype.load(path, (err, font) => {
await new Promise(resolve => opentype.load(path, (err, font) => {
if (err) {
p5._friendlyFileLoadError(4, path);
if (typeof onError !== 'undefined') {
Expand All @@ -146,8 +145,7 @@ p5.prototype.loadFont = function(path, onSuccess, onError) {
if (typeof onSuccess !== 'undefined') {
onSuccess(p5Font);
}

self._decrementPreload();
resolve();

// check that we have an acceptable font type
const validFontTypes = ['ttf', 'otf', 'woff', 'woff2'];
Expand All @@ -174,7 +172,7 @@ p5.prototype.loadFont = function(path, onSuccess, onError) {
);
document.head.appendChild(newStyle);
}
});
}));

return p5Font;
};
Expand Down
1 change: 1 addition & 0 deletions src/webgl/loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,7 @@ function parseObj(model, lines, materials = {}) {
model.vertexColors.push(materialDiffuseColor[0]);
model.vertexColors.push(materialDiffuseColor[1]);
model.vertexColors.push(materialDiffuseColor[2]);
model.vertexColors.push(1);

This comment has been minimized.

Copy link
@limzykenneth

limzykenneth Nov 14, 2024

Member

@davepagurek Came across this while working on the test, the test is not expecting this alpha value so just want to make sure this is supposed to be here then I'll add the relevant value to the test?

This comment has been minimized.

Copy link
@davepagurek

davepagurek Nov 15, 2024

Author Contributor

ah did I forget to update a unit test with that? the vertex colors should include alpha, thanks!

This comment has been minimized.

Copy link
@limzykenneth

limzykenneth Nov 15, 2024

Member

It's the loadModel one that was marked as .todo.

}
} else {
hasColorlessVertices = true;
Expand Down
2 changes: 1 addition & 1 deletion test/unit/visual/cases/typography.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ visualSuite('Typography', function() {
p5.createCanvas(50, 50);
p5.textSize(20);
p5.textAlign(p5.LEFT, p5.TOP);
p5.text('test', 0, 0);
p5.text('broken', 0, 0);
screenshot();
});

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
105 changes: 49 additions & 56 deletions test/unit/visual/visualTest.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import p5 from '../../../src/app.js';
import { server } from '@vitest/browser/context'
import resemble from 'resemblejs'
const { readFile, writeFile } = server.commands

/**
* A helper class to contain an error and also the screenshot data that
* caused the error.
*/
class ScreenshotError extends Error {
constructor(message, actual, expected) {
super(message);
this.actual = actual;
this.expected = expected;
}
function writeImageFile(filename, base64Data) {
const prefix = /^data:image\/\w+;base64,/;
writeFile(filename, base64Data.replace(prefix, ''), 'base64');
}

function toBase64(img) {
Expand Down Expand Up @@ -37,45 +33,49 @@ export function visualSuite(
callback,
{ focus = false, skip = false } = {}
) {
const lastPrefix = namePrefix;
namePrefix += escapeName(name) + '/';

let suiteFn = suite;
let suiteFn = describe;
if (focus) {
suiteFn = suiteFn.only;
}
if (skip) {
suiteFn = suiteFn.skip;
}
suiteFn(name, callback);
suiteFn(name, () => {
let lastPrefix;
beforeAll(() => {
lastPrefix = namePrefix;
namePrefix += escapeName(name) + '/';
})

namePrefix = lastPrefix;
callback()

afterAll(() => {
namePrefix = lastPrefix;
});
});
}

export function checkMatch(actual, expected, p5) {
export async function checkMatch(actual, expected, p5) {
const maxSide = 50;
const scale = Math.min(maxSide/expected.width, maxSide/expected.height);

for (const img of [actual, expected]) {
img.resize(
Math.ceil(img.width * scale),
Math.ceil(img.height * scale)
);
}
const diff = p5.createImage(actual.width, actual.height);
diff.drawingContext.drawImage(actual.canvas, 0, 0);
diff.drawingContext.globalCompositeOperation = 'difference';
diff.drawingContext.drawImage(expected.canvas, 0, 0);
diff.filter(p5.ERODE, false);
diff.loadPixels();

let ok = true;
for (let i = 0; i < diff.pixels.length; i++) {
if (i % 4 === 3) continue; // Skip alpha checks
if (Math.abs(diff.pixels[i]) > 10) {
ok = false;
break;
}
}

resemble.outputSettings({ useCrossOrigin: false });
const diff = await new Promise(resolve => resemble(toBase64(actual))
.compareTo(toBase64(expected))
.ignoreAntialiasing()
.onComplete((data) => {
resolve(data)
})
)
const ok = diff.rawMisMatchPercentage === 0;

return { ok, diff };
}

Expand Down Expand Up @@ -103,8 +103,7 @@ export function visualTest(
callback,
{ focus = false, skip = false } = {}
) {
const name = namePrefix + escapeName(testName);
let suiteFn = suite;
let suiteFn = describe;
if (focus) {
suiteFn = suiteFn.only;
}
Expand All @@ -113,9 +112,11 @@ export function visualTest(
}

suiteFn(testName, function() {
let name;
let myp5;

beforeAll(function() {
name = namePrefix + escapeName(testName);
return new Promise(res => {
myp5 = new p5(function(p) {
p.setup = function() {
Expand All @@ -132,20 +133,15 @@ export function visualTest(
test('matches expected screenshots', async function() {
let expectedScreenshots;
try {
metadata = await fetch(
`unit/visual/screenshots/${name}/metadata.json`
).then(res => res.json());
const metadata = JSON.parse(await readFile(
`../screenshots/${name}/metadata.json`
));
expectedScreenshots = metadata.numScreenshots;
} catch (e) {
console.log(e);
expectedScreenshots = 0;
}

if (!window.shouldGenerateScreenshots && !expectedScreenshots) {
// If running on CI, all expected screenshots should already
// be generated
throw new Error('No expected screenshots found');
}

const actual = [];

// Generate screenshots
Expand All @@ -163,34 +159,31 @@ export function visualTest(
);
}
if (!expectedScreenshots) {
writeTextFile(
`unit/visual/screenshots/${name}/metadata.json`,
await writeFile(
`../screenshots/${name}/metadata.json`,
JSON.stringify({ numScreenshots: actual.length }, null, 2)
);
}

const expectedFilenames = actual.map(
(_, i) => `unit/visual/screenshots/${name}/${i.toString().padStart(3, '0')}.png`
(_, i) => `../screenshots/${name}/${i.toString().padStart(3, '0')}.png`
);
const expected = expectedScreenshots
? (
await Promise.all(
expectedFilenames.map(path => new Promise((resolve, reject) => {
myp5.loadImage(path, resolve, reject);
}))
expectedFilenames.map(path => myp5.loadImage('/unit/visual' + path.slice(2)))
)
)
: [];

for (let i = 0; i < actual.length; i++) {
if (expected[i]) {
if (!checkMatch(actual[i], expected[i], myp5).ok) {
throw new ScreenshotError(
`Screenshots do not match! Expected:\n${toBase64(expected[i])}\n\nReceived:\n${toBase64(actual[i])}\n\n` +
'If this is unexpected, paste these URLs into your browser to inspect them, or run grunt yui:dev and go to http://127.0.0.1:9001/test/visual.html.\n\n' +
`If this change is expected, please delete the test/unit/visual/screenshots/${name} folder and run tests again to generate a new screenshot.`,
actual[i],
expected[i]
const result = await checkMatch(actual[i], expected[i], myp5);
if (!result.ok) {
throw new Error(
`Screenshots do not match! Expected:\n${toBase64(expected[i])}\n\nReceived:\n${toBase64(actual[i])}\n\nDiff:\n${result.diff.getImageDataUrl()}\n\n` +
'If this is unexpected, paste these URLs into your browser to inspect them.\n\n' +
`If this change is expected, please delete the screenshots/${name} folder and run tests again to generate a new screenshot.`,
);
}
} else {
Expand Down

0 comments on commit 190d516

Please sign in to comment.