Login page with Selenium webdriver

Question

I am automating a login page. I am using the page object design pattern Selenium with Java. What should I improve on in my code?

package Pages;

import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;
import org.openqa.selenium.support.PageFactory;


public class LoginPage 
{
    private WebDriver driver;

       @FindBy(id="userName")
       WebElement username;
       @FindBy(id="password")
       WebElement password;
       @FindBy(className="btn-info")
       WebElement button;
       public LoginPage(WebDriver driver)
       {
        //initialize elements
          PageFactory.initElements(driver, this);

       }
       public void set_username(String usern)
       {
        username.clear();
        username.sendKeys(usern);
       }
       public void set_password(String userp)
       {
        password.clear();
        password.sendKeys(userp);
       }
       public void click_button()
       {
        button.submit();
       }
}

---

package Test;
import org.testng.annotations.Test;


import Pages.LoginPage;
import org.testng.Assert;
import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.firefox.FirefoxDriver;
import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeTest;



public class LoginTest
{
WebDriver driver;
@BeforeTest
public void setup()
{
    System.setProperty("webdriver.firefox.marionette","pathToGeckodriver");
    driver=new FirefoxDriver();
    driver.manage().window().maximize();
    driver.get("https://twhyderabad.github.io/demo_site/");
}
@Test(priority=5)
public void verify1()
{
    LoginPage login=new LoginPage(driver);
    login.set_username("admin");
    login.set_password("admin");
    login.click_button();
    Assert.assertTrue(driver.getPageSource().contains("Blog"));
}
@Test(priority=1)
public void verify2()
{
    LoginPage login=new LoginPage(driver);
    login.set_username("adm");
    login.set_password("admin");
    login.click_button();
    Assert.assertEquals(driver.findElement(By.xpath("//*[@id='errorMessage']")).getText(),"Wrong username or password!");
}
@Test(priority=2)
public void verify3()
{
    LoginPage login=new LoginPage(driver);
    login.set_username("admin");
    login.set_password("adm");
    login.click_button();
    Assert.assertEquals(driver.findElement(By.xpath("//*[@id='errorMessage']")).getText(),"Wrong username or password!");
}
@Test(priority=3)
public void verify4()
{
    LoginPage login=new LoginPage(driver);
    login.set_username("adm");
    login.set_password("adm");
    login.click_button();
    Assert.assertEquals(driver.findElement(By.xpath("//*[@id='errorMessage']")).getText(),"Wrong username or password!");
}
@Test(priority=4)
public void verify5()
{
    LoginPage login=new LoginPage(driver);
    login.set_username("");
    login.set_password("");
    login.click_button();
    Assert.assertEquals(driver.findElement(By.xpath("//*[@id='errorMessage']")).getText(),"Wrong username or password!");
}
@AfterTest
public void close()
{
    driver.close();
}
}

Answer 1

• The method names are consistent, but usually camelcase is advocated.
• Indentation is inconsistent, but maybe that's from pasting it here?
• Variables like usern and userp are bad, why not name and password or something that's readable?
• Why is the window maximised? The site doesn't look that's necessary for anything.
• Consider having a login method instead of repeating the same code five times, e.g. login.login("admin", "admin").
• Extract common code to a new method. That is, for four of the five tests it could just be checkFailedLogin("foo", "bar").
• IDs are/should be unique to elements in HTML, so the XPath is way too complicated, consider just By.id (and By.className and By.name for that matter).
• Also consider returning the expected page from that method so that it's clear from the code, MainPage main = login.login(...); and have it raise an exception if the new page doesn't match what was expected (e.g. we usually have an invisible value unique to each page to identify it in tests, but there are probably other ways to do that).

Answer 2

All menetioned by @ferada and:

• Create a TestBase class where you keep all info that tests doesn't need to know. Like, what webdriver are you initializing, what properties are you setting to it and so on.
• Move all element locators away from test. It's not readable and irrelevant.
• You can have PageBase class where you do this inside constructor. PageFactory.InitElements(driver, this)

In this way you will avoid the same repetitive code in all pages you have.

• For asserting message - you could have separate object Message which would have a text property. And instead of hardcoded string for message, I prefer creating enums. For instance in test you would call something like:

  Assert.assertEquals(_loginPage.ErrorMessage, loginError.WrongUsername)

• Isn't there a better way to verify blog than to assert page source?

• I once learned a rule of thumb - if your test contains imports for openqa lib - you are doing it wrong :)