-
Notifications
You must be signed in to change notification settings - Fork 763
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
No Fail For Capture Page Screenshot When No Browser Is Opened (Issue #816) #899
Conversation
Please also write a test and use status checker [1] to verify message [1] https://github.com/robotframework/statuschecker/blob/master/README.rst |
Not sure how to setup a test for this. If I close the browser, it will break other tests since they rely on the global suite setup to open their browser. Two ideas: 1) Call 2) Simply add a Thoughts? |
The first option is best. If I recall correctly, there are other test that do the similar thing. |
I've added an acceptance test, and according to Travis it passes, but the build fails. The log doesn't really help, I can't seem to find what crashes. |
# we got an error, just exit | ||
self.info("Couldn't capture page screenshot because no browser is opened") | ||
return | ||
|
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.
Comments and empty lines are unnecessary and somewhat against the style used elsewhere in this codebase. I'd also say it's clearer to just have
try:
self.browser
except RuntimeErrror:
# ...
than
try:
b = self.browser
except RuntimeErrror:
# ...
when the b
variable isn't used for anything. Could also consider using try/except/else
instead of returning from except
.
It might actually also be a good idea to create new BrowserNotOpen
exception and the browser
property to use it instead of the generic RuntimeError
. The code here would then look like
try:
self.browser
except BrowserNotOpen:
# ...
which would be really clean. Not sure is it worth it, though.
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.
Good suggestions. I'll get back to this on Monday
The failure is most likely caused by the Status checker tool finding a problem when it does post-execution analysis. Do you get that failure also locally? |
Here we go :) |
The keyword
Capture Page Screenshot
will no longer fail, and simply return with a message when no browser is opened.