Skip to content
New issue

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

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

Already on GitHub? # to your account

Update cy.screenshot() to accept directory location + filename #1771

Closed
jennifer-shehane opened this issue May 24, 2018 · 8 comments · Fixed by #1858
Closed

Update cy.screenshot() to accept directory location + filename #1771

jennifer-shehane opened this issue May 24, 2018 · 8 comments · Fixed by #1858
Assignees
Milestone

Comments

@jennifer-shehane
Copy link
Member

Current behavior:

The code below saves the screenshot as the name foobarbazquux.png within the screenshotsFolder

cy.screenshot('foo/bar/baz/quux')

Desired behavior:

The above code should create the necessary directories in order for the screenshot to be saved at foo/bar/baz/quux.png within the screenshotsFolder.

@brian-mann brian-mann added this to the 3.0.2 milestone Jun 5, 2018
@jennifer-shehane jennifer-shehane added stage: pending release and removed stage: in progress stage: needs investigating Someone from Cypress needs to look at this stage: proposal 💡 No work has been done of this issue stage: needs review The PR code is done & tested, needs review stage: ready for work The issue is reproducible and in scope labels Jun 27, 2018
@brian-mann
Copy link
Member

Released in 3.0.2.

@egucciar
Copy link
Contributor

egucciar commented Jul 2, 2018

@jennifer-shehane @brian-mann how can I disable the automatic folder placement for screenshots? since I have custom commands which store images and move them into a specific folder, please let me know the option i need to pass to disable this feature temporarily while I evaluate the new screenshot capabilities and work on solving the matching. Thanks!

EDIT: Nevermind. was fairly simple to solve for!

image

@brian-mann
Copy link
Member

brian-mann commented Jul 2, 2018

@egucciar I wouldn't hard code its path there - thats the problem. I would use the new after:screenshot event in your pluginsFile so you can do whatever it is you want to do in node. That would avoid the need to ever need cy.exec here.

Alternatively you could use the onAfterScreenshot callback that cy.screenshot() takes, which will always include the path to the screenshot, but then you'd still need the cy.exec().

The after:screenshot event is ideal for you as you can do whatever you'd like to do in node - it'll be much faster and it will avoid the need for extra commands which are orthogonal to the cy.screenshot() command.

@egucciar
Copy link
Contributor

egucciar commented Jul 2, 2018

@brian-mann the issue is that this is not a plugin but rather a Cypress.command. the way I understood is commands go into support and plugins go into plugins. Would i be able to write a command such that I can actually invoke a cy.matchScreenshot ?

the onAfterScreenshot sounds like a good option for future-proofness, though. So once I'm able to get the MVP for the diffing algorithm out ill look into that refactor, as this is working fine for now :P

@brian-mann
Copy link
Member

No, but your matchScreenshot command is simply doing something like cy.exec which is just invoking a child process. You could do all of that easier / better from directly inside of node.

@egucciar
Copy link
Contributor

egucciar commented Jul 2, 2018

@brian-mann the question is can we make an assert & fail a test from a plugin ? Because the idea is to run assert on the results of the diffing algorithm.

@brian-mann
Copy link
Member

Yes every plugin event is promise aware so if you reject a promise or throw then the cy.screenshot will fail and the error will propagate as the source of the failure.

@egucciar
Copy link
Contributor

egucciar commented Jul 2, 2018

Very cool! Thanks! Will definitely look into this solution.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants