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

Modify Capture Page Screenshot keyword not fail if browser is not open. #816

Closed
aaltat opened this issue Jun 2, 2017 · 6 comments
Closed

Comments

@aaltat
Copy link
Contributor

aaltat commented Jun 2, 2017

For new users, it might be confusing when the Open Browser keyword fails due some invalid configuration and then also the Capture Page Screenshot keyword also fails. It might be good idea not to raise exception if browser is not open when running Capture Page Screenshot keyword

@alexandrul
Copy link

What should it do in this case? Get a screenshot of the desktop (hoping for some helpful message dialogs) or create an empty file?

@pekkaklarck
Copy link
Member

I was initially thinking it should just log a message that a screenshot cannot be taken because browser is not open. Taking a screenshot of the desktop is a valid idea too. Should probably somehow separate that from normal case anyway.

@pekkaklarck
Copy link
Member

The simplest solution could be adding something like this to the beginning of the keyword:

if not self.browser:
    self.info('Cannot take screenshot because browser is not not open')
    return

but, as already discussed, taking a screenshot of the entire desktop might be even better. Either way, I don't think this needs any documentation changes but a new acceptance test is needed.

Ar you @alexandrul interested to look at this?

@aaltat
Copy link
Contributor Author

aaltat commented Sep 13, 2017

Taking a screen shot from a desktop might be problematic, because there might not be a desktop running. Example if using headless chrome or PhantomJS. Also there are other scenarios which might prevent taking a screen shot from a desktop, although it might be running. Example running the browser using the Selenium grid causes some problems.

Perhaps it would be best to split the issue in two parts. First make the change Pekka suggested, because it's easy to do and there is a low risk doing it. Then in separate issue, investigate is it possible to find a solution that would work in most use cases and in cross platform way.

@pekkaklarck
Copy link
Member

It's a good point that it's not always possible to take a screenshot of the desktop. Let's not complicate this unnecessarily and just do the simple thing that avoids the annoying warning. I would even open a separate issue about taking a screenshot of the desktop unless someone really needs such functionality. It's anyway possible to use Robot's Screenshot library for that purpose if needed.

aaltat pushed a commit that referenced this issue Sep 19, 2017
…816) (#899)

* Modified capture page screenshot keyword to not fail when no browser is opened
@aaltat
Copy link
Contributor Author

aaltat commented Sep 19, 2017

The original issue is now done. If there is a need to take a screen capture from the desktop, separate issue should be raised.

@aaltat aaltat closed this as completed Sep 19, 2017
@pekkaklarck pekkaklarck added beta 3 and removed rc 1 labels Sep 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants