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"));
}


}

closed as unclear what you're asking by dzieciou, IAmMilinPatel, Kate Paulk, NarendraC, Yu Zhang Oct 28 '16 at 18:32

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 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 Grinberg 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.