2

Can you check my code and suggest improvements?

Here is the scenario .. enter image description here

This is for add folder module

package Pages;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.FindBy;
import org.openqa.selenium.support.PageFactory;
import org.openqa.selenium.support.ui.Select;

public class FolderPage 
{

    @FindBy(linkText="App")
    WebElement app;
    @FindBy(linkText="Folders")
    WebElement folders;
    @FindBy(xpath="//*[@id='search-container']/a/i")
    WebElement search;
    @FindBy(id="name")
    WebElement name;
    @FindBy(id="parent_id")
    WebElement parentid_dropdown;;
    @FindBy(id="client")
    WebElement client_dropdown;

    @FindBy(name="permmissions")
    WebElement permmissions;
    @FindBy(className="btn-primary")
    WebElement button;
    //constructor
    public FolderPage(WebDriver driver)
    {
        PageFactory.initElements(driver, this);
    }
    public void set_App()
    {
        app.click();
    }
    public void set_Folders()
    {
        folders.click();
    }
    public void set_Search()
    {
        search.click();
    }
    public void set_Name(String nam)
    {
        name.clear();
        name.sendKeys(nam);
    }
    public void select_List(String parent_id)
    {
        Select parentidDropdown=new Select(parentid_dropdown);
        parentidDropdown.selectByVisibleText(parent_id);
    }
    public void Select_List2(String client)
    {
        Select clientDropdown=new Select(client_dropdown);
        clientDropdown.selectByVisibleText(client);
    }

    public void set_Permmission(String perm)
    {
        permmissions.click();
    }
    public void set_Button()
    {
        button.submit();
    }
}

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

import Pages.FolderPage;

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

public class FolderTest 
{
    WebDriver driver;
@BeforeTest
public void setup()
{
    System.setProperty("webdriver.firefox.marionette","pathToGeckodriver");
    driver=new FirefoxDriver();
    driver.manage().window().maximize();
    driver.get("http://doc.local");
}
@Test(priority=1)
public void verify1()
{
    LoginPage login=new LoginPage(driver);
    login.set_username("");
    login.set_password("1");
    login.click_button();
    Assert.assertTrue(driver.getPageSource().contains("Hello Administrator"));
}
@Test(priority=2)
public void verify2()
{
    FolderPage folder=new FolderPage(driver);
    folder.set_App();
    folder.set_Folders();
    folder.set_Search();
    folder.set_Name("DCOO2");
    folder.select_List("/foo");
    folder.Select_List2("Demo");

    folder.set_Permmission("yyr");
    folder.set_Button();
    Assert.assertTrue(driver.getPageSource().contains("Hello Administrator"));
}
@Test(priority=3)
public void verify3()
{
    FolderPage folder=new FolderPage(driver);
    folder.set_App();
    folder.set_Folders();
    folder.set_Search();
    folder.set_Name("");
    folder.select_List("");
    folder.Select_List2("");

    folder.set_Permmission("");
    folder.set_Button();
    Assert.assertTrue(driver.getPageSource().contains("Name is mandatory!"));   
}
@Test(priority=4)
public void verify4()
{
    FolderPage folder=new FolderPage(driver);
    folder.set_App();
    folder.set_Folders();
    folder.set_Search();
    folder.set_Name("DCOO2");
    folder.select_List("/folder");
    folder.Select_List2("Demo");

    folder.set_Permmission("sdfsd");
    folder.set_Button();
    Assert.assertTrue(driver.getPageSource().contains("Hello Administrator"));
}


}
  • 1
    It is not clear what you're asking for. Improving code quality, test coverage or what? – dzieciou Oct 28 '16 at 9:47
  • 3
    Maybe this would go better on the code review site? – David says Reinstate Monica Oct 28 '16 at 13:07
  • 1
    @DavidGrinberg You think regular developers are better in reviewing Selenium testcode? I think it fits perfect on SQA. If I would have time today I might even try to answer it :) Also read this meta question and comment please: meta.sqa.stackexchange.com/questions/418/… – Niels van Reijmersdal Oct 28 '16 at 15:06
  • 1
    @NielsvanReijmersdal While I'm inclined to agree with your comment about regular devs vs selenium devs, I have to admit the same could be said for a wide variety of types of code and the code review site manages to do those very well. – corsiKa Oct 28 '16 at 15:43
  • 1
    And folks, please visit Niels' post and give feedback. I'd love to see CR posts allowed here, but I think it should be a community decision not a modhammer. :-) – corsiKa Oct 28 '16 at 20:33
1

I have a feeling this question will be closed but here's my feedback on your code:

Your Page Object Model code looks spot on. I would potentially put more emphasis into describing each element (What button is it?) and I typically use a base class that I inherit from that has the PageFactory.InitElements in it (optional but good practice for code reuse).

Your tests have a high amount of dependencies on other tests. (In particular your login test) This is why you have had to label them all with a priority, which will keep them working for now. I would highly recommend avoiding this as if there is a failure in the, for example, login step it will cause all of your other tests to fail without a decent explanation. If the login process is too time consuming, you can easily get a cookie from a successful login and then use that cookie in subsequent tests and avoid the login steps after your first successful login.

Overall though, good work.

1

I will comment on code quality

  • Java method names convention is to use camel case, starting with lower case and with no underscores between words. Same goes for parameter names.
  • Making page objects with fluent interface will make your tests more concise:

    FolderPage folder= new FolderPage(driver)
      withApp()
      withFolders()
      withSearch();
    
  • Use assertThat instead of assertTrue: it will make your error messages more clear.

  • Use dependsOn instead of priority. It will make dependencies between your tests more explicit.
  • Names of your test methods say nothing about what actually the test verifies. Take some convention like given-when-then, e.g., loginShouldFailForInvalidPassowrd.
  • Like Paul Muir mentioned, work variables and fields naming. button sounds a bit cryptic to me.