First Feedback

[yahman72]

This is some sort of braindump just to start some discussion i.e. I can open issues and/or PR's based on the outcome of the discussion...

I've been now trying the Library for a couple of days and I'm pretty happy about it. I especially like:

• The simplicity of it, one can indeed get something working very quickly
• The idea of having each Page as it's own RF Library
• The contextmanager handling the page-loading stuff

I chose rather complex Use Case to see what is possible and what is not: "Password Reset" i.e. 1) go to website "X", enter Username and click "Forgot My Password" 2) Open another Browser and login to gmail and open the password reset Email from website "X" 3) login with the new password

This use case has a couple tricky things:

• manage multiple browser sessions (one for the "X" and the other for "Gmail") within one test case
• gmail login has two "subpages" i.e. login prompt and the passwd prompt
• gmail login also involves a lots of redirections (gmail.com --> accounts.google.com --> gmail.com)

The Braindump in no particular order:

• support for "sub pages" (e.g. gmail login: 1) login name page 2) password page) would be nice, i.e. now the contextmanager waits for the old page to go stale AND the new to finish loading, this fails in the gmail case because the login-->passwd transition does not reload the page. Without contextmanager entering passwd fails sometimes because the page is not displayed
• because of the gmail login redirections going to gmail.com lands on the accounts.google.com --> I have to use the 'page_root' with 'go to page' KW, IMHO it would be better if this information is not exposed to the Test Case developer --> additional ROOT_URL variable in the page object would be good (if not set, user se2.getLocation())
• put load timeout behind a variable (override in pageObject itself, and maybe also as RF variable)
• support for multiple browser sessions would be good
• IMHO 'the current page should be' at the end of the 'go_to_page' is not a good idea (additional 'verify=True/False' argument would be nice)
• support for "page transitions" would be great i.e. how to move to the next page without calling 'go_to_page' i.e. after gmail login I'm on the Inbox page, but my Inbox page-object is not loaded automatically. I need to explicitly import the Library or call "Current Page Should Be" that loads the page-object
• The Current Page Should Be should be Current Page Should Be RF KWs in general do not use "The" e.g. The BuiltIn has Variable Should Exist KW, not The Variable Should Exist
• Tutorial could include also one more step after the login (to demonstrate the above page transitions)
• current page check with the title could be made more flexible (one shouldn't be forced to override the default behaviour that often, e.g. the title is "gmail" on the pages = I needed to override it for each pageobject), e.g. check the title with regular expression or even better: someting xpath based that support verification of any element on the page.

[boakley]

Thank you very much for the feedback! You raised some very interesting points.

Here are a few quick thoughts:

Multiple browser support

Multiple browser support should work out of the box using selenium2library's "switch browser" keyword. For example, here's how you can interact with two browsers at once:

| | Open browser | ${ROOT} | firefox | alias=firefox
| | Open browser | ${ROOT} | chrome  | alias=chrome

| | switch browser | firefox
| | Go to page | PageOne
| | The current page should be | PageOne

| | switch browser | chrome
| | Go to page | PageTwo
| | The current page should be | PageTwo

Is that good enough, or do you need something more? I suppose we could add a with or in parameter (eg: go to page | PageTwo | IN | firefox). Does that help, or add unnecessary complexity?

Default implementation of _is_current_page

I don't think I like the idea of making the default current page check more complex, though I'll stay open-minded on it. If you want something different, you can create your own base class for all your pages that does whatever is right for your site.

Sub pages and switching page objects without waiting for page transition

I can see the value in being able to switch to a page object without waiting for a page transition. One concept of a "page object" isn't an object that represents the whole page, but rather a single component or region of a page. An actual page might be made up of several objects. Being able to switch contexts without waiting for a page transition might be very useful.

What if there was a switch to page object keyword (eg: switch to page object | PasswordScreen) that didn't do any waiting or verification, it would simply move that page object to the front of the library search order? Would that work for supporting sub pages and page transitions that don't cause a browser refresh?

Another idea might be a keyword that behaves a bit like run keywords, where the first argument is a page object. For example:

| | with page object | PasswordPage
| | ... | some keyword
| | ... | AND | some other keyword

Adding ROOT_URL

This seems like a bad idea, though maybe I don't understand what you're asking for. Adding a root url to the page object means that page object would only work for that specific root. How would you use the same page object to test both locally and on a staging or production server? You don't want to hard-code an exact url into a generic keyword library.

page should be at the end of go to page

Why don't you want this assert? If you go to a page, and you end up on the wrong page, shouldn't the test assert? Though, adding verify=False seems easy to do. I'll consider that.

Automatic page transitions

You wrote:

after gmail login I'm on the Inbox page, but my Inbox page-object is not loaded automatically.

The very first iteration of this library had this feature. When you do something, it would scan all of the known page objects to try to figure out which page it was on. This required that all page objects be instantiated up front so that you could call _is_current_page on them until one of them returned True. My teams current test suite uses this, and it's annoying because you have to remember to import the page objects you expect to use.

I specifically decided against this approach. When reading tests written by others for a page I was unfamiliar with, I found this approach made the tests hard to read. Consider this fictitious test:

| | go to page | Whatever
| | do something
| | click on something
| | do something else

Now, what page object does do something else belong to? You don't know. You can assume it is Whatever, but what if click on something transitioned automatically to some other page? There's no way to know that.

I decided that an explicit assert was better, because it let the test writer assert their intention, and the test reader to not have to know about which keywords caused transitions. Compare the above to the following code, and notice how it's crystal clear that a transition to another page was expected:

| | go to page | Whatever
| | do something
| | click on something
| | the current page should be | New Page
| | do something else

Again, thanks for your well thought out comments. I really appreciate it!

[yahman72]

More from my side, partially still braindumping without too much consideration whether the things make sense or not...

Before that some background info that hopefully helps to understand why I'm trying to do things in a certain way:

• I'm working with Test teams that have different backgrounds
• There are "Library Developers" that have a programming background
• There are "Test Case Developers" that have hardly any programming background. Similar to a sysadmin that knows everything about a system set-up & configuration (i.e. can do shell scripts, at most)

I.e. the work-split would roughly be PageObjects are implemented by the "Library Developers" (Python) and the Test Cases are implemented by the "Library Developers" and maintained mainly by the "Test Case Developers".

Multiple Browser Support

My idea was to abstract/hide the selenium stuff as much as possible, using Switch Browser inside the Test Case looks somehow too cumbersome. Ideally I'd like to have it so clean that switching browser even for every step in the test case would still be clear. For Example testing browser based chat between Helpdesk agent and an end user:

Send Chat Message | Hello, I have a problem | user_session
Send Chat Message | Hello, how can I help?  | agent_session
Send Chat Message | Test sending a link: this page is broken: http://blabla... | user_session
Send Chat Message | Test multiline chat message: Let me check\nyeah, it's broken  | agent_session
Send Chat Message | Test sending a trouble ticker ID: your trouble ticket ID is #1 | agent_session

In other words: yeah IN or WITH parameter sounds good, or simply a named argument alias=None

As part of the "hide selenium" idea, I'm also hiding the opening of the browser behind a KW that does all necessary steps i.e. open the browser, set proxy if necessary etc. Sticking to the above example opening the browsers would look something like this:

Initialize Browser | alias=agent_session | browser=firefox | page_object=HelpdeskLoginPage | proxy=False
Initialize Browser | alias=user_session  | browser=firefox | page_object=SelfcareLoginPage | proxy=False

Maybe something like this would also be useful for the PageObjectLibrary?

Default implementation of _is_current_page

Well, this is kind of design principle thing:

• offer a simple-to-understand basic implementation and let the users override with custom code vs.
• offer a works-for-most-cases implementation that users can configure to make it work (whilst still offering the possibility to override it)

My approach would be the latter, because I feel that a library should do as much as possible. This of course makes things more complex, because with the XPath-approach one would need to know XPath.

Sub pages and switching page objects without waiting for page transition

First of all, I find it important that all "page transition" scenarios i.e.

• load full page (do stale + readyState checks)
• load sub page (do readyState check)
• select sub page (no checks)

should be integrated with the contextmanager. I.e. after a page transition the user should not need to worry about the page being completely loaded or not.

Both of your proposals sound good but I can't say which one would be better. Maybe both?

Using with page object might end up with really long lines (for example a page object with a lot of fields e.g. registeration with first-name, last-name, email, birth-year, etc. etc.), but is clearer for the contextmanager i.e. all KW's arguments are executed within one context

Using switch to page object looks cleaner in the TC, but the context is not that clear.

Or maybe hide this somehow so that the PageObject knows whether it's a full page or just a sub-component of the page i.e. hide the page structure from the Test Case Developer.

For example, this looks good:

search for | user_name=DummyUser*
switch to page object | UserSearchResults
open record | DummyUser1
switch to page object | UserAccoutnDetails
deactivate user

but this looks even better

search for | user_name=DummyUser*
open record | DummyUser1
deactivate user

But I'm not so sure whether the latter makes sense:

• In some cases it makes the test hard to read, because you don't know on which page you are
• if this would be feasible at all, because the contextmanager stuff might get too complex. In the switch to page object the page object would "know" that it requires the readyState == 'complete' check before proceeding with the next step.

At the moment I'm experimenting with the contextmanager with additional stale_check & readyState_check arguments to handle all above scenarios E.g. that gmail login --> password page transition

    def type_user_name(self, user):
        self._se2lib.input_text("Email", user)
        with self._wait_for_page_refresh(stale_check=False):
            self._se2lib.click_element("next")

Anyway, I'll imlpement more use cases to get more hands-on experiences on what kind of scenarios there could be for the page objects...

Adding ROOT_URL

right, I didn't consider the production vs. test environment point. I was too focused on the redirection. Would be great if both cases could be managed, I have to think about this a bit more ...

page should be at the end of go to page

I was just answering the "should I be calling this keyword?" question in the code. The more flexible the better.

For example: I my case the current implementation became problematic because I was overriding the go to page and page should be with additional alias argument i.e. do a switch browser in my class before calling the KW in the super class which of course failed because the super class go to page calls page should be without the alias argument that I introduced in my sub class.

And for the second question "Should this keyword return true/false, or should it throw an exception?" there I would say, also a new named arugment fail_on_error=True I have been using this approach in my other Libraries, because handling the scenarios where an error is expected the code is easier to read (e.g. ${ret}= | Do Stuff | fail_on_error=False vs. ${ret}= | Run Keyword And Return Status | Do Stuff)

Automatic page transitions

That page object loading wouldn't be a problem in my case, because I would put that kind of logic into my Initialize Browser KW. I.e. the TC developer wouldn't need to know anything about the Page Objects.

The best solution could be to offer both options and leave the final decision to the developer who implements the page objects.

That "scan all of the known page objects to try to figure out which page it was on" logic would be useful also for:

• meaningful error messages i.e. when the is_cuurrent_page fails it would be good to raise an "Expected to be on page X, but page Y was displayed" Exception.
• deciding what to do next. E.g. if something unexpected happened and you should react differently depending on what page you are on. Could be useful in teardown-scenarios where different errors require different actions