garris / BackstopJS

Catch CSS curve balls.
http://backstopjs.org
MIT License
6.67k stars 605 forks source link

Output format JPG not compatible with Lemur fallback images #1438

Open gruberro opened 1 year ago

gruberro commented 1 year ago

Hello everybody!

We're heavily using the outputFormat: "jpg"-option in large projects, as this helps us keeping the repository (reference files are stored in LFS) smaller. I think we might have discovered a problem with the fallback images (e.g. '/capture/resources/unexpectedErrorSm.png'). These are PNGs, regardless the outputFormat option. So whenever some of these fallback images are stored as a reference, the following test-run results in an exception like:

See: /src/tests/backstop/bitmaps_test/20220929-064330/failed_diff_xxx.jpg
/usr/local/lib/node_modules/backstopjs/core/util/compare/store-failed-diff.js:20
    fs.writeFileSync(failedDiffFilename, data.getDiffImageAsJPEG(85));
                                              ^

TypeError: data.getDiffImageAsJPEG is not a function
    at module.exports (/usr/local/lib/node_modules/backstopjs/core/util/compare/store-failed-diff.js:20:47)
    at /usr/local/lib/node_modules/backstopjs/core/util/compare/compare.js:21:14
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

Even this might be acceptable in the end, right now this error is catched by the test runner and test execution continues/ends with success. When running tests as part of the CI pipeline everything seems pretty ok, and no errors are reported. This error can only be discovered by reading the test entire output, what - at least I think so - usually doesn't happen in an automated ci-env.

IMHO the error should cause the test run to exit with an error code. Having different fallback/error images relying on the outputFormat could help too 😄 . I'm interested in your opinion on it before trying to contribute a possible solution.

gruberro commented 1 year ago

@garris any feedback on this?