Is Selenium's LoadableComponent a good solution as test design pattern?

Question

I need to refactor my Selenium tests, and come across their LoadableComponent wiki page. I began trying to use it, though I couldn't really understand what is it good for. I took a look at their current implementation (as for 2011-07-26), and am now even more confused. The code is basically using error handling to control the execution flow. Something that comes to me very weird:

@SuppressWarnings("unchecked")
public T get() {
  try {
    isLoaded();
    return (T) this;
  } catch (Error e) {
    load();
  }

  isLoaded();

  return (T) this;
}

That is, against the design principle "use exceptions only for exceptional use", here, the exception is the rule, and isLoaded is expected to throw an exception at least the first time the method is invoked.

Moreover, this LoadableComponent is an abstract class, meaning that I cannot have a child component inheriting from a parent component.

This all seems wrong to me. I'd just implement a get() method per page object that would load the page if needed based on the status of the Selenium driver, no need to capture exceptions. Am I getting all this wrong?

Answer 1

I do not know which ''this'' you are referring to when you ask, "Am I getting all this wrong?" According to the Wiki page, the isLoaded method should use JUnit assertions to check whether the page is actually loaded. JUnit assertions throw Errors, not exceptions, to signal that something went wrong.

LoadableComponent could have refactored isLoaded into two methods: one (call it isLoadedBoolean) that returns a boolean indicating whether the page was loaded, and one (call it assertLoaded) using JUnit assertions. Of course, if checking whether the page is loaded requires evaluating multiple conditions, it might be valuable for assertLoaded to indicate which condition fails. Lazy people would probably do this:

protected void assertLoaded() {
  assertTrue(condition1,"condition 1 failed");
  assertTrue(condition2,"condition 2 failed");
}

protected boolean isLoadedBoolean() {
  try {
    assertLoaded();
    return true;
  }
  catch (Error e) {
    return false;
  }

Other people would think that design was too weird, and instead would do this:

protected void assertLoaded() {
  assertTrue(condition1,"condition 1 failed");
  assertTrue(condition2,"condition 2 failed");
}

protected boolean isLoadedBoolean() {
  return condition1 && condition2;
}

Of course, if the criteria for checking whether the page is loaded ever changes (and of course it will, since this is a test for a web interface), both methods will need to be updated. So developers who code this way will trade off weirdness for maintenance bugs.

I suppose another option would be something like this:

public class PageLoadCheckFailure {
   public String reason;
   ... other interesting data ...

   public PageLoadCheckFailure(reason, ... other interesting data ...) {
      this.reason = reason;
      etc.
   }

   public String getReason() {
      return reason;
   }

   ... getters for other interesting data ...
}

protected PageLoadCheckFailure checkWhetherLoaded() {
   if (!condition1) {
      return new PageLoadCheckFailure("condition 1 failed", ... other interesting data...);
   }
   if (!condition2) {
         return new PageLoadCheckFailure("condition 1 failed", ... other interesting data...);
   }
   return nil;
}

protected void assertLoaded() {
  PageLoadCheckFailure result = checkWhetherLoaded();
  if (result != nil) {
     Assert.fail(result.toString());
  }
}

protected boolean isLoadedBoolean() {
  return checkWhetherLoaded() == nil;
}

But that sure seems like a lot of work compared to the Selenium design, especially since this is, after all, just test code.