Selenium Framework using a lot of statics

Question

I am writing a Selenium framework that should be really easy for a tester with relatively little Java knowledge to use and write tests for. In order to keep the framework as user friendly as possible, I've noticed that I am writing a lot of statics so that the tester doesn't have to instantiate the page objects and methods every time. This of course goes against the principles of OOP. Before I get in too deep with this I'd like to see if there are viable alternatives to my approach. I'm including below the login page class that instantiates a webdriver Singleton, has a username/password field, and a login button. There are 2 tests for this page that are run using TestNG. All code to follow:

LoginPage.java

public class LoginPage {

    public static WebDriver driver;
    private static String username_selector = "username";
    private static String password_selector = "password";
    private static String login_button_selector = "#loginbutton > input";
    private static String fail_message_selector = "error";


    public static void goTo(String environment_url){
        driver = Driver.getDriver();
        driver.get(environment_url);
    }

    public static void loginAs(String username, String password){
        WebElement user = driver.findElement(By.name(username_selector));
        user.clear();
        user.sendKeys(username);
        WebElement pass = driver.findElement(By.name(password_selector));
        pass.clear();
        pass.sendKeys(password);
        WebElement loginBtn = driver.findElement(By.cssSelector(login_button_selector));
        loginBtn.click();
    }

    public static boolean loginErrorDisplayed(){
        WebElement failMessageContainer = driver.findElement(By.className(fail_message_selector));
            if(failMessageContainer.isDisplayed()){
                return true;
            }
            else return false;
    }

    public static String getLoginErrorMessage(){
        WebElement failMessageContainer = driver.findElement(By.className(fail_message_selector));
        String failMessage = failMessageContainer.getText();
        return failMessage;
    }

}

Driver.java

public class Driver {
    public static WebDriver driver = null;

    public static WebDriver getDriver(){
        if(driver == null){
            driver = new FirefoxDriver();
        }

        return driver; 
    }

    public static void killDriver(){
        driver.quit();
    }
}

LoginTest.java

public class LoginTest {

@Test(priority = 0)
public void loginfail() {
    LoginPage.goTo("http://127.0.0.1:8080/login");
    LoginPage.loginAs("wrong username", "wrongpassword");
    boolean didLoginFail = LoginPage.loginErrorDisplayed();
    Assert.assertTrue(didLoginFail == true, "Bad login was successful");
    if (didLoginFail){
        LoginPage.getLoginErrorMessage();
    }
}

@Test(priority = 1)
public void loginsuccess() {
    LoginPage.loginAs("correct_username", "correctpass");
    boolean didLoginFail = LoginPage.loginErrorDisplayed();
    Assert.assertTrue(didLoginFail == false, "Valid Login was unsuccessful");
}

}

Answer 1

        if(failMessageContainer.isDisplayed()){
            return true;
        }
        else return false;

Why not simply return failMessageContainer.isDisplayed()?

---

@Test(priority = 0)
public void loginfail() {
    LoginPage.goTo("http://127.0.0.1:8080/login");
    LoginPage.loginAs("wrong username", "wrongpassword");
    boolean didLoginFail = LoginPage.loginErrorDisplayed();
    Assert.assertTrue(didLoginFail == true, "Bad login was successful");
    if (didLoginFail){
        LoginPage.getLoginErrorMessage();
    }
}

What about Assert.assertTrue((((didLoginFail == true) == true) == true, ...). I mean, if writing a==true was better than a, then doing it more often must be even better.

The following if is superfluous, the getLoginErrorMessage doesn't get used.

---

I've noticed that I am writing a lot of statics so that the tester doesn't have to instantiate the page objects and methods every time.

I'd start by dropping all the static modifiers and making it fluent. With a proper API the tester will happily do the instantiation as it leads them to what they want to do.

I don't find something like

public void loginfail() {
    LoginPage loginPage = new LoginPage()
        .goTo("http://127.0.0.1:8080/login")
        .loginAs("wrong username", "wrongpassword");
    Assert.assertTrue(loginPage.loginErrorDisplayed());
    Assert.assertTrue(loginPage.getLoginErrorMessage().contains("WTF?"));
}

harder to use than the static version. You could provide even more, but this is most probably not worth the effort (you'd sort of do the tester's job by providing fluent versions of all asserts they may need; this goes too far, but might be useful for the most commonly used assertions):

public void loginfail() {
    LoginPage loginPage = new LoginPage()
        .goTo("http://127.0.0.1:8080/login")
        .loginAs("wrong username", "wrongpassword")
        .assertThatLoginErrorDisplayed()
        .assertThatLoginErrorMessageContains("WTF?");
}

Answer 2

I've noticed that I am writing a lot of statics so that the tester doesn't have to instantiate the page objects and methods every time.

If you don't want to instantiate something every time, making it static is far from the only option. One option is to create an instance once and reuse it. Another option is to create the instance in a @Before method, so that all test cases can use it, and get a fresh instance.

This of course goes against the principles of OOP.

When you find yourself going against good principles, ask yourself (or your rubber duck), if your use case is really so special to warrant to throw away good principles. I see nothing in your goals that make it inevitably conflict with OOP. Going straight against OOP sounds like a huge design problem. If you try to rewrite without static methods (as @maaartinus suggested), and the solution is not so great, I bet the flaw will be less grave as violating good OOP.

Ordering of unit tests

Keep in mind that ideally, unit tests should not have side effects. When you force unit tests in a specific order (using @Test(priority = ...) in your example), it's a symptom that the tests have side effects.

Side effects are dangerous, because some tests may "happen to pass" because of a bug, thanks to an obscure side effect.

Tests with side effects are hard to read, because in order to understand what the test does and how it works, you cannot read the test alone, you also need to read and understand everything that runs before it.

It would be better rewrite to avoid side effects, even if the result is less efficient. Sure, creating a new driver instance in a @Before method for every single test case may seem inefficient, but performance shouldn't be the primary concern of unit testing.

Singletons

If you must use a singleton (like the Driver class), consider an implementation using an enum, that's short and sweet. (However, it's not lazy, as yours. I hope you don't mind.)

enum DriverManager {
    INSTANCE;

    private WebDriver driver = new FirefoxDriver();

    public WebDriver getDriver() {
        return driver;
    }

    public void killDriver() {
        driver.quit();
    }
}