Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support absolute path images when converting image to base64 #170

Merged
merged 5 commits into from
Dec 16, 2023

Conversation

kdp88
Copy link
Contributor

@kdp88 kdp88 commented Nov 20, 2023

hey all,

so currently im running into an issue when running a screenshot compare tool https://github.com/simonsmith/cypress-image-snapshot

from debugging the error :

An error was thrown in your plugins file while executing the handler for the after:run event.

The error we received was:

Error: ENOENT: no such file or directory, open 'C:\dev\frontend-e2e-cypress\cypress\screenshots\C:\dev\frontend-e2e-cypress\cypress\snapshots\cypress\e2e\compare-login\login-screen.e2e.cy.ts\__diff_output__\login.d
iff.png'
    at Object.openSync (node:fs:603:3)
    at Object.readFileSync (node:fs:471:35)
    at convertImageToBase64 (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:164:52)
    at C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:147:13
    at Array.map (<anonymous>)
    at createScreenshotsContextList (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:137:22)
    at attachMediaToTestContext (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:86:9)
    at C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:43:24
    at Array.forEach (<anonymous>)
    at C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:36:19
    at Array.forEach (<anonymous>)
    at attachMediaToSuiteTestsContext (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:34:12)
    at C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:17:7
    at Array.forEach (<anonymous>)
    at enhanceReport (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:8:18)
    at mergeAndCreate (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\generateReport.js:19:3)
    at async Promise.all (index 0)
    at async generateReport (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\generateReport.js:64:22)
    at async afterRunHook (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\index.js:35:3)
    at async Object.handler (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\plugin.js:9:5)

i was getting i came to this

my suites [ ] has a context like this (cleaned up a bit for readability)

the screenshot compare tool adds its own directory outside of screenshots that i dont see being configurable

"context": [{
        "title": "cypress-mochawesome-reporter-videos-failed",
        "value": "cypress\\\\e2e\\\\compare-login\\\\login-screen.e2e.cy.ts"
    },
    {
        "title": "cypress-mochawesome-reporter-screenshots",
        "value": [
            "\\\\login-screen.e2e.cy.ts\\\\actions\\\\login\\\\clicking-login.png\",
            "C:\\\\dev\\\\frontend-e2e-cypress\\\\cypress\\\\snapshots\\\\cypress\\\\e2e\\\\compare-login\\\\login - screen.e2e.cy.ts\\\\ __diff_output__\\\\ login.diff.png",
            "\\\\login-screen.e2e.cy.ts\\\\sanity.e2e.cy -- login page - login with email@email.com assert redirect url.com (failed).png"
        ]
    }
]

so when i console out convertImageToBase64()

i get

imagePath = C:\dev\frontend-e2e-cypress\cypress\snapshots\cypress\e2e\compare-login\login-screen.e2e.cy.ts\__diff_output__\login.diff.png
screenshotsDir = C:\dev\frontend-e2e-cypress\cypress\screenshots
imgPath = C:\dev\frontend-e2e-cypress\cypress\screenshots\C:\dev\frontend-e2e-cypress\cypress\snapshots\cypress\e2e\compare-login\login-screen.e2e.cy.ts\__diff_output__\logi
n.diff.png

// above imgPath blows up during:
const contents = 'data:image/png;base64, ' + fse.readFileSync(imgPath, { encoding: 'base64' });

so my thought was to try to see if an image exists in imgPath then do the conversion or just catch the error make it readable serve the error

This isnt really solving the issue but im not sure there needs to be a solution to screenshots being added outside the cypress config var screenshotsFolder

@kdp88
Copy link
Contributor Author

kdp88 commented Nov 27, 2023

@LironEr just bumping to see if you saw this so we can review together

@@ -147,8 +147,15 @@ function createVideoContext(video, baseFolder) {

function convertImageToBase64(screenshotsDir, imagePath) {
const imgPath = path.join(screenshotsDir, imagePath);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like imagePath in your case is an absolute path, then it adds another absolute path, so you can add a check and if the variable is already absolute, use it as it is.

what do you think? it might solve the issue

If you add this part, I think you can revert the other parts, because imagePath won't need to be inside screenshotsDir, and the current NodeJS exception is pretty good.

Suggested change
const imgPath = path.join(screenshotsDir, imagePath);
const imgPath = path.isAbsolute(imagePath) ? imagePath : path.join(screenshotsDir, imagePath);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! yeah makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the code review @LironEr i pushed your comment ! Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LironEr i notice now my latest commit i removed the main part of the function convertImageToBase64() (whoops) i am adding it back but im testing it now and it is still failing for me with

Error: ENOENT: no such file or directory, open '\screenshot-compare.e2e.cy.ts\screenshot-compare.e2e.cy.ts -- users page - assert img (failed).png'
    at Object.openSync (node:fs:596:3)
    at Object.readFileSync (node:fs:464:35)
    at convertImageToBase64 (C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:164:52)
    at C:\dev\frontend-e2e-cypress\node_modules\cypress-mochawesome-reporter\lib\enhanceReport.js:147:13
    at Array.map (<anonymous>)
======imgPath         C:\dev\frontend-e2e-cypress\cypress\snapshots\screenshot-compare\screenshot-compare.e2e.cy.ts\__diff_output__\user list.diff.png
=====path.isAbsolute(imagePath)true


======imgPath     \screenshot-compare.e2e.cy.ts\screenshot-compare.e2e.cy.ts -- users page - assert img (failed).png
=====path.isAbsolute(imagePath)true

Copy link
Contributor Author

@kdp88 kdp88 Dec 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like context.value[] has this

\screenshot-compare.e2e.cy.ts\\\\screenshot-compare.e2e.cy.ts -- users page - assert img (failed).png

which is a valid path.isAbsolute() according to https://www.w3schools.com/nodejs/met_path_isabsolute.asp..... context seems to not pass in the cypress screenshots dir in the path. would the code below be a bad idea?

function convertImageToBase64(screenshotsDir, imagePath) {
  if (fse.pathExistsSync(imagePath)) {
    return convertImg(imagePath);
  } else {
    const imgPath = path.join(screenshotsDir, imagePath);
    return convertImg(imgPath);
  }

  function convertImg(pathToFile)
  {
    return 'data:image/png;base64, ' + fse.readFileSync(pathToFile, { encoding: 'base64' });
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LironEr bump baybay bump

@LironEr LironEr changed the title check the imgPath before readFileSync feat: support absolute path images when converting image to base64 Dec 14, 2023
@LironEr LironEr merged commit 9fcb92d into LironEr:master Dec 16, 2023
1 check passed
@LironEr
Copy link
Owner

LironEr commented Dec 16, 2023

Thanks @kdp88
I will release it in the next few days, just want to wait a bit for another feature to be merged

@kdp88
Copy link
Contributor Author

kdp88 commented Dec 18, 2023

@LironEr Thanks for the help dude!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants