# Two BDD unit tests for an account repository with identical code

I am testing my repository class:

interface AccountRepository{
void save(Account account);
Optional<Account> findById(Long id);
}


And when writing tests, I ended up with two semantically different tests, but they essentially contain exactly the same code:

@DisplayName("user exists -- account does not exist -- save persists")
@Test
void save_userExistsAccountDoesNotExist_persisted() {
//arrange
User existingUser = userRepo.save(new User("John", "Smith"));
Account account = new Account(existingUser, null);
//act
repo.save(account);
//assert
assertThat(repo.findById(account.getId())).contains(account);
}

@DisplayName("user exists -- account exists -- findById finds")
@Test
void findById_userExistsAccountExists_found() {
//arrange
User existingUser = userRepo.save(new User("John", "Smith"));
Account existingAccount = repo.save(new Account(existingUser, null));
//act & assert
assertThat(repo.findById(existingAccount.getId())).contains(existingAccount);
}


I don't really want to remove one of them because that serves different purposes in my head but I am worried that I may have understood something wrong.

## 2 Answers

In my opinion, if you think that each test has a different meaning, there is nothing wrong with them.

What you can do to extract the duplication is to have a dedicated method to arrange your test. But in this case there will be no real gain.

• @Sam, please consider accepting an answer if you have one. Jan 10 '19 at 15:08
• It's very reasonable to wait more than three hours to see if a better reply come in before accepting the first answer. Jan 10 '19 at 15:24

One of the hardest piece of knowledge to learn as a developer is: code duplication does not mean responsibility duplication.

Having said this, you are testing different methods so it's ok to have more than one test. In this case, you are testing complementary (or were they supplementary? I never remember it) methods, so to test one, you need to have the other one working properly.

However, there are some code duplication that will bite you in the future (but since this question is from the past, you can tell me if I'm wrong or not).

1. You are duplicating the way a User entity is created, so if you change its constructor, you will have to modify your tests, even though it might not matter in the actual logic you are testing. It happens something similar with the Account entity. To avoid this you can have a private method or to a public builder.
2. You are duplicating the way a User is created and persisted. The same problem as in point #1 happens. You probably don't care how or when is created, you just need it to be created and persisted properly. This applies also for the Account entity with a slight different: in the first test you need to make the code of save into the repo explicit since it's what you are testing.

Besides all of this, when you are writing tests you need to tell the reader what's important and what's not and describe that accordingly. It's a balance between explicitness and abstraction. For instance in the example, I've written a method called createAccount to encapsulate the logic of creation and persistence of the Account entity. However, someone would say that if a developer reading the test needs to know what the arrange does, it has to move to the method implementation (a test should be self-explanatory and auto).

@DisplayName("user exists -- account does not exist -- save persists")
@Test
void save_userExistsAndAccountDoesNotExist_accountIsSaved() {
// Arrange
User existingUser = createUser();
Account createdAccount = makeAccount(existingUser);

// Act
repo.save(createdAccount);

// Assert
assertThat(repo.findById(createdAccount.getId())).contains(createdAccount);
}

@DisplayName("user exists -- account exists -- findById finds")
@Test
void findById_userExistsAndAccountExists_returnTheAccount() {
// Arrange
User existingUser = createUser();
Account existingAccount = createAccount(existingUser);

// Act
Optional<Account> foundAccount = repo.findById(existingAccount.getId());

// Assert
assertThat(foundAccount).contains(existingAccount);
}

private User createUser(){
return userRepo.save(new User("John", "Smith"));
}

private Account createAccount(User existingUser){
return repo.save(makeAccount(existingUser));
}

private Account makeAccount(User user){
new Account(user, null);
}

• I don't get point #2. Isn't it more readable if you call the repositories in the tests? Let's just have createUser and createAccount and then explicitly call repositories if we need to
– Sam
May 22 '19 at 8:53
• That was exactly what I was trying to explain the following paragraph: you have to find the balance between duplication, coupling, expressiveness, conciseness... It also depends on the business, the project, the team, the language... May 22 '19 at 13:11