Saturday, June 4, 2016

Proper Page Modeling - In depth

In a previous article, I tried to explain why the page object pattern that I have seen described is simply lacking from what it could be.

Let's start with the good things about page objects.

1.  They are an abstraction of what the page should do
2.  They provide an implementation agnostic approach to testing (don't care if wpf, html, etc)
3.  They allow for more easily readable, and therefore more maintainable, code
4.  They provide a easy to understand navigation pattern where the result of actions are the next thing to work with

Ok, great.  How is this accomplished.  Let's look at an example in depth and compare what I've seen as 'traditional' page objects and compare to my approach to page modeling.

Let's take two pages into account.  Login page and Account Settings page.




When you login successfully, you'll go to the account settings page.

In code, your traditional page objects may look something like this.

class LoginPage : Page
{
    /// Logs in using the credentials supplied
    AccountSettings Login(string username, string password, bool rememberMe);
    RegisterPage ClickRegisterLink();
    ForgotPasswordPage ClickForgotPassword();
    AccountSettings LoginWithTwitter(string twittername, string twitterpass);
    AccountSettings LoginWithFacebook(string facebookname, string facebookpass);
}

I think this much we can agree upon.  Now, where it starts to fall apart for me.  Here is a quote from uncle bob martin.

There are differences of opinion on whether page objects should include assertions themselves, or just provide data for test scripts to do the assertions. Advocates of including assertions in page objects say that this helps avoid duplication of assertions in test scripts, makes it easier to provide better error messages, and supports a more TellDontAsk style API. Advocates of assertion-free page objects say that including assertions mixes the responsibilities of providing access to page data with assertion logic, and leads to a bloated page object.
I favor having no assertions in page objects. I think you can avoid duplication by providing assertion libraries for common assertions - which can also make it easier to provide good diagnostics. 

I agree with no assertions in page objects.  He makes an aside that I also agree with:
One form of assertions is fine even for people like me who generally favor a no-assertion style. These assertions are those that check the invariants of a page or the application at this point, rather than specific things that a test is probing. 
So, I know that there are more things about this page like, is the login button enabled when no username or password is present.  Here are two ways I can extend this class:

class LoginPage : Page
{
    bool IsValid();
}

In this case, how do I know what is not valid so I can fix it?  Also, what does the logic look like for this method?

public bool IsValid()
{
    if(String.IsNullOrEmpty(username.Text) || String.IsNullOrEmpty(password.Text))
    {
        if(loginButton.Enabled)
        {return false;}
    }
    else
    {
        if(!loginButton.Enabled){ return false; }
    }

    // .. maybe more tests for other requirements?
    // what about this?
    // these are requirements for the page that these elements are visible
    if(!IsRegisterLinkVisible()) { return false; }
    if (!IsForgotPasswordLinkVisible()) { return false; }

    // and that they work?
    var registerHref = GetRegisterHref();
     using(var client = new HttpClient())
     {
         var result = await client.GetAsync(registerHref);
          if(!result.IsSuccessful) { return false; }
     }
    return true;
}

Or, I could just throw an exception so I can include detail about what is wrong:

class LoginPage : Page
{
    void ThrowIfNotValid();
}

but this is not good either.  I can't just ask the page if it's valid with out a try block.

So, I can expose all of the things I'm expecting?

class LoginPage : Page
{
    bool IsLoginButtonProperlyEnabledOrDisabled();
}

and then maybe a master IsValid that calls these and possibly throws an exception.

Now, say that the button is not supposed to be disabled, but it is supposed to be 'inactive', but still clickable to trigger form validation.

Ok, so I have to either update this method to

class LoginPage : Page
{
    bool IsLoginButtonPropertlyInactiveOrActive();
}

which is crazy.  Or, I can not rename the method and change the logic so that it doesn't really match.

Another approach to this would be just add methods for each thing you want to test.  Great,

class LoginPage : Page
{
    bool IsLoginButtonEnabled();
    LoginPage SetUsernameAndPassword(string user, string pass); // don't login, just set
}

but when requirement changes to inactive,

class LoginPage : Page
{
    bool IsLoginButtonActive();
}

So, requirements change again.  The password is only visible when a username with 6 or more characters is present.  Great, more methods.

class LoginPage : Page
{
    bool IsPasswordVisible();
    LoginPage SetUsername(string user);
}

To me, this is clearly a violation of open / close principal.  Especially since this is highly predictable.  UI elements have a natural set of consistent things you may want to do with them.  Testing UI elements for visibility, click-ability, enabled-ness, ... are extremely natural concerns and should be consistently available for all UI elements.

We'll look at crafting tests after the comparison with my approach to page modeling.

In my approach to page modeling, I strive for consistency of expression of the UI elements that create the control.  In my approach, the model looks like this:

// because of this inheritance, the login page is also testable for isvisible, exists, ...
interface ILoginPage : IPageModel
{
    // this type of method doesn't even belong here.  it belongs at some other level of abstraction
    // what a 'Login' action means is not any concern of this control
    //AccountSettings Login(string username, string password, bool rememberMe);

    // instead, expose the components that allow a login to happen
    IReadWriteValuePageModel<string, ILoginPage> Username {get;}
    IReadWriteValuePageModel<string, ILoginPage> Password {get;}
    ISelectionablePageModel<ILoginPage> RememberMe {get;}
    IClickablePageModel<IAccountSettings> Login {get;}

    // expose an object that supports click, and all native ui tests (is visible, enabled, ...)
    IClickablePageModel<IRegisterPage> Register {get;}
    IClickablePageModel<IForgotPasswordPage> ForgotPassword {get;}
    IClickablePageModel<ITwitterLoginPage> Twitter {get;set;}
    IClickablePageModel<IFacebookLoginPage> Facebook {get;set;}
}

Another interesting this happens here.  What happens if the facebook login button is on multiple pages.  The LoginWithFacebook(string facebookname, string facebookpass) logic has to be duplicated to all the pages that use it or some helper class is created to share logic.  Again, this belongs at a different level of abstraction for the testing.

To enable these scenarios, you could do something like:

class LoginPageUserActions
{
    protected ILoginPage LoginPage{get;}

    public LoginPageUserActions(ILoginPage loginPage)
    {
         this.LoginPage = loginPage;
    }

    public AccountSettings Login(string username, string password, bool rememberMe)
    {
         // even here, just assume that login works, no need for assertions
         // there should be tests to verify that login works
        return  this.LoginPage
                         .Username.SetText(username)
                        .Password.SetText(password)
                        .RememberMe.SetSelected(rememberMe)
                        .Login.Click();
    }
}

Now, lets compare how you write the tests for the above business requirements and implement the changes in both approaches.

Business Requirements:

1.  Main Login Control
-> Username, Password, and Remember Me all present and enabled
-> Login button visible, but disabled until username and password are set

2.  External Login Control
-> Facebook and Twitter links are present and enabled always

3.  Register Link
-> Present and enabled always

4.  Forgot password
-> Present and enabled always

5.  Upon successful login, the user lands on account settings page.

So the start of the test class would look like this:

public class LoginPageTests()
{
         protected BrowserWindow window;

         [TestInitialize]
         public void GivenLoginPage()
         {
              this.window = BrowserWindow.Launch("loginpageurl");
         }
}

I like to follow the Given, When, Then type of syntax where the Given is my test initialize as the arrange, the When and Then are part of the TestMethod as the act and assert.

To add the first business requirement for the main login control, we run into the first issue that I mentioned.  I could have an IsValid method that captures all this logic, or maybe an IsMainLoginControlValid method that only tests that part of the control.  Seems wrong for the model to do the testing; I would have thought the test class decides what IsValid means*. (* full disclosure, I violate this run on very rare occasion)

So maybe the test looks like:

public void ThenUsernamePasswordAndRememberMeVisibleAndEnabled()
{
    Assert.IsTrue(new LoginPage(this.window).IsValid());
}

Granted, it's short and sweet, it doesn't really tell me much.  And the next test would look the same:

public void ThenTheLoginButtonIsNotEnabled()
{
    Assert.IsTrue(new LoginPage(this.window).IsValid());
}

And finally some actual work:

public void WhenUsernameAndPasswordEntered_ThenLoginButtonIsEnabled()
{
    var loginPage = new LoginPage(this.window);
    Assert.IsTrue(loginPage.IsValid());
    loginPage.SetUsernameAndPassword("user", "pass");
    Assert.IsTrue(loginPage.IsValid()); 
}

Aside from the method name, I really have no idea what this would be testing for :)

Ok, so ditch the is valid approach and expose the methods of interest:

public void ThenUsernamePasswordAndRememberMeVisibleAndEnabled()
{
    var loginPage = new LoginPage(this.window);

    // add all these methods to the class first ~_~
    Assert.IsTrue(loginPage.IsUsernameVisible() && 
    loginPage.IsUsernameEnabled() &&
    loginPage.IsPasswordVisible() &&
    loginPage.IsPasswordEnabled());

    // maybe more simply, add these instead and hope you don't need to test them separately
    Assert.IsTrue(loginPage.IsUsernameVisibleAndEnabled() && loginPage.IsPasswordVisibleAndEnabled()); // still not great :/
}

public void ThenTheLoginButtonIsNotEnabled()
{
    Assert.IsFalse(loginPage.IsLoginButtonEnabled());
}

public void WhenUsernameAndPasswordEntered_ThenLoginButtonIsEnabled()
{
    loginPage.SetUsernameAndPassword("user", "pass");
   Assert.IsTrue(loginPage.IsLoginButtonEnabled());
}

To implement the next requirements, I would do the same thing = add tons of methods for testing the UI state, which are business requirements.  I'll skip 2, 3, and 4 for this reason.  You can imagine what that looks like.

The last requirement would involve the next page's model.  Presently, there is no way to test if the page is actually the page displayed to the user.  So maybe that should be in the isvalid method.  Or maybe it should be an IsPageVisible() method on the page object.

public void WhenValidCredentials_ThenAccountSettingsIsShown()
{
    var loginPage = new LoginPage(this.window);
    var accountPage = loginPage.Login("user", "pass");
    
    Assert.IsTrue(accountPage.IsPageVisible());
    Assert.IsFalse(loginPage.IsPageShown()); // there is no enforcement on consistency and someone named it differently...
}

Ok, so now the requirement changes.  

1.  Login button is not supposed to be disabled, it is supposed to be inactive and validation error is shown, if any, when clicked.
2.  Password is only visible if username with 6 or more characters is present.

In the case of IsValid, we'd go into the method and change the logic to reflect this, shuffle some test names around and add one test method:

// method name change
public void ThenUsernameAndRememberMeVisibleAndEnabledPasswordHidden()
{
    // body doesn't change as it doesn't really express much 
    Assert.IsTrue(new LoginPage(this.window).IsValid());
}

// method name change
public void ThenTheLoginButtonIsInactive()
{
    Assert.IsTrue(new LoginPage(this.window).IsValid());
}

public void WhenUsernameWithSixOrMoreCharacters_ThenPasswordIsShown()
{
     var loginPage = new LoginPage(this.window);
    Assert.IsTrue(loginPage.IsValid());
    loginPage.SetUsername("username");
    
     // still not an overly expressive test :/
    Assert.IsTrue(loginPage.IsValid()); 
}

In the case of exposing tons of methods for everything, you would change the IsEnabled methods to IsActive for the login button.  Not much different from the previous.

Using page models, lets do the same exercise.  Firstly, because of this approach, there is no ambiguity as to what methods to add to your UI.  Simply, you expose whatever UI elements are present and that is enough for the test to drive (or some middle, orchestration abstraction).

public void ThenUsernamePasswordAndRememberMeVisibleAndEnabled()
{
    var loginPage = new LoginPage(this.window);
     // more expressive about what is valid means for a given set of inputs (in this case, no inputs)
    Assert.IsTrue(loginPage.Username.IsActionable() && loginPage.Password.IsActionable() && loginPage.RememberMe.IsActionable());
}

public void ThenLoginButtonIsDisabled()
{
    Assert.IsTrue(new LoginPage(this.window).Login.IsNotEnabled())
}

public void WhenUsernameAndPasswordEntered_ThenLoginButtonIsEnabled()
{
    Assert.IsTrue(loginPage.Username.SetText("user")
                                        .Password.SetText("pass")
                                        .Login.IsEnabled())
}

public void WhenValidCredentials_ThenAccountSettingsIsShown()
{
    var loginPage = new LoginPage(this.window);
    // this method chain to login can be shared easily (see below)
    var accountPage = loginPage.UserName.SetText("user").Password.SetText("pass").Login.Click();
    
    Assert.IsTrue(accountPage.IsVisible());
    Assert.IsFalse(loginPage.IsVisible()); // enforcement on consistency
}

Notice, when writing these tests, I did not need to add anything to my model.  Also, there is room for a shared set of actions that are higher level that setting values and clicking things.  

public void WhenValidCredentials_ThenAccountSettingsIsShown()
{
    var loginPage = new LoginPage(this.window);
    var loginActions = new LoginPageActions(loginPage);
    var accountPage = loginActions.Login("username", "pass");
    
    Assert.IsTrue(accountPage.IsVisible());
    Assert.IsFalse(loginPage.IsVisible()); // enforcement on consistency
}

The underlying point I'm trying to make here is that you may want to very rigorously test the login page details in the login page tests, and then assume that login works in the shared code which does not need to be rigorous in assertions about page state.  This is probably my biggest gripe with page object pattern that I've seen explained.  It is not the page object's concern to know all these things.  It only stores values that may or may not be read and exposes hooks to click on things or other user actions.

So, to make the same requirement changes, nothing changes in the model.  Only the test for that requirement changes.

public void ThenUsernameAndRememberMeVisibleAndEnabledPasswordHidden()
{
     var loginPage = new LoginPage(this.window);
     // more expressive about what is valid means for a given set of inputs (in this case, no inputs)
    Assert.IsTrue(loginPage.Username.IsActionable() && loginPage.Password.IsNotRendered() && loginPage.RememberMe.IsActionable());
}

// method name change
public void ThenTheLoginButtonIsInactive()
{
    Assert.IsTrue(new LoginPage(this.window).Login.IsNotActive());
}

public void WhenUsernameWithSixOrMoreCharacters_ThenPasswordIsShown()
{
     var loginPage = new LoginPage(this.window);
     loginPage.Username.SetText("abc");
     Assert.IsTrue(loginPage.Password.IsNotRendered());
     loingPage.Username.SetText("userna");
     Assert.IsTrue(loginPage.Password.IsRendered());
}

Another point that falls out of this example is that in the traditional page objects approach, I had a Login(string username, string password, bool rememberMe) method for setting all three inputs and clicking login.  I had to add another method to only set username and password so I can test the login button visibility.  Then I had to add one to set only user name to test password visibility when requirement changed.

In this case, nothing had to be changed.  You can expect to want to test input boxes for visibility and to set and read text from them.

Now, we add in the requirement for the validation messages to be shown when login is clicked, but username or password not set.

In the traditional approach, we have to add a method for clicking the button (since we're not doing a login, but trying anyway; maybe the Login method allows empty string as input, or maybe it throws; so implementation details are bleeding out if I assume something about it).  Or a method to explicitly clear username or password and click ClickLoginWithoutUsernamePassword();

class LoginPage : Page
{
    // notice this return type is ambiguous.  If i click after SetUsernameAndPassword,
   // I would actually go to AccountSettingsPage, but I'm adding to satisfy test requirement
   // of click to fail
    LoginPage ClickLoginButton();
    AccountSettings ClickLoginButton2();

    LoginPage ClickLoginWithoutUsernamePassword(); // not ambiguous

    bool IsValidationMessageShown(); // maybe IsCorrectValidationMessageShown()?
     string GetValidationMessageText();
    // or worse
    bool IsExpectedValidationMessage(string message);
}

Notice, I have to add two methods for validation message:  Is it shown (not consistent naming) and what is the value of the text shown.  If I want to know more about the message, more methods!

Using page modeling, you get the login button click for free and just need to add a property for the validation message.

public ILoginPage : IPageModel
{
    IValuedPageModel<string, ILoginPage> ValidationMessage {get;}
}

and the tests for both:

// Using is valid is still not interesting
public void WhenNoUsernameAndLoginClicked_ThenValidationMessageShown()
{
    Assert.IsTrue(loginPage.IsValid());
    loginPage.Setusername("User");
    var page = loginPage.ClickLoginButton();

    Assert.IsTrue(loginPage.IsValid());
}

public void WhenNoUsernameAndLoginClicked_ThenValidationMessageShown()
{
      Assert.IsFalse(loginPage.IsValdationMessageShown());
      loginPage.SetUsername("username").ClickLoginButton();
      Assert.IsTrue(loginPage.IsValidationMessageShown());
       Assert.IsTrue(loginPage.GetValidationMessage().Equals(expectedMessage));
}

Again, very inconsistent methods and requirement to constantly add stuff to the page model.  With page modeling:

public void WhenNoUsernameAndLoginClicked_ThenValidationMessageShown()
{
      Assert.IsFalse(loginPage.ValidationMessage.IsRendered());
      loginPage.SetUsername("username").Login.Click();
      Assert.IsTrue(loginPage.ValidationMessage.IsRendered());
       Assert.IsTrue(loginPage.ValidationMessage.Text.Equals(expectedMessage));
}

Edit: Added examples for changing login method to require a pin

So, I was challenged that this approach is not maintainable considering the example of login changing to require a pin instead of a password.

I will show how page modeling is less affected by this change than page objects in this case.

Regardless, this is a large change.  In the case of page objects, I would update my login page as:

class LoginPage : Page
{
    // AccountSettings Login(string username, string password, bool rememberMe);
    AccountSettings Login(string username, int pin, bool rememberMe);

// all the IsPinVisible, IsPinEnabled methods have to replace the password ones
// and any orchestration method that uses password
    // bool IsPasswordVisible();
   bool IsPinVisible();

    // bool IsPasswordEnabled();
    bool IsPinEnabled();

   // there is nothing like this in page modeling to update since it belongs in a test
   // SetUsernameAndPin(string username, int pin);
}

This change requires EVERY test that logs in to change.  There is no avoiding this as the password type changed from string to int so at a minimum you'll have to make that change.

This is also true for the page modeling abstraction, but in a different way.  The orchestration layer is affected, and the tests for the login page it self are affected.

class LoginPageUserActions
{
    protected ILoginPage LoginPage{get;}

    public LoginPageUserActions(ILoginPage loginPage)
    {
         this.LoginPage = loginPage;
    }

    public AccountSettings Login(string username, int pin, bool rememberMe)
    {
         // even here, just assume that login works, no need for assertions
         // there should be tests to verify that login works
        return  this.LoginPage
                         .Username.SetText(username)
                        .Pin.SetValue(pin)
                        .RememberMe.SetSelected(rememberMe)
                        .Login.Click();
    }

    // there is no need for an orchestration method like SetUsernameAndPassword
    // it doesn't help anything and is added to page object purely to provide functionality for testing
}

This is exactly the same change that was required by page object.  However, the model for the page changes only by updating it's properties (do not need to change methods around at all since they have the same abstraction over the input).  This reflects much more accurately the real change.  The page added a pin and removed a password.

interface ILoginPage : IPageModel
{
     // password changes to pin
    // IValuable<string, ILoginPage> Password {get;}
    IReadWriteValuePageModel<int, ILoginPage> Pin{get;}
}

The only places that use the Pin directly are the tests for the login page itself.  Everything else uses the LoginPageUserActions orchestration wrapper.

To show what this would look like, lets continue on to the AccountSettings tests.

public class AccountSettingsTests
{
    BrowserWindow window;
    IAccountSettings accountSettings;

    [TestInitialize]
    public void GivenAccountSettingsPage()
    {
          this.window = BrowserWindow.Launch("loginpageurl");

          // before change to use pin
          //new LoginPageUserActions(new LoginPage(this.window)).Login("user", "password");

          // after the change to use pin
          this.accountSettings = new LoginPageUserActions(new LoginPage(this.window)).Login("user", 1234);
    }

    public void WhenChangeFirstNameAndSave_ThenFirstNameIsUpdated()
     {
            this.accountSettings.FirstName.SetText("New Value").Save.Click();
            this.window.Refresh();
            Assert.IsTrue(this.accountSettings.FirstName.Value.Equals("New Value"));
     }
}

Even if I have 1,000 tests in account settings, the only change was in Initialize().  Further, that change is required for page objects as well.

Now, the changes required to page objects are actually more because of the change required for all the methods.  Page Models had exactly one change and that was to remove old property and add new one.  Page objects has to replace all old methods with new ones.

In the page modeling approach, you have extensive tests for a given page and other pages use an orchestration class.  To me, this is far more maintainable than updating tons of methods.  Also, the Pin property is exactly the same as the Password property except that its .SetValue() method takes an int instead of a string.  Testing for visibility, enabled, etc... are all exactly the same and would not change in tests (aside from property name).  You could probably just do a refactor -> rename, change the type, and update the methods that use it to put an int when setting the text.  All other usage would be identical.


No comments:

Post a Comment