A refactoring writeup

Part of what I am doing for a client right now is going through their scripts and doing ‘illustrative refactoring’ (a term I just made up now to describe selectively refactoring scripts to show an idea or concept). Here is one from last week…


I picked one of the failing scripts from the homepage class and started investigating a bit. It looked something like:

@pytest.marks('all', 'deep', 'home')
def test_footer_about_us_link(self):
    url = self.home_page.click_portal_about_us_link()
    self.assertEqual("http://client.com/about/", url)

A couple things stand out here.

  • The first line calls an action on a PO that will produce a new window with some content yet returns a string? The action results in a new page so should return a new PO
  • The url is also production; which is more a comment about the app rather than the automation. Somewhere on my list of application ‘rules’ is ‘thou shalt not leak into a different environment’ — and this is clearly a leak


I can’t to anything about the second thing, but I did a hefty refactoring of this particular test method and this is what is not checked in.

@pytest.marks('all', 'deep', 'home')
def test_footer_about_us_link(self):
    about_page = self.home_page.click_portal_about_us_link()
    self.assertTrue(about_page.content_title == "About")

It still isn’t really a ‘useful’ script per se, but the underlying implementation is much better.

Let’s start with click_portal_about_us_link


def click_portal_about_us_link(self):
    return str(self.se.get_location())


def click_portal_about_us_link(self):
    from pages.AboutPage import AboutPage
    return AboutPage().wait_until_loaded()

The big difference here is that it is no longer returning the url of the original window; which will be wrong everywhere except in production (where it becomes the equivalent of assertTrue(True)). Instead it is returning a PO — as it should since that is what happens as a result of clicking that link.

The import is there to get around the circular dependency of inheriting from ClientBasePage but being created in it.

The return pattern is one I’ve started using recently. Here is the entire PO.

locators = {
    "content title": "css=.contentText h2"

window_title = "Client App"

class ContentTitle(Text):
    def __init__(self):
        self.locator = locators["content title"]

    def __get__(self, obj, cls=None):
        wrapper().connection.select_window("title=%s" % window_title)
        return super(ContentTitle, self).__get__(obj, cls)

    def __set__(self, obj, val):

class AboutPage(ClientBasePage):
    Page Object to deal with the about popup
    content_title = ContentTitle()

    def __init__(self):
        self.se = wrapper().connection

    def wait_until_loaded(self):
        ClientSynchro.wait_for_element_available(locators["content title"])
        return self

The chaining is possible because of the ‘return’ self at the end of the wait_until_loaded method.

I also had to add some more wait_* stuff to ClientBasePage to deal with the popup window synchronization.

def wait_for_popup_window_by_title(self, title):
    return self._wait_for_popup_window(title)

def _wait_for_popup_window(self, title = None):
    if title:
        for i in range(timeout_seconds):
                self.se.select_window("title=%s" % title)
            except saunter.exceptions.WindowNotFound:
            raise ClientException.ElementVisiblityTimeout("waiting for a window with a title of '%s' timed out" % title)
        return True

Notice as well the various self.se.select_window() commands. In RC there is no way of knowing what the active window is so I’d rather err on the side of caution and change windows too often than not enough and get a weird sync error. Note however that I don’t leave the window selected in the wait_*. Sometimes that the window appeared could be enough.


Page Objects are incredibly useful. But only if you use them ‘correctly’ — to some definition of correctly. In this case the ‘incorrect’ aspect was that an action that resulted in a new page wasn’t returning a PO which required a bit of work to address but the resulting script is much nicer. I think at any rate.

Post a Comment

Your email is never published nor shared. Required fields are marked *