Testing multiple logins using Selenium listed in a property file

Question

Excuse please review my code. I am working in automation testing. I did a automation testing for my login page. I am using selenium with page object pattern. I wrote here my code. I am using java language and I am keeping my test data in property file. what should I need to improve ? and It will help me to go to next step.

    package pageobject;

    import org.openqa.selenium.WebElement;
    import org.openqa.selenium.support.FindBy;
    import org.openqa.selenium.support.How;

    /**
     * Created by naveen on 6/3/17.
     */


    public class HomePage {
        @FindBy(how= How.NAME, using = "username")
        WebElement username;
        @FindBy(how=How.NAME, using = "password")
        WebElement password;
        @FindBy(how=How.XPATH, using="//*[@id=\'login-container\']/form/div[3]/div/p/input[1]" )
        WebElement button;

        //enter  username
        public void  userLogin(String user, String pass)
        {
            username.sendKeys(user);
            password.sendKeys(pass);
            button.click();
        }
    }



    package steps;

    import org.openqa.selenium.support.PageFactory;
    import pageobject.HomePage;
    import util.DriverManager;
    import java.io.IOException;
    /**
     * Created by naveen on 6/3/17.
     */
    public class LoginSteps {

        public LoginSteps() throws IOException {
        }

        public HomePage Login(String uname, String pwd) {


            HomePage homePageObj = PageFactory.initElements(DriverManager.driver, HomePage.class);

            homePageObj.userLogin(uname, pwd);

            return homePageObj;
            }
        }


package util;

import org.openqa.selenium.WebDriver;
import org.openqa.selenium.chrome.ChromeDriver;

import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.Properties;

/**
 * Created by naveen on 6/3/17.
 */
public class DriverManager
{
    public static WebDriver driver;
    String baseUrl;
    public Properties prop;

    public DriverManager() throws IOException {
        // Get baseUrl property
        File file = new File("config.properties");
        FileInputStream fileInput = null;
        try {
            fileInput = new FileInputStream(file);
        } catch (FileNotFoundException e) {
            e.printStackTrace();
        }
        //load properties file
        prop = new Properties();
        try {
            prop.load(fileInput);
        } catch (IOException e) {
            e.printStackTrace();
        }




        // Start driver
        System.setProperty("webdriver.chrome.driver", "/home/naveen/chromedriver");
        driver = new ChromeDriver();

        driver.manage().window().maximize();
        baseUrl = prop.getProperty("baseurl");
        driver.get(baseUrl);

    }
}


package login;

import org.openqa.selenium.By;
import org.testng.Assert;
import org.testng.annotations.AfterSuite;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import steps.LoginSteps;
import util.DriverManager;

import java.io.IOException;



public class LoginTest extends DriverManager
{

    LoginSteps lSteps;

    int inputDataFlag = 0;
    // 0 -> Invalid i/p
    // 1 -> Valid inputs


    String[] successMsg;
    int successCounter = 0;

    String successText;

    public LoginTest() throws IOException {
        lSteps = new LoginSteps();
        successMsg = prop.getProperty("loginSuccess").split(",");
    }



//    Invalid Credential as input
    @Test(dataProvider = "setData")
    public void initDriver(String name, String password) {
        //Arrange
        lSteps.Login(name, password);
        Assert.assertFalse(driver.getPageSource().contains("Hello Sony George"), "Valid credentials");
    }
     //   valid Credential as input
    @Test(priority = 1,dataProvider = "setData")
    public void init(String name,String password)
    {
        lSteps.Login(name,password);
        Assert.assertTrue(driver.getPageSource().contains(successMsg[successCounter]), "Valid credentials");


//logout
        if(driver.getPageSource().contains(successMsg[successCounter])){
            driver.findElement(By.xpath("//*[@id=\'rightMenu\']/li/a")).click();
            driver.findElement(By.linkText("Logout")).click();
            successCounter++;
        }

    }

    @DataProvider
    Object[][] setData() {

        Object[][] dataValues = getValues();

        return dataValues;
    }




    private String[][] getValues(){

        String[] namArray = new String[0];
        String[] pasArray = new String[0];

        if(inputDataFlag == 0){
            namArray = prop.getProperty("invalidnames").split(",");
            pasArray = prop.getProperty("invalidPwd").split(",");
        }else if(inputDataFlag == 1){
            namArray = prop.getProperty("validnames").split(",");
            pasArray = prop.getProperty("validPwd").split(",");
        }


        String nam, pas;

        int len = namArray.length;

        String[][] dataValues = new String[len][2];

        for (int i = 0; i < len; i++) {
            nam = namArray[i];
            pas = pasArray[i];

            dataValues[i][0] = nam;
            dataValues[i][1] = pas;

        }

        inputDataFlag++;

        return dataValues;

    }

    @AfterSuite
    public void close() {
    driver.close();
    }

}

property file

invalidnames = geo,sony,geo,geogr,geoson,geo,"",    sony
invalidPwd = 1234,123,56,345,123456789,123,"",  123
#Valid Test case
validnames = somevalidnames
validPwd = somevalidpassword

loginSuccess = Hello Sony George,Hello Naveen,Hello Juswin

Answer 1

Anthony,

Here are things I would suggest to improve:

HomePage

1) Use xpath only when you really need to. Xpath is powerful and there is no need to be afraid of it, but it's misused quite often. For example in button element. You use absolute xpath. This means that when your homepage will have additional div somewhere after login container - your tests will most likely fail. When you use xpath - use relative xpath. Also, think if it's really needed? Maybe you can put id or class on button?

2) Initialize page elements in constructor so that you don't need to do that anywhere else. Add

public HomePage(WebDriver driver) {
   PageFactory.initElements(driver, this)
}

note that I have also passed driver as a page dependency. Inject dependencies whenever possible. You might run into concurrency problems when you use static driver fields

LoginSteps

1) Check java convention for method names. It has to be lowercase.

DriverManager

1) This class, as name suggests, should be responsible only for managing the driver. You gave it and it's constructor too many responsibilities. Take a look at single responsibility pattern.

LoginTest

1) Check JAVA convention for curly braces. Consistency is important.

2) extends DriverManager ? No. Test should not extend driver manager, in fact it has nothing to with driver besides using it.

3) Your test names are unclear. If initDriver is responsible for what it says it is - it shouldn't be annotated with Test. BeforeTest, AfterTest (not sure how it's called in ngtest) is what you are looking for. No asserts can appear in before or after methods

4) Why do you need priority? YOur tests has to be independent from each other.

5) No element locators should appear in tests. No xpaths, no other selectors.