All posts

Page Object refactor in C# - components instead of fat pages
Page Object refactor in C# - components instead of fat pages

Polski

Page Object refactor in C# - components instead of fat pages

August 2021 C#/Selenium 3.141 refactor: drop PageFactory, model UI fragments, fixture-scoped WebDriver. Not a Cypress App Actions rewrite.

QA

Introduction

Two years ago, while refactoring tests in Cypress, I was asking myself a question along the lines of “App Actions or Page Object Model”. I wrote both variants against the same demo application at the time and concluded that it was a matter of taste and project size. Today my question is completely different because the suite I am writing about is not in JavaScript but in C#, runs on Selenium 3.141, and has simply grown. It has grown in a very specific way: the HomePage class has more than five hundred lines and over forty calls to FindElement. In it, I can find the header, article list, pagination, footer, and, incidentally, three login helper methods because someone once needed them “on the home page”. Adding one test starts with fifteen minutes spent reading someone else’s class.

I am not going to rewrite this suite using another pattern. Page Object stays. What changes is what I consider “one page” and who owns the WebDriver. The application under test is still a public RealWorld-style demo: login, a header with links for a logged-in user, and an article list. No client names and no code from a commercial project.

The project targets net5.0, and I have the package versions pinned:

<PackageReference Include="Selenium.WebDriver" Version="3.141.0" />
<PackageReference Include="Selenium.Support" Version="3.141.0" />
<PackageReference Include="NUnit" Version="3.13.2" />
<PackageReference Include="NUnit3TestAdapter" Version="3.17.0" />

Selenium 4 is currently in beta, and 4.0.0-beta-4 was released on June 7. I keep it in a separate project for checking the migration, just as I did with Grid in Docker, while the daily run stays on 3.141.0. This refactor is also deliberately independent of the binding version: I want moving to version four later to be a change in one place, not in forty.

What hurt

Four things, in order from the most irritating. I will start with the static driver. Somewhere at the bottom of the project there is a public static IWebDriver Driver, set once in [OneTimeSetUp], and every page object reaches for it without asking. As long as the tests run sequentially, it works. On the first attempt to run them in parallel, two tests start controlling the same browser.

PageFactory. It has been marked obsolete in the .NET bindings since March 2018, with Selenium 3.11. Jim Evans described it at the time on his blog in a post about deprecating parts of the .NET bindings, and the code moved to a separate DotNetSeleniumExtras project. The compiler says the same thing during every build:

warning CS0618: 'PageFactory.InitElements(ISearchContext, object)' is obsolete:
'The PageFactory and supporting classes are deprecated
and will be removed in a future release.'

I shortened the message, but its meaning is unambiguous. This is not “Selenium’s recommended way to implement page objects”, but a piece of history that still compiles in my repository. Assertions in the page object and Thread.Sleep remain, and those two things usually come as a pair:

public class HomePage
{
    public static IWebDriver Driver;

    [FindsBy(How = How.CssSelector, Using = ".navbar .nav-link[href='/settings']")]
    [CacheLookup]
    private IWebElement _settings;

    public HomePage() => PageFactory.InitElements(Driver, this);

    public void AssertUserIsLoggedIn(string username)
    {
        Thread.Sleep(2000);
        Assert.That(_settings.Displayed, Is.True);
        Assert.That(Driver.FindElement(ProfileLink(username)).Displayed, Is.True);
    }
}

[CacheLookup] remembers the element reference after finding it for the first time. On a page that redraws itself after login, what I get in return is a StaleElementReferenceException at a random moment. Thread.Sleep(2000) is there to mask it, and it does exactly two things: adds two seconds to every test and does not remove the problem, only makes it happen less often. The method is called AssertUserIsLoggedIn, so when it fails, the report shows Expected: True, But was: False without telling me which of the two links is involved.

Fowler: a fragment, not a URL

I returned to the text where this entire puzzle began, namely Martin Fowler’s Page Object. The key sentence says nothing about URLs. A page object wraps a fragment of the interface and exposes it as an API, while whether that fragment is an entire page or a part of it depends on what makes sense for the test. Fowler explicitly writes that if a page contains a significant component worthy of a separate model, a separate page object can be created for it. My “one class per URL” was an interpretation I added myself, and it was what grew my five-hundred-line HomePage. Since the header is on every page and there is one HomePage, the header helper ended up there. The new division looks like this:

  • Header - the navigation bar, the same object on every page
  • LoginForm - the login form, regardless of where I embed it
  • ArticleFeed and ArticleRow - the article list and one row
  • LoginPage, HomePage - thin classes that only compose these components

Along the way, the BasePage with forty helper methods, which everything inherited from, disappears. The header is no longer something that every page “inherits”, but a property the page has:

public sealed class HomePage
{
    public HomePage(IWebDriver driver)
    {
        Header = new Header(driver);
        Feed = new ArticleFeed(driver);
    }

    public Header Header { get; }
    public ArticleFeed Feed { get; }
}

Composition instead of inheritance is not a textbook decoration here. When the header is a separate type, I can use it in a settings test and a logout test without dragging the rest of the home page along with it.

Without PageFactory

If PageFactory is obsolete, what replaces it? Plain By fields and finding the element at the moment of the action.

public sealed class LoginForm
{
    private static readonly By Root = By.CssSelector("form");
    private static readonly By EmailInput = By.CssSelector("input[type=email]");
    private static readonly By PasswordInput = By.CssSelector("input[type=password]");
    private static readonly By SubmitButton = By.CssSelector("button[type=submit]");
    private static readonly By ErrorItem = By.CssSelector(".error-messages li");

    private readonly IWebDriver _driver;

    public LoginForm(IWebDriver driver) => _driver = driver;

    private IWebElement Container => _driver.FindElement(Root);

    private void Fill(string email, string password)
    {
        var container = Container;
        container.FindElement(EmailInput).SendKeys(email);
        container.FindElement(PasswordInput).SendKeys(password);
        container.FindElement(SubmitButton).Click();
    }
}

Three decisions here are deliberate. The locators are static readonly By, not IWebElement. A By is a description of how to find an element and can be safely shared between instances. An IWebElement is a handle to a specific node in a specific DOM state, and it is precisely what breaks after the page is redrawn.

Container is a property, not a field set in the constructor, so every access performs a fresh FindElement. It costs one additional request to the browser and, in return, removes an entire class of stale element errors. It also narrows the search: container.FindElement(EmailInput) searches inside the form, so I will not hit the newsletter email field in the footer.

And I do not add DotNetSeleniumExtras just to keep [FindsBy]. That package is where the old code survived, not a direction for future development. Adding a dependency to keep using something that was removed from the bindings is a lot of work in the wrong direction.

Waits in the component

Fowler points out that a page object is a good place to hide the interface’s asynchronous behavior. The test should say “log in”, not “click, wait ten seconds, check”. That is why WebDriverWait lives in the component and never appears in the test method. I expand the LoginForm constructor and add the rest of the class:

private readonly WebDriverWait _wait;

public LoginForm(IWebDriver driver)
{
    _driver = driver;
    _wait = new WebDriverWait(driver, TimeSpan.FromSeconds(10));
    _wait.IgnoreExceptionTypes(typeof(StaleElementReferenceException));
}

public HomePage SubmitExpectingSuccess(string email, string password)
{
    Fill(email, password);
    _wait.Until(d => d.FindElements(Header.SettingsLink).Count == 1);
    return new HomePage(_driver);
}

public LoginForm SubmitExpectingFailure(string email, string password)
{
    Fill(email, password);
    _wait.Until(d => d.FindElements(ErrorItem).Count > 0);
    return this;
}

public IReadOnlyList<string> ErrorMessages() =>
    _driver.FindElements(ErrorItem).Select(e => e.Text).ToList();

I read the error list from the driver rather than from Container because in this demo .error-messages is a sibling of <form>, not its child, and the condition I wait on uses exactly the same scope.

The condition is a lambda, not ExpectedConditions. That class was labelled obsolete by the same decision as PageFactory, and its continued life happens in SeleniumExtras.WaitHelpers. A lambda with FindElements is shorter, requires no additional package, and returning an empty list instead of throwing an exception makes the condition an ordinary Boolean value. WebDriverWait itself swallows NotFoundException, so a NoSuchElementException in the condition does not stop the wait, whereas StaleElementReferenceException must be added explicitly. I always do that because while the page is being redrawn, it is a transient state, not an error.

The two methods instead of one Submit come from the conclusion I had already reached with Cypress in 2019: after a successful login I am on a different page than after a failed one, so the method cannot return a single type. The name says explicitly what I expect, and it determines what I wait for. It is also worth distinguishing waiting from retrying. WebDriverWait waits for an interface state, it does not repeat the action. Retrying the operation itself, as with Polly policies in integration tests, is a different tool for a different problem, and putting it around clicks can hide a real defect.

NUnit 3.13 and the lifecycle

The most interesting part remains, namely the static driver. By default, NUnit creates one instance of the test class for the entire fixture and runs all tests on it. This is why the driver ended up in a static field: since there was only one instance anyway, the difference seemed cosmetic.

Since NUnit 3.13, released on January 7 this year, there has been an attribute for this. The version I have pinned is 3.13.2 from April 27.

[TestFixture]
[FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
public class LoginTests : IDisposable
{
    private const string BaseUrl = "http://localhost:4100";
    private readonly IWebDriver _driver;

    public LoginTests()
    {
        var options = new ChromeOptions();
        options.AddArgument("--window-size=1920,1080");
        _driver = new ChromeDriver(options);
    }

    public void Dispose() => _driver.Quit();
}

With InstancePerTestCase, NUnit creates a new class instance for every test case, so the constructor runs once per test and Dispose runs after every test. The driver stops being shared state and becomes an instance field whose lifetime matches the lifetime of one test. There is no static IWebDriver anywhere in the project. Two things came out of this in practice. First, with this lifecycle, [OneTimeSetUp] and [OneTimeTearDown] must be static because there is no longer one instance on which NUnit could invoke them. Second, the driver is created in the constructor, so I hang the cleanup on IDisposable: NUnit calls Dispose after every test as long as the class implements that interface. [SetUp] and [TearDown] work here exactly as they always do, before and after every test, so this is a choice for symmetry with the constructor, not a requirement. When I forget to close the driver, the browsers remain open, and I notice when my laptop starts making noise. To avoid repeating the attribute on every class, it can be set once for the entire assembly with [assembly: FixtureLifeCycle(LifeCycle.InstancePerTestCase)].

I am doing this now even though the run is still sequential. Parallelism in NUnit is not a switch that can simply be turned on at the end. It only reveals shared state that has been in the code from the beginning. As long as the driver is static, adding Parallelizable will not make the tests faster, but will turn them into a generator of random red results. When the driver belongs to the instance and the instance lives as long as one test, it becomes possible to start discussing running the suite in parallel on the Grid. That is a topic for a separate post, and I will return to it when I have runs that can demonstrate something.

Assertions

Fowler is quite unambiguous here: a page object should not contain assertions because that mixes two responsibilities. The object should provide access to the interface state, while whether that state is correct is knowledge that belongs to the test. The exception he allows is assertions that guard a page’s invariants, for example that we are where we should be at all, rather than the specific things a given test is probing.

In 2019, I did exactly the opposite. I added a shouldBeLoggedIn custom command to Cypress and hid three assertions about the header links inside it. It looked elegant in the test and was completely unreadable in a failure report. Now the component returns data and the test evaluates it:

public sealed class Header
{
    public static readonly By SettingsLink = By.CssSelector("nav .nav-link[href='/settings']");
    private static readonly By Root = By.CssSelector("nav.navbar");
    private static readonly By AnyLink = By.CssSelector(".nav-link");

    private readonly IWebDriver _driver;
    public Header(IWebDriver driver) => _driver = driver;

    public IReadOnlyList<string> LinkHrefs() =>
        _driver.FindElement(Root).FindElements(AnyLink)
               .Select(e => e.GetAttribute("href")).ToList();
}

And the tests themselves:

[Test]
public void SuccessfulLoginShowsLinksForLoggedInUser()
{
    _driver.Navigate().GoToUrl($"{BaseUrl}/login");

    var home = new LoginPage(_driver).Form
        .SubmitExpectingSuccess("test@test.com", "test");

    Assert.That(home.Header.LinkHrefs(), Is.SupersetOf(
        new[] { $"{BaseUrl}/settings", $"{BaseUrl}/editor", $"{BaseUrl}/@test" }));
}

[Test]
public void InvalidPasswordShowsValidationMessage()
{
    _driver.Navigate().GoToUrl($"{BaseUrl}/login");

    var form = new LoginPage(_driver).Form
        .SubmitExpectingFailure("test@test.com", "wrong-password");

    Assert.That(form.ErrorMessages(), Is.EqualTo(new[] { "email or password is invalid" }));
}

The test now has three lines of substance that show the scenario, not the mechanics. When the assertion fails, NUnit will show the complete list of links that were actually present in the header instead of Expected: True. This is the difference that made taking assertions out of the page object worthwhile. The wait still sits in SubmitExpectingSuccess, so there is not a single Wait in the test: the component knows when the page is ready, and the test knows what should be there.

Summary

Page Object Model stays. After two weeks spent digging through this code, I have no argument for discarding the pattern, but I have three for changing how I used it.

  • An object’s boundary is an interface fragment, not a URL. The header, form, and table row are separate types composed into thin page classes.
  • I find elements at the moment of the action. No PageFactory, no [CacheLookup], and no package keeping code removed from the bindings three years ago alive.
  • The WebDriver belongs to one test. FixtureLifeCycle.InstancePerTestCase in NUnit 3.13 handles this with one attribute and is a prerequisite for any discussion about parallelism.

What is deliberately missing here: selectors. The fact that I use input[type=email] instead of an attribute dedicated to tests in the examples is a separate topic, and I wrote about it in the selector refactor. Code structure and the way elements are located are two independent problems, and mixing them in one refactor ends with neither being finished. And one last thing: this refactor means adding a test now takes me a dozen or so minutes instead of an hour, but it does not mean I am testing the right things. That remains a question about the entire test process, not about the arrangement of classes in the project.