Once in a while, I see a method like the following one:
public boolean isSectionDisplayed() {
try {
driver.findElement(sectionLinkBy).click();
return driver.findElement(sectionHeaderBy).isDisplayed();
}
catch (Exception e) {
driver.navigate().refresh();
driver.findElement(sectionLinkBy).click();
return driver.findElement(sectionHeaderBy).isDisplayed();
}
}
The method is simple:
it clicks on the section link to expand it
it returns true if the section header is displayed and false otherwise
Both of these actions happen in the try clause.
Things get interesting in the catch clause.
Since it is possible that an exception is generated in the try clause, either while finding/clicking the section link or finding the section header, the try statement is used. If an exception is generated in the try clause, the execution continues in the catch clause as follows:
the page is reloaded
section link is clicked again
the section header is found again
true is returned is the section header is displayed and false otherwise
The method basically implements a way of retrying what happens in the try clause.
If something goes wrong, the page is reloaded and the same thing is attempted again.
This is so wrong.
What is wrong with it?
Many things are wrong:
the try code for finding/clicking the section link is duplicated in the catch clause
the try code for finding the section header is duplicated in the catch clause
about the page reloading, after the page is reloaded, the code should wait until the correct page is displayed. It does not do that.
catch should use a specific exception (NoSuchElementException or ElementNotInteractableException) instead of Exception.
the code should wait until the section link is available and enabled before clicking it instead of finding it right away
the code should wait until the section header is available and visible before checking if it is displayed instead of finding it right away
And, what if an exception is generated while finding/clicking the section link in the catch clause? How is that managed?
The code is far from being correct.
But, there is another question about it.
Why would we need to retry what happens in the try clause?
We would need to do that because
the code cannot find the section link; maybe the section link is not displayed in the page for some reason
the code cannot click the section link; maybe the link is not enabled for some reason
the code cannot find the section header; maybe the section is not displayed after clicking the section link
What are all of these?
Bugs.
Why should we disregard the bugs, pretend that they do not exist and try again in an incorrect way?
Automated tests should find bugs instead of pretending that bugs do not exist.
The method should not wrap the code in a try/catch statement.
It should add waiting for synchronization before finding any of the 2 elements.
But that is about it.
If the code fails, a bug should be reported.
No attempt should be done at rescuing the test because this means ignoring the bug:
public boolean isSectionDisplayed() {
wait.until(ExpectedConditions.elementToBeClickable(sectionLinkBy));
driver.findElement(sectionLinkBy).click();
wait.until(ExpectedConditions.visibilityOfElementLocated(sectionHeaderBy));
return true;
}