This talk will cover tools and techniques for writing tests that are a pleasure to read, easy to maintain, and prove their worth time and time again.
We’ll cover the following:
- Readability
- Brittleness: breaking every time the implementation is changed
- Non-obvious cause and solution upon test failure
- Unreliability
- Slow execution
We’ll improve these tests in JUnit, move on to Hamcrest and AssertJ, then show how Spock can take us even further to tests that are easily maintainable and incredibly useful.
Introduction
I’m John Reeve, and I’m a freelance Android developer. I often see bad tests that are massive blocks of code. Though they do have value, they’re really quite appallingly written tests.
Example - Bad Testing
A bad test is one that has too many comments. If the test is readable, it shouldn’t need so many comments:
@Test
public void sharedPreferencesHelper_SaveAndReadPersonalInformation() {
// Save the personal information to SharedPreferences
boolean success = mMockSharedPreferencesHelper.savePersonalInfo(mSharedPreferenceEntry);
assertThat("Checking that SharedPreferenceEntry.save... returns true", success, is(true));
// Read personal information from SharedPreferences
SharedPreferenceEntry savedSharedPreferenceEntry = mMockSharedPreferencesHelper.getPersonalInfo();
// Make sure both written and retrieved personal information are equal.
assertThat("Checking that SharedPreferenceEntry.name has been persisted and read correctly", mSharedPreferenceEntry.getName(), is(equalTo(savedSharedPreferenceEntry.getName())));
assertThat("Checking that SharedPreferenceEntry.dateOfBirth has been persisted and read " + "correctly", mSharedPreferenceEntry.getDateOfBirth(), is(equalTo(savedSharedPreferenceEntry.getDateOfBirth())));
assertThat("Checking that SharedPreferenceEntry.email has been persisted and read " + "correctly", mSharedPreferenceEntry.getEmail(), is(equalTo(savedSharedPreferenceEntry.getEmail())));
}
Many tests seem to be testing lots of things at once. They have failure messages because they use standard JUnit assertions, which is unnecessary noise.
The following is the method that’s being tested:
public boolean savePersonalInfo(SharedPreferenceEntry sharedPreferenceEntry){
// Start a SharedPreferences transaction.
SharedPreferences.Editor editor = mSharedPreferences.edit();
editor.putString(KEY_NAME, sharedPreferenceEntry.getName());
editor.putLong(KEY_DOB, sharedPreferenceEntry.getDateOfBirth().getTimeInMillis());
editor.putString(KEY_EMAIL, sharedPreferenceEntry.getName());
// Commit changes to SharedPreferences.
return editor.commit();
}
You can run this unit test, and it will pass. But, this is a useless unit test as it’s convincing us everything is okay when in fact it’s not, giving us false confidence.
Improving the Example
The main parts of the test are saving and reading - this can be separated.
@Test
public void savesPersonalInformation() {
// Given
sharedPrefsSaveWillSucceed():
// When
boolean saveSuccess = sut.savePersonalInfo(mSharedPreferenceEntry);
// Then
verify(mMockEditor).putString(KEY_NAME, mSharedPreferenceEntry.getName());
verify(mMockEditor).putString(KEY_EMAIL, mSharedPreferenceEntry.getEmail());
verify(mMockEditor).putLong(KEY_DOB, mSharedPreferenceEntry.getDateOfBirth().getTimeInMillis());
verify(mMockEditor).commit();
assertThat(saveSuccess, is(true));
}
@Test
public void readsPersonalInformation() {
// Given
givenSharedPrefsContains(mSharedPreferenceEntry);
// When
SharedPreferenceEntry retrievedInfo = sut.getPersonalInfo();
// Then
assertThat(retrievedInfo.getName(), is(equalTo(mSharedPreferenceEntry.getName())));
assertThat(retrievedInfo.getDateOfBirth(), is(equalTo(mSharedPreferenceEntry.getDateOfBirth())));
assertThat(retrievedInfo.getEmail(), is(equalTo(mSharedPreferenceEntry.getEmail())));
We can further improve this. In savesPersonalInformation(), there are two separate tests.
// Given
sharedPrefsSaveWillSucceed():
// When
boolean saveSuccess = sut.savePersonalInfo(mSharedPreferenceEntry);
assertThat(saveSuccess, is(true));
This is checking that all the information have been written correctly, and that the return value is correct. This can also be two separate tests. If one of these fails, you’ll know what the problem is.
Even after splitting up the tests, there is this:
verify(mMockEditor).putString(KEY_NAME, mSharedPreferenceEntry.getName());
verify(mMockEditor).putString(KEY_EMAIL, mSharedPreferenceEntry.getEmail());
verify(mMockEditor).putLong(KEY_DOB, mSharedPreferenceEntry.getDateOfBirth().getTimeInMillis());
verify(mMockEditor).commit();
The mini wall of code can be further abbreviated, by making use of a matcher:
@Test
public void savesPersonalInformation() {
// Given
sharedPrefsSaveWillSucceed():
// When
sut.savePersonalInfo(mSharedPreferenceEntry);
// Then
assertThat(mMockEditor, savedAllThePersonalInfo());
}
Overall, I’ve found that many people end up writing tests that are trying to look clean, like this:
@Test
public void givenNoCriteriaWhenFindingMessagesForCleaningThenResultShouldBeEmpty()
throws InterruptedException {
givenNoCriteria();
whenFindingMessagesForCleaning();
thenResultsShouldBeEmpty();
}
But, there is nothing here that conveys what it’s testing, how it’s testing it, or how anything fits together. I have no extra confidence from looking at this, and if this test fails, I have no idea what it does, so I won’t know how to fix it.
After examining the body methods, we’d just end up with this.
messagesInfoResult = messagesFinder.findMessagesForCleaning(NO_CRITERIA);
It’s good to aim for this readable code, but not at the expense of all of the information, particularly something I describe as the nouns. The nouns are what is being tested along with its collaborators.
After a final fix, it is more readable than before, and it tells us what it’s doing.
@Test
public void givenNoCriteriaWhenFindingMessagesForCleaningThenResultShouldBeEmpty()
throws InterruptedException {
assertThat(sut.findMessagesForCleaning(NO_CRITERIA)).isEmpty();
}
Another Example
This test deals with users and jobs in a system.
@Test
public void givenUsersAndJobs_whenGettingJobsForMultipleUsers_thenWeGetJobsMappedToExternalUsers() {
givenJobs(Arrays.asList(ANY_JOB, ONE_MORE_JOB), ANY_USER);
givenJobs(Collections.singletonList(YET_ANOTHER_JOB), YET_ANOTHER_USER);
TestSubscriber<Map.Entry<String, List<Job>>> testSubscriber = new TestSubscriber<>(); List<String> someSystemUsers = Arrays.asList(ANY_USERNAME, YET_ANOTHER_USERNAME);
serviceClient.getUsersJobsByExternalNames(someSystemUsers, ANY_START_DATE, ANY_END_DATE) .subscribeOn(Schedulers.immediate()) .subscribe(testSubscriber);
List<Map.Entry<String, List<Job>>> expectedJobs = Arrays.asList(
(Map.Entry<String, List<Job»)new AbstractMap.SimpleImmutableEntry<>(
ANY_USERNAME, Arrays. asLi st(ANY_JOB, ONE_MORE_JOB)
),
new AbstractMap. SimplelmmutableEntry<>(
YET_ANOTHER_USERNAME, Collections.singletonList(YET_ANOTHER_30B)
)
);
testSubscriber.assertReceivedOnNext(expectedJobs);
}
The scenario this part is covering is along the lines of “given when, then…”
serviceClient.getUsersJobsByExternalNames(someSystemUsers, ANY_START_DATE, ANY_END_DATE) .subscribeOn(Schedulers.immediate()) .subscribe(testSubscriber);
I can rearrange this and put comments just to mark the blocks.
@Test
public void givenUsersAndJobs_whenGettingJobsForMultipleUsers_thenWeGetJobsMappedToExternalUsers() {
// Given
givenJobs(Arrays.asList(ANY_JOB, ONE_MORE_JOB), ANY_USER);
givenJobs(Collections.singletonList(YET_ANOTHER_JOB), YET_ANOTHER_USER);
TestSubscriber<Map.Entry<String, List<Job>>> testSubscriber = new TestSubscriber<>(); List<String> someSystemUsers = Arrays.asList(ANY_USERNAME, YET_ANOTHER_USERNAME);
List<Map.Entry<String, List<Job>>> expectedJobs = Arrays.asList(
(Map.Entry<String, List<Job»)new AbstractMap.SimpleImmutableEntry<>(
ANY_USERNAME, Arrays. asLi st(ANY_JOB, ONE_MORE_JOB)
),
new AbstractMap. SimplelmmutableEntry<>(
YET_ANOTHER_USERNAME, Collections.singletonList(YET_ANOTHER_30B)
)
);
// When
serviceClient.getUsersJobsByExternalNames(someSystemUsers, ANY_START_DATE, ANY_END_DATE) .subscribeOn(Schedulers.immediate()) .subscribe(testSubscriber);
// Then
testSubscriber.assertReceivedOnNext(expectedJobs);
}
The subscription portion in the test is noise. We can use AssertJ to build a simple construction that will return itself with a building method.
public static <T> TestSubscriberAssert<T> assertThatASubscriberTo(Observable<T> observable) {
TestSubscriber<T> subscriber = new TestSubscriber<>();
observable
.subscribeOn(Schedulers.immediate())
.subscribeOn(subscriber);
return new TestSubscriberAssert<T>(subscriber);
}
With this method, you can assert things on it. In this case the // When block becomes much simpler like so:
// When
Observable<Map.Entry<String, List<Job>>> observable =
sut.getUsersJobsByExternalNames(someSystemsUsers, ANY_START_DATE, ANY_END_DATE)
// Then
testSubscriber.assertReceivedOnNext(expectedJobs);
The parts of the test with maps, lists and generics can be hidden by extracting methods. We could extract a method that would give us a list or a map entry.
public void givenUsersAndJobs_whenGettingJobsForMultipleUsers_thenWeGetJobsMappedToExternalUsers() {
// Given
givenJobs(aList(ANY_JOB, ONE_MORE_JOB), ANY_USER);
givenJobs(aList(YET_ANOTHER_JOB), YET_ANOTHER_USER);
List<String> someSystemUsers = aList(ANY_USERNAME, YET_ANOTHER_USERNAME);
List<Map.Entry<String, List<Job>>> expectedJobs = aList(
aMapEntry(ANY_USERNAME, aList(ANY_JOB, ONE_MORE_JOB)),
aMapEntry(YET_ANOTHER_USERNAME, aList(YET_ANOTHER_JOB))
);
// When
Observable<Map.Entry<String, List<Job>>> observable =
sut.getUsersJobsByExternalNames(someSystemsUsers, ANY_START_DATE, ANY_END_DATE)
// Then
testSubscriber.assertReceivedOnNext(expectedJobs);
}
After inlining these, we would end up with this:
// When / Then
assertThatASubscriberTo(
sut.getJobsForSystemUsers(someSystemUsers, ANY_START_DATE, ANY_END_DATE)
).received(aList(
aMapEntry(ANY_USERNAME, aList(ANY_JOB, ONE_MORE_JOB)),
aMapEntry(YET_ANOTHER_USERNAME, aList(YET_ANOTHER_JOB))
) ) ;
I would change the top part to put the user first, then rename the method to make it more obvious that we’re setting up jobs for this user:
givenJobsForUser(ANY_USER, ANY_JOB, ONE_MORE_JOB);
givenJobsForUser(YET_ANOTHER_USER, YET_ANOTHER_JOB);
Test Data Class
By creating test data class for test data, it will allow a lot more syntactic sugar to run our tests. Instead of having the constants in our test, we can put them into structures that mirror real data in our system.
The data can be better structured. Instead of saying “given this user, we have these jobs,” it can instead have the jobs for user one, and we would be able to extract data from it.
@Test
public void findsJobsForUsersByExternalUsernames() {
// Given
given(jobService)
.hasJobsFor(USER_1);
.hasJobsFor(USER_2);
given(usernameMappingService)
.hasMapping(USER_1.externalUsername(), USER_1.username());
.hasMapping(USER_2.externalUsername(), USER_2.username());
List<String> externalUsernames =
aList(USER_1.external_username(), USER_2.externalUsername());
// When / Then
assertThatASubscriberTo(
sut.getUsersJobsByExternalNames(externalUsernames, ANY_START_DATE, ANY_END_DATE)
).receiyed(aList(
aMapEntry(USER_1.username(), USER_1.jobs),
aMapEntry(USER_2.username(), USER_2.jobs)
));
}
Spock
Spock is a Groovy based testing framework. These are the sort of tests you see in Spock:
def "subscribers see published events at least once"() {
when: publisher.send(event)
then: (1.._) * subscriber.receive(event)
where: event << ["started", "paused", "stopped"]
}
def "copes with misbehaving subscribers"() {
given: firstSubscriber.receive(_) » { throw new Exception() }
when:
publisher.send("eventl")
publisher. send("event2")
then:
1 * latterSubscriber.receive("eventl")
1 * latterSubscriber.receive("event2")
}
The user labels here have meaning, and because it’s Groovy, it can rearrange the abstract syntax tree and can set up mocks. For example, I specify when this event is going to be a specific value, then another value, and another value.
It also supports Hamcrest with special methods if you’re using Hamcrest matches. It supports DSL metaprogramming in a way similar to Kotlin.
Kotlin allows you to use a lot of syntactic sugar, but there are one or two things that this does that Kotlin won’t do, such as defining extension methods.
It can also do stuff like this:
when: flippable.flip()
then: flippable.front == old(flippable.back)
and: flippable.back == old(flippable.front)
You could say that this value equals the old value of that field, and you won’t have to store the value to compare it later.
Most amazingly, it gives you amazing failure messages:
Instead of merely displaying inequality, it will go the distance to note that have a 62% similarity between two strings.
You can set up spies like this:
def "uses spy"() {
when: thing.doSomethingWith(collaborator)
then: 1 * collaborator.callWeExpect()
}
You can set up stubbing like this:
def "uses stub"() {
given: numberGenerator.getValue() >> 4
when: result = thing.doThingWith(numberGenerator)
then: result == 42
}
You could set up wildcards and use an underscore here:
_.reveive(event) // reveive is called on any mock object
0 * _._ // No more interactions
0 * _ // Same ^^^
Ranges:
3 * subscriber.receive(event) // exactly 3 times
(1.._) * subscriber.receive(event) //at least 1 time
(_..2) * subscriber.receive(event) // at most 2 times
I suggest giving Spock a look because it may be something you’re interested in, or you may see a feature you’d like to request the developers of other frameworks to add.
About the content
This talk was delivered live in March 2017 at DroidKaigi. The video was recorded, produced, and transcribed by Realm, and is published here with the permission of the conference organizers.
