Handle errors other than Timeout in keyword 'Handle Alert' #1501
Conversation
Raise the current exception only for TimeoutException and add new except block for WebDriverException. robotframework#1500
|
I understand what you have done and I can see the reasoning behind. Only thing to do is to write some test for the chance. What you think, is this easy to test in the unit or in the acceptance tests |
| % secs_to_timestr(timeout)) | ||
| except WebDriverException as e: | ||
| raise AssertionError('An exception occurred waiting for alert: %s' | ||
| % str(e)) |
There was a problem hiding this comment.
str(e) fails if the error contains non-ASCII characters on Python 2. Good news is that str() can just be removed and it ought to work fine.
e alone perhaps looks a bit odd and could be renamed to err at the same time. I think that's used more in SL code base.
There was a problem hiding this comment.
I've made those changes in commit d61a803
Based on comment from @pekkaklarck in robotframework#1501
|
It looks like something broken in the alert handling and test in CI fails. Could you take a look and fix the issue. |
99096db to
1f0ece6
Compare
Acceptance test "Handle Alert when popup window closes" switched window to a popup window, but that window is closed by javascript. Switch back so future tests don't fail.
|
In some of the environments there are constant failures: Unfortunately the log.html is not saved anywhere from the Travis runs. I wonder is this a timing issue and would this be easier to test in the unit level. |
Yes, it would be much easier to troubleshoot if we could see the log.html |
|
oops, didn't mean to close |
6d1cbb1 to
e858356
Compare
23aaeb0 to
cf5176f
Compare
|
Fixed one minor thing and then this ready to go. Thank you for the contribution. |
Handle TimeoutException separately in AlertKeywords._wait_alert
Raise the current exception only for TimeoutException and add new except block for WebDriverException.
#1500