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");
}
}
-
\$\begingroup\$ Just thought I'd note that I've had decent success using Cucumber and Selenium to write a framework where users with absolutely no programming knowledge can write tests. It does make things a little less flexible (you have to define a certain number of set rules ahead of time) but could be worth looking into. \$\endgroup\$cbreezier– cbreezier2015年05月24日 13:49:31 +00:00Commented May 24, 2015 at 13:49
2 Answers 2
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?");
}
-
\$\begingroup\$ With regards to dropping
static
modifiers, the exception is possibly for theDriver
since instantiating a Selenium driver actually causes a full browser to be created and takes a long time. \$\endgroup\$cbreezier– cbreezier2015年05月24日 13:52:46 +00:00Commented May 24, 2015 at 13:52
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();
}
}