Clean up keywords related to alerts #933

Activity

1. pekkaklarck commented on Sep 28, 2017

   

   pekkaklarck

   on Sep 28, 2017

   MemberAuthor

   More actions

   The current situation is so bad that I assign this tentatively to SL 3.0 scope. If there's too much work, it may slip to 3.1. In that case we should at least enhance the docs a little.
2. pekkaklarck

   added this to the v3.0.0 milestone on Sep 28, 2017
3. pekkaklarck

   added

   enhancement

   priority: high

   rc 1

   deprecation

   on Sep 28, 2017
4. pekkaklarck commented on Sep 28, 2017

   

   pekkaklarck

   on Sep 28, 2017

   Last edited by pekkaklarck

   MemberAuthor

   More actions

   My initial proposal for a list of alert keywords is below. It's based on the actions you actually can do with alerts and influenced also by the existing set of keywords and how they behave.

   1. Alert Should Be Present [expected_message=None]: Existing keyword but changed argument name. Old functionality is OK, but could also return the actual alert message. Can also consider adding negative variant Alert Should Not Be Present.

   2. Handle Alert [action=ACCEPT]: New keyword. By default accepts the alert (i.e. presses Ok), but allows dismissing it (i.e. pressing Cancel) by using like action=DISMISS, and could also support action=LEAVE or similar to leave the alert in place. Should return the alert message.

   3. Input Text Into Alert [text, action=ACCEPT]: Mostly the same as the current Input Text Into Prompt but accepts the alert by default. Action can be also be configured using the action argument exactly like with Handle Alert.
5. pekkaklarck commented on Sep 28, 2017

   

   pekkaklarck

   on Sep 28, 2017

   Last edited by pekkaklarck

   MemberAuthor

   More actions

   My proposal above would mean that many of the existing keywords would need to be deprecated and removed later. My proposed actions and reasoning is below. Notice that with term loud deprecation I mean emitting a deprecation warning right now and silent deprecation means documenting that keyword is deprecated but not yet emitting a warning. Loudly deprecated keywords could then be removed already in SL 3.1, and silently deprecated keywords could be loudly deprecated at that point.

   1. Choose Ok/Cancel On Next Confirmation: Totally useless keywords nowadays. Much better to pass the action to the keyword handling the alert as an argument directly. These should be deprecated loudly. Could even consider removing their underlying functionality altogether already at this point, but that would obviously be backwards incompatible change.

   2. Confirm Action: Can work as an alias for Handle Alert but should be silently deprecated. If the functionality of the above keywords is removed, won't work correctly if Choose Cancel On Next Confirmation is used.

   3. Get Alert Message: This keyword currently dismisses the alert by default and can be configured to leave the alert in place with dismiss=False. In my opinion that is really strange behavior, and I'd like to change it so that alerts are accepted by default but it's possible to dismiss or leave them. The problem is that changing the default behavior would be backwards incompatible. Although I'd prefer an explicit keyword for getting the alert message, I don't think it's worth the effort deprecating the current behavior and changing it later. The proposed Handle Alert returns the message, and support everything this keyword should. Thus I think we should just silently deprecate this keyword in favor of Handle Alert.

   4. Dismiss Alert: This keyword currently accepts the alert by default and requires configuration to actually dismiss it. The proposed 'Handle Alert` handles this much better and this keyword can be silently deprecated.

   5. Input Text Into Prompt: This keyword doesn't accept the alert, it cannot even be configured, meaning that Confirm Action or some other keyword needs to be called separately afterwards. Also the name with prompt is a bit weird, although I know this particular alert is created by using window.prompt(). The proposed Input Text Into Alert is consistently named and also accepts the alert by default.
6. boakley commented on Sep 28, 2017

   

   boakley

   on Sep 28, 2017

   Contributor

   More actions

   How about instead of multiple new keywords, you add a single new keyword with a name like With the next alert that takes actions as arguments.

   For example:

       With the next alert
       ...  Click  OK
   
       With the next alert
       ...  Input text  Hello, world
   
       With the next alert
       ...  message should be  Thank you for signing up!
       ...  AND  click ok
   

   Another possible name might be When the alert pops up

       When the alert pops up  Click  OK
       When the alert pops up  Input text  Hello, world
       When the alert pops up  message should be  Thank you for signing up  AND  click  ok
   

   Note: "Click", "input text", "message should be" aren't keywords, though they could have the same as existing keywords I suppose.
7. pekkaklarck commented on Sep 28, 2017

   

   pekkaklarck

   on Sep 28, 2017

   MemberAuthor

   More actions

   Examples by @boakley look nice but I see two problems with them in this context:

   • The style is inconsistent with other kws in this lib.
   • It's a bit strange to say With next alert when the alert is likely to have occurred at that point.

   I wouldn't thus use this style here, but in some other context it could work really well. Also, I propose adding just one totally new keyword Handle Alert.
8. boakley commented on Sep 28, 2017

   

   boakley

   on Sep 28, 2017 via email

   Contributor

   More actions

   The name could be `with the current alert` instead of `with next alert`, to satisfy your second concern. I certainly think it looks better in a test than "Handle Alert", but I realize that's pretty subjective.

   …

   On Thu, Sep 28, 2017 at 9:47 AM, Pekka Klärck ***@***.***> wrote: Examples by @boakley <https://github.com/boakley> look nice but I see two problems with them in this context: - The style is inconsistent with other kws in this lib. - It's a bit strange to say With next alert when the alert is likely to have occurred at that point. I wouldn't thus use this style here, but in some other context it could work really well. Also, I propose adding just one totally new keyword Handle Alert. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#933 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABEmYuAn0CXOUIq4ky3M9y-lunHdYsQJks5sm7F8gaJpZM4Pm4aY> .
9. pekkaklarck commented on Sep 28, 2017

   

   pekkaklarck

   on Sep 28, 2017

   MemberAuthor

   More actions

   Yeah, these examples look nice. But as I said, the style they use is totally different than what other keywords use and I really like consistency. Also, the keywords provided by this library aren't really meant to be used directly on the test case level. They should be used by higher level keywords created as Robot's user keyword or by higher level libraries.
10. pekkaklarck commented on Sep 28, 2017

    

    pekkaklarck

    on Sep 28, 2017

    MemberAuthor

    More actions

    After some more investigation I noticed that some of the things I considered really strange were in fact repressions (#934). At the same time I found some new strange things, like Input Text Into Prompt not accepting the alert automatically. I have updated my earlier comments based on this new insight.
11. aaltat

    closed this as completedin #935on Oct 5, 2017