How many issues can you find in this Selenium test?

This is a Selenium test that I found recently in LinkedIn:

public class Tests {

    @Test
    public void search() {
       String url = "https://www.site.com/";

       driver.get(url);

       try {

          WebElement searchBox = driver.findElement(
                                   By.xpath("//input[@id='searchtextbox']"));

          Assert.assertTrue(searchBox.isDisplayed(), 
                            "Failed: Search Box is not Present");

          searchBox.click();
          searchBox.sendKeys(product);
 
          WebElement searchbtn = driver.findElement(
                                   By.id("search-submit-button"));

          searchbtn.click();

          WebDriverWait wait = new WebDriverWait(driver, 
                                                 Duration.ofSeconds(30));

          wait.until(
             ExpectedConditions.visibilityOfElementLocated(
                 By.xpath("//span[@class='a-color-state a-text-bold']")));

          WebElement resultPage = driver.findElement(
                       By.xpath("//span[@class='a-color-state a-text-bold']"));

          Assert.assertTrue(resultPage.isDisplayed(),
             "Failed: Searched Keyword is not present in result page");
     } 
     catch (Exception e) {
       // TODO: handle exception
       e.printStackTrace();
     }

  }
}

---

How many issues can you find in this test?

Let’s explore them all together.

---

1. the test will never fail if there is an exception generated. Since the code is wrapped in a try/catch statement, if an exception is generated, the stack trace is displayed but the test does not fail.

2. the url variable should be declared as a class field since other tests may need it.

3. all findElement lines of code are very long because of the locators being hard-coded; the locators should all be declared as class fields since other tests may need them as well

4. there is no assertion to verify that the home page is displayed; the code should not execute further if home page is not displayed

5. there is no assertion to verify that results page is displayed; the code should not execute further if results page is not displayed

6. the code should wait for each element to be available before interacting with it; this is not done for the search box or for the search button

7. searchbtn is not a good variable name

8. search() is not a good test name

9. one of the locators is duplicated, being used once in a wait(), second in a findElement()

10. resultsPage is a bad name; it should be resultsPageLabel

---

How should the code look like?

public class Tests {

   private static final String URL = "https://www.site.com/";
   
   private static final By SEARCH_BOX_ID    = By.id("searchtextbox");
   private static final By SEARCH_BUTTON_ID = By.id("search-submit-button");
   private static final By SEARCH_LABEL_BY  = 
         By.xpath("//span[@class='a-color-state a-text-bold']");


   @Test
   public void executingSearchDisplaysResultsPageTest() {  

      WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(30));

      driver.get(URL);
      Assert.assertTrue(driver.getCurrentUrl().contains("/home"), 
                        "Home Page is not displayed!");

      wait.until(ExpectedConditions.elementToBeClickable(SEARCH_BOX_ID));
      WebElement searchBox = driver.findElement(SEARCH_BOX_ID);
      searchBox.click();

      searchBox.sendKeys(product);

      wait.until(ExpectedConditions.elementToBeClickable(SEARCH_BUTTON_ID));
      WebElement searchButton = driver.findElement(SEARCH_BUTTON_ID);
      searchButton.click();

      Assert.assertTrue(driver.getCurrentUrl().contains("/results"), 
                        "Results Page is not displayed!");

     wait.until(ExpectedConditions.visibilityOfElementLocated(
                                          SEARCH_LABEL_BY));

     WebElement resultPageLabel = driver.findElement(SEARCH_LABEL_BY);

     Assert.assertTrue(resultPageLabel.isDisplayed(), 
                       "Searched Keyword is not present in result page");
   }

}

The code can be shortened further by:

• removing clicking in the search box as this is rarely needed

• removing the findElement() since the wait returns the web element

public class Tests {

   private static final String URL = "https://www.site.com/";
   
   private static final By SEARCH_BOX_ID    = By.id("searchtextbox");
   private static final By SEARCH_BUTTON_ID = By.id("search-submit-button");
   private static final By SEARCH_LABEL_BY  = 
               By.xpath("//span[@class='a-color-state a-text-bold']");


   @Test
   public void executingSearchDisplaysResultsPageTest() {  

      WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(30));

      driver.get(URL);
      Assert.assertTrue(driver.getCurrentUrl().contains("/home"), 
                        "Home Page is not displayed!");

      WebElement searchBox = wait.until(
                               ExpectedConditions.elementToBeClickable(
                                 SEARCH_BOX_ID));

      searchBox.sendKeys(product);

      WebElement searchButton = wait.until(
                                  ExpectedConditions.elementToBeClickable(
                                    SEARCH_BUTTON_ID));

      searchButton.click();

      Assert.assertTrue(driver.getCurrentUrl().contains("/results"), 
                        "Results Page is not displayed!");

      WebElement resultPageLabel = wait.until(
                                ExpectedConditions.visibilityOfElementLocated(
                                 SEARCH_LABEL_BY));

      Assert.assertTrue(resultPageLabel.isDisplayed(), 
                        "Searched Keyword is not present in result page");
   }

}

And, if you want to use methods instead of Selenium code in the test method:

public class Tests {

   private static final String URL = "https://www.site.com/";

   private static final By SEARCH_BOX_ID    = By.id("searchtextbox");
   private static final By SEARCH_BUTTON_ID = By.id("search-submit-button");
   private static final By SEARCH_LABEL_BY  = 
                  By.xpath("//span[@class='a-color-state a-text-bold']");


   @Test
   public void executingSearchDisplaysResultsPageTest() {  
      openHomePage();
      Assert.assertTrue(isHomePageDisplayed(), 
                        "Home Page is not displayed!");

      search(product);

      Assert.assertTrue(isResultsPageDisplayed(), 
                        "Results Page is not displayed!");

      Assert.assertTrue(isResultsPageLabelDisplayed(), 
                        "Searched Keyword is not present in result page");
  }

  private void openHomePage() {
     driver.get(URL);
  }

  private boolean isHomePageDisplayed() {
     return driver.getCurrentUrl().contains("/home");
  }

  private void search(String keyword) {
     WebDriverWait wait = getWait(30);

     WebElement searchBox = wait.until(
                              ExpectedConditions.elementToBeClickable(
                                SEARCH_BOX_ID));

     searchBox.sendKeys(keyword);

     WebElement searchButton = wait.until(
                                 ExpectedConditions.elementToBeClickable(
                                   SEARCH_BUTTON_ID));

     searchButton.click();
  }

  private boolean isResultsPageDisplayed() {
    return driver.getCurrentUrl().contains("/results");
  }

  private boolean isResultsPageLabelDisplayed() {
    WebDriverWait wait = getWait(30);

    wait.until(
      ExpectedConditions.visibilityOfElementLocated(SEARCH_LABEL_BY));

    return true;
  }

  private WebDriverWait getWait(int seconds) {
    return WebDriverWait wait = new WebDriverWait(driver,
                                                  Duration.ofSeconds(seconds));
  }
}

What’s next?

Page Object Model.