Take the 2-minute tour ×
Software Quality Assurance & Testing Stack Exchange is a question and answer site for software quality control experts, automation engineers, and software testers. It's 100% free, no registration required.

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?

share|improve this question

1 Answer 1

up vote 3 down vote accepted

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.

share|improve this answer
    
Thanks a lot for your answer. I'm afraid I didn't thought long enough about this, and your reasoning is what I was missing. The two things I didn't understand were: 1- Why use Errors to control the flow? (I wrote 'exceptions' meaning Throwables that must be caught); and 2- Why is LoadableComponent a class that must be extended, instead of an interface? –  Alberto Nov 28 '11 at 13:31
    
You have answered my first question, as after reading your answer I have noticed the important part I have missed from their documentation: "By using these assertions it's possible to give users of the class clear information that can be used to debug tests." That is, your checkWheterLoaded method is much too cumbersome compared to a list of informative assertTrues. I should have read the Are exceptions really for exceptional errors question in stackoverflow! –  Alberto Nov 28 '11 at 13:31
    
I still don't like very much the second issue. I'm using a PageObjects hierarchy to model the nested components of our UI. Take for instance their example. To me, it makes sense that EditIssue component also offers the signOut() method in its API (it'd pertain to SecuredPage component). However, the LoadableComponent implementations is fairy symple to include directly in my root component, so this is now also fine to me. –  Alberto Nov 28 '11 at 13:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.