Note: This is Part 2 of a two-part series on most common test automation mistakes. Part 1 is here.
Here is my take on the “most common test automation mistakes” topic. However, I’m not going to delve into the mistakes in setting up the automation strategy or using Selenium etc. Instead, I’m going to concentrate on mistakes on a coding level applicable to many testing tools, frameworks and programming languages (I will use Java and JUnit4 in my examples, though).
6) “Hacks” with no explanation
Consider the following piece of code:
void enableLogging() {
loggingCheckbox.enable();
loggingCheckbox.disable();
loggingCheckbox.enable();
}Is this enable-disable-enable cycle a mistake, a typo we should fix? Or is it workaround for some application quirk, and we should leave it as is? Or is it workaround for some bug which is already fixed? We don’t know, because there is no comment with explanation.
One should always write comments for “hacks”, preferably with reference to tickets in bug tracker, if any.
7) Not checking if the return value is null
void enableLogging() {
UIObject2 loggingCheckbox = device.findObject(By.res("res_id"));
loggingCheckbox.click();
}What’s wrong with this code? If device.findObject() doesn’t find the object, it will return null value, and then the next line will fail with java.lang.NullPointerException exception, which will be impossible to understand while reading the test report without looking into the test script source code.
This can be fixed either by checking loggingCheckbox for null before clicking it, and then raising an exception with custom error message which explains what has happened, or by handling NullPointerException and re-throwing it with custom error message.
void enableLogging() {
String res_id = "res_id";
UIObject2 loggingCheckbox = device.findObject(By.res(res_id));
if (loggingCheckbox == null) {
throw new ObjectNotFoundException("Logging checkbox was not found. Resource id: " + res_id);
}
loggingCheckbox.click();
}Which approach is better? In general, exception handling is expensive, so theoretically it is more efficient to check for null and throw the exception, than do catch and re-throw. However in automation code we usually don’t have to worry about cutting milliseconds or reducing memory footprint too much, so choose whichever approach you like.
Also, depending on the framework you use, findObject() (or its analogue) might throw an exception instead of returning null if it doesn’t find the object, so you won’t have to do anything. I think, Selenium API works that way. However, this issue is not limited to test framework APIs, but might occur when using some library as well.
8) Method name and return value mismatch
Novice programmers often don’t know that typically methods which return boolean value should have name starting with “is” — e.g. isEnabled().
Take a look at the two following if blocks:
if (!getNotificationStatus()) {
doSomething();
}if (!isNotificationEnabled()) {
doSomething();
}
Which one is easier to understand? I think the second one by far.
Usually, if method has name like getSomething(), one would expect that it returns not boolean, but some other type — string, enum, integer etc. However in our case it was returning boolean, so the more appropriate name would be “isNotificationEnabled” instead of “getNotificationStatus”.
Note, that in many languages, non-boolean values still can be used as boolean in logical expressions, but that is considered a bad practice because it is error-prone and hard to read.
9) Using boolean method parameter instead of creating two methods
Consider the following method:
void enableLogging(boolean enable) {
if (enable) {
loggingCheckbox.setEnabled();
} else {
loggingCheckbox.setDisabled();
}
}When reading a piece of code which calls this method one should pay attention to which argument — true or false — was passed to it in order to know whether it will enable or disable logging, and that requires some mental effort. Especially so, when we are passing false to the method.
Doesn’t this look confusing to you?
enableLogging(false);Better approach would be to define two methods instead:
void enableLogging() {
loggingCheckbox.setEnabled();
}void disableLogging() {
loggingCheckbox.setDisabled();
}
At first glance this looks like a more verbose approach, but our goal is not to save bytes of disk space. Our goal is to write robust and easy to understand and maintain code, and the second approach serves this goal better.
10) Bad error messages
I’ve already written a lot about an importance of writing custom error messages when throwing an exception and writing an assertion. Here I want to stress an importance of not only writing an error message, but writing a good error message.
Take a look at the example below:
public String isNotificationEnabledFor(String appName) {
UIObject appInList = device.findObjectbyText(appName);
if (appInList == null) {
throw new RuntimeException("Could not find an app");
}
...
}Although the error message is not that bad at the first glance, imaging that you’re seeing it in the test report. Will it be possible to easily understand what happened where? I don’t think so.
Good error message would mention not only what happened, but where or when. And together with logging, it would become a powerful tool for doing RCA.
public boolean isNotificationEnabledFor(String appName) {
UIObject appInList = device.findObjectbyText(appName);
if (appInList == null) {
throw new RuntimeException("Could not find '" + appName + "' in Settings->Notifications");
}
...
}Those are all most common test automation mistakes I wanted to share.
Let me know in the comments about other mistakes you see often, or if you disagree with something.
