Unit test for a method that adds tweets to a database

Question

I have a class which inserts a list of tweets to a database table and also inserts the tweet's key with the associated filter's key to a many-to-many table. I am testing the sole public method of this class:

class TweetByFilterDatabaseRepository {

    private final DatabaseFacade database;

    private final Mapper<TweetEntity, ContentValuesWrapper> toTweetContentValuesMapper;

    private final TwoToOneMapper<SearchFilterEntity, TweetEntity, ContentValuesWrapper>
            toTweetAndSearchFilterContentValuesMapper;

    private final Mapper<Cursor, TweetEntity> toTweetMapper;

    TweetByFilterDatabaseRepository(DatabaseFacade database,
                                    Mapper<TweetEntity, ContentValuesWrapper> toTweetContentValuesMapper,
                                    TwoToOneMapper<SearchFilterEntity, TweetEntity, ContentValuesWrapper>
                                            toTweetAndSearchFilterContentValuesMapper,
                                    Mapper<Cursor, TweetEntity> toTweetMapper) {
        this.database = database;
        this.toTweetContentValuesMapper = toTweetContentValuesMapper;
        this.toTweetAndSearchFilterContentValuesMapper = toTweetAndSearchFilterContentValuesMapper;
        this.toTweetMapper = toTweetMapper;
    }

    public void addAll(SearchFilterEntity filter, List<TweetEntity> tweets) {
        database.beginTransaction();

        try {
            for (TweetEntity tweet : tweets) {
                try {
                    saveTweet(tweet);
                    saveTweetSearchFilterForeignKeys(filter, tweet);
                } catch (MappingException e) {
                    //no-op
                }
            }
            database.setTransactionSuccessful();
        } finally {
            database.endTransaction();
        }
    }

    private void saveTweet(TweetEntity tweet) throws MappingException {
        final ContentValuesWrapper tweetContentValues = toTweetContentValuesMapper.mapOrThrow(tweet);
        database.insertOrUpdate(TweetSchema.TABLE_NAME, tweetContentValues, TweetSchema.COLUMN_ID);
    }

    private void saveTweetSearchFilterForeignKeys(SearchFilterEntity filter,
                                                  TweetEntity tweet) throws MappingException {
        final ContentValuesWrapper tweetAndSearchFilterContentValues
                = toTweetAndSearchFilterContentValuesMapper.mapOrThrow(filter, tweet);
        database.insertOrIgnore(TweetSearchFilterSchema.TABLE_NAME, tweetAndSearchFilterContentValues);
    }
}

I want to test the two functionalities, that each Tweet is inserted to the database and that each pair is inserted to the database. I am testing the first functionality like so:

@RunWith(MockitoJUnitRunner.class)
public class TweetByFilterDatabaseRepositoryTest {

    @Mock private DatabaseFacade database;
    @Mock private Mapper<TweetEntity, ContentValuesWrapper> toTweetContentValuesMapper;
    @Mock private TwoToOneMapper<SearchFilterEntity, TweetEntity, ContentValuesWrapper>
            toTweetAndSearchFilterContentValuesMapper;
    @Mock private Mapper<Cursor, TweetEntity> toTweetMapper;
    private TweetByFilterDatabaseRepository tweetByFilterDatabaseRepository;

    @Before
    public void beforeEach() {
        tweetByFilterDatabaseRepository = new TweetByFilterDatabaseRepository(database, toTweetContentValuesMapper, toTweetAndSearchFilterContentValuesMapper, toTweetMapper);
    }

    @Test
    public void addingAllItems_withoutErrors_addsTweetsToDatabase() throws MappingException {
        SearchFilterEntity filter = mock(SearchFilterEntity.class);
        List<TweetEntity> tweets = mockedListOf(TweetEntity.class);
        List<ContentValuesWrapper> contentValues = mockedListOf(ContentValuesWrapper.class);
        for (int i = 0; i < tweets.size(); i++) {
            TweetEntity tweet = tweets.get(i);
            when(toTweetContentValuesMapper.mapOrThrow(tweet)).thenReturn(contentValues.get(i));
        }

        tweetByFilterDatabaseRepository.addAll(filter, tweets);

        for (int i = 0; i < tweets.size(); i++) {
            TweetEntity tweet = tweets.get(i);
            ContentValuesWrapper contentValuesWrapper = contentValues.get(i);
            verify(toTweetContentValuesMapper).mapOrThrow(tweet);
            verify(database).insertOrUpdate(TweetSchema.TABLE_NAME, contentValuesWrapper, TweetSchema.COLUMN_ID);
        }
    }
}

Is there a better way to test this without the for loops or is maybe my design flawed? I am primarily interested in removing the loops from the unit test but I am also interested in improving the design of the main class if it is needed.

Answer 1

Disclaimer: I'm not a huge fan of mocks.

Some issues I see with the approach:

• The fixture setup is quite complicated, involving a lot of mocks. It's a lot to read.
• Using loops in test methods is a test smell.
• Verifying that .mapOrThrow gets called on each tweet, and verifying that insertOrUpdate gets called with specific parameters on database call for concern. The test knows too much about intimate details of the implementation. As such, the tests are too tightly coupled to the implementation, and very likely fragile tests (another test smell). The slightest change in implementation details will likely break tests, and become a maintenance nightmare.

I suggest to take a different approach: use a fake database. With an in-memory implementation of DatabaseFacade, the test class could become a lot simpler, easier to read, and eliminate the current test smells.

//@RunWith(MockitoJUnitRunner.class) -> not needed anymore -> to delete
public class TweetByFilterDatabaseRepositoryTest {

    @Test
    public void addingAllItems_withoutErrors_addsTweetsToDatabase() {
        DatabaseFacade database = newFakeDatabase();
        SearchFilterEntity filter = mock(SearchFilterEntity.class);
        List<TweetEntity> tweets = mockedListOf(TweetEntity.class);

        tweetByFilterDatabaseRepository.addAll(filter, tweets);

        assertEquals(tweets, database.getTweets());
    }
}

I mentioned at the top that I don't like mocks. But notice that I didn't eliminate all the mocks. I only eliminated the ones that didn't help reducing complexity. I kept the ones that are useful, and I have no problems with those.

It should be straightforward to add further tests for the cases of some tweets with mapping errors, all tweets with mapping errors, filtering logic, and so on.

Answer 2

One improvement would be to extract mappers part to separate class. Then, you could add addAll(List<ContentValuesWrapper> contents) method to your repository and verify invocations of that single method. That would probably simplify testing a little bit.