Keeping tests valuable: Avoid commons mistakes.
More than the act of testing, the act of designing tests is one of the best known bug preventatives - Boris Beizer
There is no more space for criticism or making excuses for not testing any application, frontend or backend. The software development industry demands this of us developers no matter the deadlines or circumstances! So we have no time to lose, we need to be effective, agile and accurate when we are developing our code. But at the same time, we need to make sure that everything we write, every algorithm, does what the client requires without problems. In this article, we will understand how to avoid common mistakes in our day-to-day life as software developers.
Testing should be based on requirements.
Testing should be based on the requirements because if we don't follow the requirements the code we write is useless. If the algorithm does not follow the requirements we fail! When we understand the requirements and the business rules, it becomes clearer what the application should or should not do. A common case for developers who need to test components on the front-end is when dealing with lists, forms, events and so on. We need to worry about many factors, especially when testing the behaviors of each component. Let's look at a quick example with a form:
<form [formGroup]="form">
<input type="text" formControlName="email" placeholder="Digite o e-mail">
<br>
<br>
<input type="password" formControlName="password" placeholder="Digite a senha">
<br>
<br>
<button type="submit" [disabled]="form.invalid" class="btn-login" (click)="login()">Fazer login</button>
</form>
Okay, this is a very basic login page. We deal with this all the time. But let's look at the requirements:
must be a valid email address
must contain a special character @
must not contain a special character @ in initial
must not allow the local part of the email to contain more than 64 characters
We have requirements for the e-mail field well defined. But let's look at the code component:
export class FormLoginComponent implements OnInit {
form!: FormGroup;
constructor(private fb: FormBuilder, private http: HttpService) {
this.form = this.fb.group({
email: [null, [Validators.required]],
password: [null, [Validators.required]],
})
}
ngOnInit(): void {
}
login() {
if(this.isValidForm()) {
this.http.login(this.createPayload()).subscribe()
}
}
isValidForm(): boolean {
return this.form.valid;
}
onClick() {
console.log(this.form)
}
getValueControl(form: any, control: string) {
return form.controls[control].value
}
createPayload(
email = this.getValueControl(this.form, 'email'),
password = this.getValueControl(this.form, 'password')) {
const payload = {
email,
password
}
return payload;
}
}
Now let's see a unit test:
it('should be enabled when the form is valid', () => {
component.form.controls['email'].setValue('neymar.santos@gmail.com');
component.form.controls['password'].setValue('Password123!@');
const button = fixture.debugElement;
fixture.detectChanges();
expect(button.nativeElement.querySelector('.btn-login').disabled).toBeFalse();
})
It is clear that we are not following the requirements, the tests only validate if the form is valid or not, i.e., it does not validate if the input e-mail is valid or not. Let's improve the tests by being more precise and following the requirements:
it('should return false if local-part to be longer than 64 characters', () => {
let email = 'm'.repeat(65) + '@gmail.com';
component.form.controls['email'].setValue(email);
const result = component.form.controls['email'].valid;
expect(result).toBeFalse()
})
it('should return true if local part is less than 64 characters', () => {
let email = 'm'.repeat(64) + '@gmail.com';
component.form.controls['email'].setValue(email);
const result = component.form.controls['email'].valid;
expect(result).toBeTrue()
})
it('should return true if contain a special character @', () => {
const email = 'rafael@gmail.com'
component.form.controls['email'].setValue(email);
const result = component.form.controls['email'].valid;
expect(result).toBeTrue()
})
it('should return false if not contain a special character @', () => {
const email = 'rafael.gmail.com'
component.form.controls['email'].setValue(email);
const result = component.form.controls['email'].valid;
expect(result).toBeFalse()
})
it('should return false if email start with the @ special character', () => {
const email = '@t@gmail.com'
component.form.controls['email'].setValue(email);
const result = component.form.controls['email'].valid;
expect(result).toBeFalse()
})
We have improved our tests a little bit. Note that for the tests to pass, it is necessary to include a validator in the code, in Angular we have one for email called Validators.email:
email: [null, [Validators.required, Validators.email]],
Angular Validators provides us with this as help, but it would be interesting to follow the business rules and requirements to create your own. The point I want to get to is that in these tests described above we are testing cases of success and error, this is very important because if someone for lack of attention removes this validator and inserts another he must continue to follow the requirements and the tests will fail, the developer will understand that these requirements need to be revised, everything will depend on the context that your application is.
Why does it matter? When we understand the entire flow of the feature from start to finish, we can better understand the test cases and add real value to them by testing the limits of each requirement. Remember that there is a clear difference between features and requirements:
Feature: is about something the system offers. A feature can meet one or several needs. Example: An e-commerce store offers the possibility to add products to an online shopping cart.
A requirement expresses a need or a constraint that the system has to satisfy, i.e. it is a capability that a product must possess to satisfy a customer need or goal, it is more granular. Examples: The user will be able to add books to the online shopping cart; The user will be able to remove books from the online shopping cart.
To better understand features and requirements, let's look at another example.
There is a system needed by a local hospital that will allow doctors to know the date of service, patient name, the record of the doctor who treated the patient, action taken to treat the patient and monitor the patient's symptoms.
Using this scenario, the requirements are listed below:
Date of service
Patient's name
Record of the physician who treated the patient
Monitor this patient's symptoms
Action taken
Monitor this patient's symptoms
Using the example mentioned above, an example of a resource would be a "Patient Report". In this way, all the requirements mentioned above can become part of the feature.
Finally, we can understand that the tests in the backend should follow the requirements closely, documenting everything that was treated as an essential requirement, in the case of email, we can follow the tests:
import java.util.regex.Pattern;
public class EmailValidator {
/**
* Email validation pattern.
*/
public static final Pattern EMAIL_PATTERN = Pattern.compile(
"[a-zA-Z0-9\\+\\.\\_\\%\\-\\+]{1,256}" +
"\\@" +
"[a-zA-Z0-9][a-zA-Z0-9\\-]{0,64}" +
"(" +
"\\." +
"[a-zA-Z0-9][a-zA-Z0-9\\-]{0,25}" +
")+"
);
private boolean isValid = false;
public boolean isValid() {
return isValid;
}
/**
* Validates if the given input is a valid email address.
*
* @param emailPattern The {@link Pattern} used to validate the given email.
* @param email The email to validate.
* @return {@code true} if the input is a valid email. {@code false} otherwise.
*/
public static boolean isValidEmail(CharSequence email) {
return email != null && EMAIL_PATTERN.matcher(email).matches();
}
}
See the unit test for this class:
class EmailValidatorTest {
@Test
void ensureThatEmailValidatorReturnsTrueForValidEmail() {
assertTrue(EmailValidator.isValidEmail("joe.junit@gmail.com"));
assertTrue(EmailValidator.isValidEmail("joe.vogel@analytics.gmail.com"));
}
}
Cool... but did you notice two asserts for this single test? Is this ideal? Let's talk about what each test should do.
Keep Asserts focused
The statements are essential for unit testing in any testing framework. An assertion is a statement of fact. So normally in our tests we expect a property to be true, or false, or a list of contacts and so on.
assertTrue(EmailValidator.isValidEmail("joe.junit@gmail.com"));
assertTrue(EmailValidator.isValidEmail("joe.unit@analytics.gmail.com"));
We seem to have two different cases here. In this case, the last assertion seems to test whether the subdomain is valid. This is a very simple example and it is easy to understand why the developer chose to put these two asserts in the same unit test, he just took advantage of the fact that the test is about success scenarios and put them all in one test. We see this a lot in the software industry. Is this a bad practice that can harm the whole application? Not in this context. The only care we should take is not to insert unnecessary asserts!
Very long unit tests, with many mocks and many asserts, can be confusing in complex systems! And this is true. The more complex the system gets as new features are finalized, our test suite will grow even longer. In the first example, it is easy to read and understand, we can easily modify this test and create one specific to the feature requirement:
@Test
void ensureThatEmailValidatorReturnsTrueForValidEmail() {
assertTrue(EmailValidator.isValidEmail("joe.junit@gmail.com"));
}
@Test
void ensureThatEmailValidatorReturnsTrueForValidEmailSubDomain() {
assertTrue(EmailValidator.isValidEmail("joe.unit@analytics.gmail.com"));
}
But look at the following scenario below:
public class MessagesSorterTest {
private final MessagesSorter messagesSorter = new MessagesSorter();
@Test
public void sortsMessagesByTimestampThenBySenderThenByReceiver() {
List<Message> unsortedMessages = asList(
new Message(250, "John", "Luke"),
new Message(100, "Benjamin", "Oliver"),
new Message(250, "John", "George"),
new Message(100, "Anthony", "Samuel")
);
List<Message> sortedMessages = messagesSorter.sort(unsortedMessages);
assertNotNull(sortedMessages);
assertEquals(4, sortedMessages.size());
assertEquals(100, sortedMessages.get(0).getTimestamp());
assertEquals("Anthony", sortedMessages.get(0).getSender());
assertEquals("Samuel", sortedMessages.get(0).getReceiver());
assertEquals(100, sortedMessages.get(1).getTimestamp());
assertEquals("Benjamin", sortedMessages.get(1).getSender());
assertEquals("Oliver", sortedMessages.get(1).getReceiver());
assertEquals(250, sortedMessages.get(2).getTimestamp());
assertEquals("John", sortedMessages.get(2).getSender());
assertEquals("George", sortedMessages.get(2).getReceiver());
assertEquals(250, sortedMessages.get(3).getTimestamp());
assertEquals("John", sortedMessages.get(3).getSender());
assertEquals("Luke", sortedMessages.get(3).getReceiver());
}
}
The question that should be on the mind of whoever is working on this class is, what is happening here? What scenario am I testing? What is the focus of this test?
Many questions, few answers, and sometimes several errors may appear. If the developer does not master the software and its rules, the scenario gets even more difficult. If this test fails, will it be easy to understand the real problem? An error similar to this one:
Expected: 100
Actual: 250
This a very general error, you will need to go to the test to check which line it failed on because the message will not tell you that. After you find out which line the test failed on, you still have no idea why. There is no overview and you cannot deduce which specific parts of the implementation are faulty.
This kind of test with many assertions costs time to read, understand and maintain. Usually, the time is very short and can directly affect the final delivery deadlines of the feature.
So as not to go on too long, I will leave here the link to the resolution of this problem. In the specific case of lists, the example of using too many asserts unnecessarily makes us reflect on how we test and whether or not it is necessary to apply a particular strategy. Let's take another topic, now on the front-end.
Keep your tests clear
Keeping your tests clear is a challenge, we have to admit that as the number of methods and classes increases, the more we need to refine our tests and make revisions on top of them. But many in the software development industry know that this is not possible. Too many factors can occur. Developers move in and out of the project, changing priorities and management... so it is our duty to defend the quality of testing and to strive to build quality tests.
it(`displays E-mail in use when email is not unique`, () => {
let httpTestingController = TestBed.inject(HttpTestingController);
const signUp = fixture.nativeElement as HTMLElement;
expect(signUp.querySelector(`div[data-testid="email-validation"]`)).toBeNull();
const input = signUp.querySelector(`input[id="email"]`) as HTMLInputElement;
input.value = "non-unique-email@mail.com";
input.dispatchEvent(new Event('input'));
input.dispatchEvent(new Event('blur'));
const request = httpTestingController.expectOne(({ url, method, body}) => {
if (url === '/api/1.0/user/email' && method === 'POST') {
return body.email === "non-unique-email@mail.com"
}
return false;
})
request.flush({});
fixture.detectChanges();
const validationElement = signUp.querySelector(`div[data-testid="email-validation"]`);
expect(validationElement?.textContent).toContain("E-mail in use");
})
What's too weird above? Lots of things. But what bothers me the most is the lack of division between where each test should be. The description of this unit test says: should display an already-used email message when the email is not unique. Inside the test we do two things, we test the component and a service! And this can cause a lot of confusion because a component should not know in depth about the implementation of a service. But the test shows us that there is no service in the app. Let's take a look at the tested component:
constructor(private userService: UserService, private uniqueEmailValidator: UniqueEmailValidator) { }
onClickSignUp(){
const body = this.form.value;
delete body.passwordRepeat;
this.apiProgress = true;
this.userService.signUp(body).subscribe({
next: () => {
this.signUpSuccess = true;
},
error: (httpError: HttpErrorResponse) => {
const emailValidationErrorMessage = httpError.error.validationErrors.email
this.form.get('email')?.setErrors({backend: emailValidationErrorMessage});
this.apiProgress = false;
}
});
}
We see that in the constructor there is a declaration of a service userService. But why in this case would a test be doing two things? Simulating requests and testing properties of the HTML and the component? Probably there was some oversight or no planning to create separate tests for the service! So this makes the test long and confusing with many responsibilities and uncertainties. The component test should be specific to the component and never simulate anything related to the service implementation, but should only mock its return, as we can see in the example below:
it("should call service perform login", () => {
component.form = new FormGroup({
email: new FormControl('developer.hashnode@gmail.com'),
password: new FormControl('P4ssword12@'),
})
let response = {
"email": "developer.hashnode@gmail.com",
"password": "P4ssword12@",
"id": 1
}
let spiedService = spyOn(service, 'login').and.returnValue(of(response))
component.isValidForm();
component.login()
expect(spiedService).toHaveBeenCalledTimes(1)
})
These are two different cases, login and signup, but nothing would prevent you from applying and using spyOn (SpyOn is a Jasmine feature that allows dynamically intercepting the calls to a function and changing its result).
So to improve the testing of the Signup feature we can simply add and remove unnecessary code and allocate what is requested to its proper test case and just handle and simulate responses:
spyOn(authService, 'signUp').and.returnValue(throwError({ status: 400, error: { validationErrors: { email: 'Email in use' } }}));
Let's review, in this test case what happens is that we don't need this in our test case:
const request = httpTestingController.expectOne(({ url, method, body}) => {
if (url == '/api/1.0/user/email' && method == 'POST') {
return body.email == "non-unique-email@mail.com"
}
return false;
})
Because we have spyOn that does this in a way that is easy to understand and doesn't cost us any reading time, plus the test doesn't get polluted!
But here I am just showing you one way, there are others, there may be a specific case that you want to use this way, but it mustn't become a common practice, long and confusing tests tend to lead other developers to possible mistakes.
Define description conventions
A unit test method name is the first thing any developer trying to understand your code will read. These method names communicate what the code does. A name should be concise, unambiguous, and consistent. That's why it's important to write descriptive method names that help readers quickly identify the purpose of a test case. A commonly used convention is this one below:
[MethodUnderTest]_[Scenario]_[ExpectedResult]:
MethodUnderTest is the name of the method you are testing.
Scenario is the condition under which you test the method.
ExpectedResult is what you expect the method under test to do in the current scenario.
This may be a good option for certain projects, but I believe there are better conventions. Let's understand what a test name should tell us before we read the code:
Separate words by underscores. This helps improve the readability of long names.
Name the test as if you were describing the scene to a non-programmer who is familiar with the problem domain. A domain expert or a business analyst are good examples
The test name should express a specific requirement
Describe the requirement under test
The more you can find the right words for a test, the clearer it becomes and the more reliable it becomes for other developers and non-programmers to understand what is being tested. It's easy to write this in an article, I know! Applying it in the industry is a big challenge, there are many hurdles to overcome and many have to do with an architect's or customer's choice.
I won't dive into this topic, because it is very much a matter of each project. I have seen many conventions applied and also tests that had none. But I will leave some suggestions for reading:
Test behavior, not just a code
This is a question we hear little about, but it is a valuable tip that I have learned from some book readings over the years and am starting to apply in the industry. As developers, we often worry about just testing the happy path and leave aside the more important tests, like testing the behavior of each method. If we focus on only testing functions, it's very easy to end up with a set of test cases that don't check all the important behaviors we care about.
The thought of testing the behaviors is also a great way to identify potential problems with the code. We will probably end up wondering what will happen in a method that is responsible for validating if a customer's expenses exceed their revenue. Or in a method, that must check whether or not to apply an adjustment to a certain employee.
Usually, we never think about testing the boundaries, like breaking such behavior in a class or method, because we are concerned with other issues and time can influence them as well. But here we can highlight some tips to understand if the methods are being tested for their behavior:
Could any arithmetic or logical operator be replaced by alternatives and still result in the tests passing? Examples could be changing the
&&to a||.Are there lines of code that could be excluded and still result in the code compiling and passing the tests?
If a variable has its state changed, does this affect the tests?
Each line, logical expression, and if-statement of a method must exist for a reason. If it is superfluous code, it is not being used and should be removed. If it is not superfluous, it means that there must be some important behavior. If there is an essential behavior, there must be a test case, so any change in the application's behavior must fail the test that validates the expected behavior. Otherwise, not all behaviors are being tested. But remember that refactorings that do not affect behavior should not affect the tests, if the test fails after a refactoring that does not change the observable behavior, we are probably having problems with these tests, it could be that the test is concerned with the implementation details. And that is not a good sign!
This goes for both front-end and back-end. Testing the behaviors should be a priority, as success and failure cases, true and false returns. Let's look at a frontend example that is quite common, going back to the login form example:
it('should return that form is valid', () => {
component.form.controls['email'].setValue('developer.hashs@gmail.com')
component.form.controls['password'].setValue('Password!3')
const result = component.isValidForm()
expect(result).toBeTrue()
})
There is nothing wrong with this test, but if we just use it to assume that all the behaviors of the login component are correct, we are just creating line-coverage tests. We want tests that go further, which assure us that the behaviors happen and in the right way. So the right thing to do would be to have tests for every expected behavior of the input email and password, also testing the limits.
Testing each behavior should be done clearly and objectively and for each behavior in software, we saw that earlier, we should keep our unit tests as focused as possible on each requirement. Requirements involve one or more behaviors depending on the context and the feature.
Testing behavior and testing the boundaries of each behavior are different issues. I don't want to confuse you at this point, so I am preparing two posts for each topic. What I am referring to here in this topic is that when testing tries to focus on the behaviors that your users will see in the application. It is not necessary to test the details of your code, such as how many times a variable is called and whether it is null or not. Worry about the inputs and outputs. Especially check the behavior of the application when invalid input is entered. When testing the limit of behavior, look for bugs, what the user could do unexpectedly, and look at the output that this generates. Again, these are different topics but they are connected to this theme, having these tests in our software adds value.
Resume
Everything we've seen throughout this post is intended to help you make decisions daily and avoid some common mistakes that you and I make. It's easy to write and difficult to apply many things. But we can always try and improve our unit tests by putting into practice what we read little by little. Talking with the team is also important to align with all conventions and standards that must be followed and respected for the good construction of unit tests. I also need to improve my tests, so I keep fighting my comfort zone and reading books on the subject and training weekly to improve over the years. We will never have perfect tests, but we can have more effective tests with a willingness to learn more.
If you want to leave constructive feedback, please leave it in the comments, if you want to share something, I am available for contact at the Linkedin described in my hash node profile. Thank you very much for reading until the end! See you in the next post!
References: