HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Writing better JUnit tests

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
junitwritingtestsbetter

Problem

I am using Spring Framework, Hibernate and JUnit. I am testing persisting of a UserEntity.

The UserEntity has the following associations:

  • ManyToMany - City



  • ManyToMany - ActivityCategory



-
ManyToOne - UserType

@Test
public void testCreateNotExistingEmail() {
    UserTypeEntity userTypeEntity = userTypeDao.find(1L);

    BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(16);
    String password = encoder.encode("123456");

    List cityEntities = new ArrayList<>();
    cityEntities.add(cityDao.find(1L));
    cityEntities.add(cityDao.find(2L));

    List activityCategoryEntities = new ArrayList<>();
    activityCategoryEntities.add(activityCategoryDao.find(4L));
    activityCategoryEntities.add(activityCategoryDao.find(5L));

    UserEntity userEntity = new UserEntity();
    userEntity.setEmail("user@domain.com");
    userEntity.setIsActive(true);
    userEntity.setPassword(password);
    userEntity.setUserTypeEntity(userTypeEntity);
    userEntity.setCities(cityEntities);
    userEntity.setActivityCategories(activityCategoryEntities);

    userDao.create(userEntity);

    UserEntity userEntityCreated = userDao.find(userEntity.getId());

    Assert.assertNotNull(userEntityCreated);
    Assert.assertEquals(2, userEntityCreated.getCities().size());
    Assert.assertEquals(2, userEntityCreated.getActivityCategories().size());
}


Is there anything I could do better in the test?

Solution

A minor remark: your test is named testCreateNotExistingEmail. The @Test annotation already indicates that this is a test so it doesn't have to be repeated in the methodname itself.

Adding to that: I like to structure my tests in the format [UnitOfWorkName]_[ScenarioUnderTest]_[ExpectedBehaviour].

It is hard to tell what exactly your test does when I look at the code because the methodname indicates something about a nonexisting email, which leads me to think about an exception handled somewhere.

But when I look at the code I see no assertions like that at all. In fact, I only see an email being used once in a seemingly irrelevant context.

Some type of clarification is needed: a better method name. Something along the lines of createUser_WithNonExistingEmail_ShouldReturn_StandardCitiesAndCategories

Now when the test fails amongst a big group of tests, you can pinpoint more closely what scenario is going wrong exactly before you dig into it.

Context

StackExchange Code Review Q#47660, answer score: 15

Revisions (0)

No revisions yet.