Conversation
This is same behavior as in 1.8. Also added a FIXME about totally stupid tests inside suite setup (which did actually reveal this small regression...).
1. Introduce new keywords: Alert Should Not Be Present Handle Alert Input Text Into Alert 2. Add new `action` argument to Alert Should Be Present. 3. Silently depracate all other existing alert related keywords. 4. Change alert polling logic to use WebDriverWait only. Timeout will be made configurable later. 5. Documentation cleanup.
aaltat
left a comment
There was a problem hiding this comment.
I did read the code but didn't have time to read the test when commuting. Also would like to take a look from the bigger screen.
But generally looks OK.
| "'%s' but was '%s'" | ||
| % (text, alert_text)) | ||
| try: | ||
| alert = self._wait_alert(timeout=0) |
There was a problem hiding this comment.
Why the timeout is zero? It would make the tests more stable if we allow small polling.
There was a problem hiding this comment.
We expect that there's no alert so in the common case any polling would slow down execution unnecessarily. Very small timeout could be considered, but at least using the global timeout that's 5 seconds by default would be a very bad idea. I'd leave the default timeout here to be zero and just make the timeout configurable with an optional argument.
| versions the alert was always accepted. | ||
| """ | ||
| message = self.handle_alert(action) | ||
| if text and text != message: |
There was a problem hiding this comment.
Now the text is exact match, but I was thinking could if text and text not in message: be also useful. Just a random thought, without a real use case.
There was a problem hiding this comment.
Yeah, if I had designed this I'd probably supported glob-style patterns (e.g. foo *) similarly as Run Keyword And Expect Error does. Don't think changing existing logic is a good idea, though. Could add pattern=False, but that's a topic for a separate issue. It's anyway already possible to do, for example:
${text} = Alert Should Be Present
Should Match ${text} foo *
There was a problem hiding this comment.
True. And alerts are pretty simple in any case.
Also cleanup is_truthy/falsy tests.
|
Again I did read the code but didn't have time to read the test when commuting. Will read rest of the PR later. |
This makes it a lot easier to see failed tests. Would have changed run_tests.py, but we still want to support RF 2.8 where --dotted isn't supported.
This PR is contains following changes to alerts and to some other (un)related functionality:
Dismiss AlertandConfirm Actioncompared to 1.8 #934.is_noneyutility and related small fix to wait keywords.--dottedwhen possible to ease seeing failed tests.This PR is now ready for final review.