Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am writing some tests that all follow the same pattern, I feel like repeating myself over and over again, how can I reduce the repetition? The (almost) repeating part is in constructor and the class level variables:

VariablesPage

public class VariablesPage extends LoadableComponent<VariablesPage> {

    private final TestHelpers helpers;
    private WebDriver driver;

    @FindBy(xpath = "//a[@title=\"Define new variable\"]") 
    private WebElement btnAddNewVariable;

    public VariablesPage() throws Exception {
        helpers = new TestHelpers();
        driver = helpers.getWebDriver();
        helpers.setHar(VariablesPage.class.getSimpleName());
        PageFactory.initElements(driver, this);
        isLoaded();
        helpers.getHar();
    }

    @Override
    protected void load() {

    }

    @Override
    protected void isLoaded() throws Error {
        helpers.waitUntil(btnAddNewVariable);
    }
}

addStepsPage

public class AddStepsPage extends LoadableComponent<AddStepsPage> {

    private final TestHelpers helpers;
    private WebDriver driver;
    @FindBy(xpath = "//div[@class=\"body sequential\"]")
    private WebElement divStepsBody;

    public AddStepsPage() throws Exception {
        helpers = new TestHelpers();
        driver = helpers.getWebDriver();
        helpers.setHar(AddStepsPage.class.getSimpleName());
        PageFactory.initElements(driver, this);
        isLoaded();
        helpers.getHar();
    }


    @Override
    protected void load() {

    }

    @Override
    protected void isLoaded() throws Error {
        helpers.waitUntil(divStepsBody);
    }
}
share|improve this question

3 Answers 3

up vote 10 down vote accepted

First, let us collect the code pieces which are repeated

Fields

private final TestHelpers helpers;
private WebDriver driver;

Constructor related

helpers = new TestHelpers();  
driver = helpers.getWebDriver();

Now, let's start refactoring by extending LoadableComponent<T>

public abstract class ExtendedLoadableComponent<T> extends LoadableComponent<T> {

    protected final TestHelpers helpers
    protected WebDriver driver;

    public ExtendedLoadableComponent() throws Exception {
        helpers = new TestHelpers();
        driver = helpers.getWebDriver();
    }
}

which reduces the first class to

public class VariablesPage extends ExtendedLoadableComponent<VariablesPage> {

    @FindBy(xpath = "//a[@title=\"Define new variable\"]") 
    private WebElement btnAddNewVariable;

    public VariablesPage() throws Exception {
        super();
        helpers.setHar(VariablesPage.class.getSimpleName());
        PageFactory.initElements(driver, this);
        isLoaded();
        helpers.getHar();
    }

    @Override
    protected void load() {}

    @Override
    protected void isLoaded() throws Error {
        helpers.waitUntil(btnAddNewVariable);
    }
}

But wait, we can do better! So, let us also call helpers.setHar(String) and thus we need to refactor the constructor of ExtendedLoadableComponent<T>

public abstract class ExtendedLoadableComponent<T> extends LoadableComponent<T> {

    protected final TestHelpers helpers
    protected WebDriver driver;

    public ExtendedLoadableComponent(Class<T> pageClassToProxy) throws Exception {
        helpers = new TestHelpers();
        driver = helpers.getWebDriver();
        helpers.setHar(pageClassToProxy.getSimpleName());
        PageFactory.initElements(driver, pageClassToProxy);
    }
}

which reduces the first class to

public class VariablesPage extends ExtendedLoadableComponent<VariablesPage> {

    @FindBy(xpath = "//a[@title=\"Define new variable\"]") 
    private WebElement btnAddNewVariable;

    public VariablesPage() throws Exception {
        super(VariablesPage.class);
        isLoaded();
        helpers.getHar();
    }

    @Override
    protected void load() {}

    @Override
    protected void isLoaded() throws Error {
        helpers.waitUntil(btnAddNewVariable);
    }
}

Naming

I don't know if the TestHelpers class is your own. If yes, you should think about changing the names for setHar() and getHar() so Mr. Maintainer can evaluate what they do by looking at the names.

share|improve this answer
    
har is referring to HTTP archive format which contains performance metrics, HTTP statuses etc. It is commented in TestHelpers class : ). And thanks for a great answer! –  Erki M. Aug 7 at 19:03
1  
@erki-m If you need a comment to explain the method, then its name is bad. –  Kolargol00 Aug 9 at 11:49
    
@Kolargol00 point taken. –  Erki M. Aug 11 at 7:10

First off, testing is inherently repeated. You are exploring many facets of the same attribute of your program, which means doing many things in slightly different ways.

Your job is to control the repeating so you aren't overwhelming yourself or costing too much money. You use the software design tools for testing that help you in the basic code.

Test Harness

As Eugene T pointed out, @Before and @After methods are available in the test harness, whether it is JUnit or TestNG. These methods might be configured to run before/after each method, class, test or suite.

Inheritance

In your example, you could create a common base class and then override the definition of some variables in the constructor.

Composition

You can create separate components that handle common elements and your test class composes them. Your test methods then invoke the methods on the components that are needed.

Further Research

This is just the start of a list of tools that you have at your disposal. You have the internet at your disposal for doing your own research on refactoring.

share|improve this answer

Just a few additional remarks on top of @heslacher's excellent review.

Don't throw Exception. If one of those methods inside the function throw some kind of Exception, then use that, don't fall back to the Exception parent class. You should always use the most specific Exception classes possible.

Definitely don't throw Error. Error is neither something to throw nor something to try to catch. Don't use it. See the javadoc for more details.

Finally, if you don't need to override a method, then don't override it:

@Override
protected void load() {}

If you must override it with an empty body, for example to prevent the method in the parent class from doing something, then add a comment to explain why this method needs to be there.

share|improve this answer

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.