tacoss / testcafe-blink-diff

Visual regression for Testcafé
65 stars 15 forks source link

Actual files saved have @2 appended to file name. #57

Closed vlads11 closed 2 years ago

vlads11 commented 2 years ago

Hi BlinkDiff team,

Reaching out for some help and guidance on 2 issues we're experiencing when trying to integrate blinkdiff into our automation repo.

1) Zwift Hub Snapshots ✓ Home page (screenshots: /Tests-Visual/screenshots/vlad/Zwift_Hub_Snapshots/Full_Home_Page/actual@2.png) -- When actual and base images are saved they @2 is appended to the image name and I am not sure why. 2) Issues running blinkdiff test via Jenkins in a docker container, same call in jenkins results in base.png being saved (no @2 appended).

Pretty sure the 2 issues are related but not sure what could be causing the inconsistency with the file-saving system. Any help is much appreciated.

pateketrueke commented 2 years ago

Hi, the @2 is the DPI from the browser that took the screenshots, if no @2 is present that means the DPI on that browser (may be headless?) is just 1, so we're not adding the suffix since it's the default.

Why and for what purpose? That value helps us to determine the DPI from images to be compared, because different DPI yields different image resolutions, so we need to know the DPI from both images to scale them properly and then run the diff.

vlads11 commented 2 years ago

TY @pateketrueke for the explanation on the @2. Learned something new =)

I can see everything working locally now which is awesome but running the our test suite in a docker container via Jenkins still only captures the base.png.

`node ./zwift-blinkdiff.js -e production --browser firefox:headless -s ./Tests-Visual/screenshots/vlad --testMeta.robinTest --take-snapshot actual Executing TestCafe runner. Credentials - Environment data found. Running tests in:

pateketrueke commented 2 years ago

I am curious about your ./zwift-blinkdiff.js script, what it does exactly?

pateketrueke commented 2 years ago

Please, format your code properly or upload it to a gist instead, it's bit hard to read and follow here, thank you!

vlads11 commented 2 years ago

Thanks for the help here: https://gist.github.com/vlads11/4eae63c855a12edefc61bd9493be9520

pateketrueke commented 2 years ago

The custom flags (e.g. --take-screenshot) from this package CLI are not supported by Testcafé itself, in fact, they're hacked and extracted when the package is used on the test files: https://github.com/tacoss/testcafe-blink-diff/blob/master/lib/index.js#L10-L12

If you don't propagate those arguments through the process.argv object, you may try using the process.env variables instead, that way you can tell this module how to behave properly.

vlads11 commented 2 years ago

Thanks for this, will give it a try and see how far I get.

vlads11 commented 2 years ago

Thanks @pateketrueke - The issue was ultimately with process.env TAKE_SNAPSHOT variable that we were over writing in our Jenkins Job Defintiion. 100% appreciate the assist though, the lines of code you shared led me to the right solution and I learned much in the process.