-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Fixed the usage information in documentation of "save_screenshot". #3804
Conversation
"save_screenshot" method was assigned from "get_screenshot_as_file" as result of that it's usage information was "driver.get_screenshot_as_file('/Screenshots/foo.png')" which is wrong. So I defined a separate method which is calling "get_screenshot_as_file" but with it's correct usage information as "driver.save_screenshot('/Screenshots/foo.png')" Fixes SeleniumHQ#3651
As per code contributor's guidelines. I ran the tests using Verbose error message: https://gist.github.com/igauravsehrawat/fc66e8c8822b6c2fa61f31127372f525 Thanks |
@igauravsehrawat Why is aliasing get_screenshot_as_file wrong? It may not be the cleanest code, but I don't see a reason to duplicate code. |
@Dude-x Thanks for replying. @cgoldberg Suggested here #3651 (comment). It was producing wrong documentation because of aliasing. Thanks |
@igauravsehrawat Since proper documentation is more helpful than duplicate code, I'll see if I can get a core committer to make this change. Did you sign the CLA? |
@Dude-x Yes, I did sign the CLA. Thanks a lot. |
@igauravsehrawat can you throw a check in the checkbox for the CLA in your PR comment |
:Usage: | ||
driver.save_screenshot('/Screenshots/foo.png') | ||
""" | ||
return self.get_screenshot_as_file(self, filename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be just
return self.get_screenshot_as_file(filename)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, confused myself with explicit self
logic.
Fixed it.
thanks @igauravsehrawat |
Thank you all for the help. |
"save_screenshot" method was assigned from "get_screenshot_as_file"
as result of that it's usage information was
"driver.get_screenshot_as_file('/Screenshots/foo.png')" which is wrong.
So I defined a separate method which is calling
"get_screenshot_as_file" but with it's correct usage information as
"driver.save_screenshot('/Screenshots/foo.png')"
Fixes #3651
X
in the preceding checkbox, I verify that I have signed the Contributor License Agreement