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

Disable creating .html file, while creating screenshot #359

Merged
merged 1 commit into from Jul 31, 2016

Conversation

Projects
None yet
2 participants
@BorisOsipov
Collaborator

BorisOsipov commented Jul 23, 2016

#358 Disable creating .html file, while creating screenshot

@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Jul 24, 2016

Member

Thanks for the PR!
However, there is one problem. If failed to take screenshot, Selenide returned html (with default settings). Now it does not return html. :(

Member

asolntsev commented Jul 24, 2016

Thanks for the PR!
However, there is one problem. If failed to take screenshot, Selenide returned html (with default settings). Now it does not return html. :(

@BorisOsipov

This comment has been minimized.

Show comment
Hide comment
@BorisOsipov

BorisOsipov Jul 28, 2016

Collaborator

Hi, Andrei!

Yes indeed, now method takeScreenShot doesn't return html.
But it's a rare case when screenshoting fails and we need return smth != null. And I believe end-user should setup its enviroment in these cases, if they want screenshots.
Also, it looks strange: method takeScreenShot() in fact could return path to non-screenshot file.

Nevertheless, if you don't think so, let me know and I change fix.

Thanks,
Boris

Collaborator

BorisOsipov commented Jul 28, 2016

Hi, Andrei!

Yes indeed, now method takeScreenShot doesn't return html.
But it's a rare case when screenshoting fails and we need return smth != null. And I believe end-user should setup its enviroment in these cases, if they want screenshots.
Also, it looks strange: method takeScreenShot() in fact could return path to non-screenshot file.

Nevertheless, if you don't think so, let me know and I change fix.

Thanks,
Boris

@asolntsev asolntsev merged commit 28d87ba into codeborne:master Jul 31, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

asolntsev added a commit that referenced this pull request Jul 31, 2016

@asolntsev

This comment has been minimized.

Show comment
Hide comment
@asolntsev

asolntsev Jul 31, 2016

Member

@BorisOsipov I don't like losing old functionality.
However, it was not well-designed. Error message should show path to both ".html" and ".png" files - see issue 210. So, let's merge current PR as is and implement issue 210.

Thanks for the PR! Merged.

Member

asolntsev commented Jul 31, 2016

@BorisOsipov I don't like losing old functionality.
However, it was not well-designed. Error message should show path to both ".html" and ".png" files - see issue 210. So, let's merge current PR as is and implement issue 210.

Thanks for the PR! Merged.

@asolntsev asolntsev added the feature label Jul 31, 2016

@asolntsev asolntsev added this to the 3.8 milestone Jul 31, 2016

@asolntsev asolntsev self-assigned this Jul 31, 2016

BorisOsipov added a commit to BorisOsipov/selenide that referenced this pull request Nov 23, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment