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

Send screenshots with test results #37

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

halinaVabishchevich
Copy link

@halinaVabishchevich halinaVabishchevich commented Mar 29, 2022

  1. Add ability to send screenshots for failed tests
    Add method attachScreenshots :
  • For Failed tests find screenshot in screenshots folder
  • Send screenshot using add_attachment_to_result api
  1. If test failed - error message + test commands are added to test Results (if cypress-failed-log is added https://github.com/bahmutov/cypress-failed-log)
    getTestComments function

Result:
2022-06-13 18 28 18

@halinaVabishchevich halinaVabishchevich force-pushed the send-screenshots-with-test-results branch from 967cabb to 6ad12fb Compare March 29, 2022 08:53
…imple into send-screenshots-with-test-results

# Conflicts:
#	package-lock.json
#	src/plugin.js
@bahmutov
Copy link
Owner

bahmutov commented Jun 9, 2022

@halinaVabishchevich Hi, thank you for the pull request, could you resolve the conflicts before I take a look at it, please?

@halinaVabishchevich
Copy link
Author

@halinaVabishchevich Hi, thank you for the pull request, could you resolve the conflicts before I take a look at it, please?

Ready for review

@treyturner
Copy link

As someone whose company does not pay for Cypress Dashboard, this is very much appreciated. Anything I can do to help get this merged?

@jwohlfahrt
Copy link

Plus one to @treyturner 's earlier post. We'd love to get this incorporated, so open to helping in any way. Thanks!

package.json Outdated
@@ -44,6 +44,9 @@
"dependencies": {
"arg": "^5.0.1",
"debug": "^4.3.2",
"find": "^0.3.0",
"form-data": "^4.0.0",
"fs": "^0.0.1-security",
Copy link
Owner

Choose a reason for hiding this comment

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

are you sure you meant to install this 3rd party module fs?

Choose a reason for hiding this comment

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

Hey ! Thanks for the review and sorry for the late response
this one is fixed and also merged with main
ready to be reviewed again

@jwohlfahrt
Copy link

@halinaVabishchevich Anything we can help out with to get this PR over the finish line? Looks like there might just be an extraneous, node-native dependency that can be removed (fs). My team would love to pull this change in to our suites that use this plugin.

…imple into send-screenshots-with-test-results

# Conflicts:
#	package-lock.json
#	package.json
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.

5 participants