RedHatQE / widgetastic.patternfly

Library of Patternfly widgets for Widgetastic.
Other
13 stars 29 forks source link

[RFR] Use XPath instead of Selenium WebElements for FlashMessage locator #116

Closed tpapaioa closed 4 years ago

tpapaioa commented 4 years ago

The FlashMessage class currently uses Selenium WebElements to implement its widget locator:

class FlashMessages(Widget):
    @property
    def messages(self):
        result = []
        msg_xpath = ('.//div[@id="flash_text_div" or '
                     'contains(@class, "flash_text_div")]/div[contains(@class, "alert")]')
        try:

            for flash_div in self.browser.elements(msg_xpath, parent=self, check_visibility=True):
                result.append(FlashMessage(self, flash_div, logger=self.logger))

class FlashMessage(Widget):
    def __init__(self, parent, flash_div, logger=None):
        Widget.__init__(self, parent, logger=logger)
        self.flash_div = flash_div

    def __locator__(self):
        return self.flash_div

This results in warnings logged to cfme.log any time a FlashMessage is read, e.g.,

2019-12-11 10:24:20,477 [I] [cfme] [RequestsView/flash]: Performing exact match of flash messages, text: 'Retirement initiated for 1 VM and Instance from the CFME Database', type: 'success' (.cfme_venv/lib64/python3.7/site-packages/widgetastic_patternfly/__init__.py:315)
2019-12-11 10:24:20,620 [W] [cfme] [RequestsView/flash]: __locator__ of FlashMessage class returns a WebElement! (.cfme_venv/lib64/python3.7/site-packages/widgetastic/widget/base.py:341)

This PR re-implements FlashMessages as a View containing FlashMessage as a ParametrizedView, to use XPath locators instead. Each FlashMessage is indexed by the XPath index of the message within the containing FlashMessages block.

Because the user can dismiss these messages in random order, this XPath index is not necessarily fixed. For example, if there are three messages on the page,

text: "Message A", locator: (msg_xpath)[1] text: "Message B", locator: (msg_xpath)[2] text: "Message C", locator: (msg_xpath)[3]

and the user dismisses the one with text "Message A", then the remaining messages will now have new locators:

text: "Message B", locator: (msg_xpath)[1] text: "Message C", locator: (msg_xpath)[2]

To account for this, FlashMessages.messages yields the individual FlashMessage instances one at a time, and the XPath index of the next message is re-calculated in case of message dismissal.

Views that use FlashMessages will need to be updated since it is no longer a Widget:

- flash = FlashMessages('.//div[@id="flash_msg_div"]')
+ flash = View.nested(FlashMessages)
coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.02%) to 52.815% when pulling dbd4435a8711f1b3f67622238400b72cc23a174e on tpapaioa:flash_message_locator into 3c4571f8337a11ebb9de0d66ccf85cdd8704f05b on RedHatQE:master.