Java 8 unit test for third-party data feeds

Question

Use Case

I have a service that does processing of records from multiple third-party feeds. The steps are generally identical for each feed, but each feed is populated in a different location on an aggregate feed. I decided to write a generic test that would allow testing of all this processing to keep things DRY.

Test

The test basically tests whether the processor method in our service properly adapts and adds the incoming record to the appropriate aggregate list.

@Mock AggregateFeed dataFeed; // contains lists of POJOs
@Mock Exchange exchange; // camel mock.. not really important

@Mock SportsAdapter sportsAdapter; // used to convert string to POJO
@Mock BusinessAdapter businessAdapter; // used to convert string to POJO

FeedService fixture = new FeedService();

...

private <T1, T2> void shouldProcessRecord(BiConsumer<T1, Exchange> processor,
        T1 incomingRecord, T2 fakeAdaptee, Function<T1, T2> mockAdapter,
        Function<AggregateFeed, List<T2>> list) {
    List<T2> fakeList = Lists.newArrayList();

    when(mockAdapter.apply(incomingRecord)).thenReturn(fakeAdaptee);
    when(list.apply(dataFeed)).thenReturn(fakeList);

    processor.accept(incomingRecord, exchange);

    assertEquals(1, fakeList.size());
    assertSame(fakeAdaptee, fakeList.get(0));
}

@Test
public void shouldProcessSports() {
    String testRecordFeed = constructSportsRecordFeed();
    SportsRecord fakeSportsRecord = new SportsRecord();
    shouldProcessRecord(fixture::addSportsRecord, testRecordFeed, 
        fakeSportsRecord, sportsAdapter::fromFeed, AggregateFeed::getSports);
}

@Test
public void shouldProcessBusiness() { 
    String testRecordFeed = constructBusinessRecordFeed();
    BusinessRecord fakeBusinessRecord = new BusinessRecord();
    shouldProcessRecord(fixture::addBusinessRecord, testRecordFeed, 
        fakeBusinessRecord, businessAdapter::fromFeed, AggregateFeed::getBusiness);
}

// builds up a csv delimited string that represents a sports article
private String constructSportsRecordFeed() {
    return new StringBuilder().append( ... ).toString();
}

// builds up a csv delimited string that represents a business article
private String constructBusinessRecordFeed() {
    return new StringBuilder().append( ... ).toString();
}

Concerns

The main problem I have doing things this way is that the signature just doesn't look natural. If another developer comes to create a test, they'll have to dig through the code to figure out how to use this test. On the other hand, it's much easier to do a copy and paste of the test and change around the variables, but that doesn't feel right either because it's not DRY.

Questions

• How can this generic test be improved to be easier to use?
• Since this is a test, should a copy-and-paste methodology be better since it's more direct on what the test does?
• Any other advice/comments would be greatly appreciated.

Answer 1

I am only going to talk about the sports method, from there you should be able to get the info you need for the others as they are all basically the same.

DRY isn't a big priority

As I said in my earlier comment, DRY (don't repeat yourself) principle applies to production code more than unit test code. I think even in general it's easy to take this too far, readability is important, but that's off-topic. Your code however is tough to grasp.

To me, something like the below is more clear. I like to specify "given/when/then" in comments (although usually I do nothing more than // Given, etc.) but it's not necessary.

@Test
public void shouldProcessSports() {

    List<SportsRecord> fakeList = Lists.newArrayList();

    // Given an incoming feed of the sports variety
    String testRecordFeed = constructSportsRecordFeed();
    SportsRecord fakeSportsRecord = new SportsRecord();

    Function<AggregateFeed, List<SportsRecord>> list = AggregateFeed::getSports;

    when(mockAdapter.apply(testRecordFeed)).thenReturn(fakeSportsRecord);
    when(list.apply(dataFeed)).thenReturn(fakeList); // (*) See note below

    // When the processor accepts the feed and exchange
    processor.accept(testRecordFeed, exchange);

    // Then there should be one result and it should be the sports record
    assertEquals(1, fakeList.size());
    assertSame(fakeSportsRecord, fakeList.get(0));
}

Note: I don't have much experience with the :: operator, if AggregateFeed::getSports.apply(dataFeed) or something similar is valid I would prefer that instead of making the list variable.

DRY is still relevant though!

While writing tests like this takes up more space I think it's more important that the test be clear.

All that said I don't think DRY is completely useless in unit tests. Suppose you were doing more checks on fakeList, like making sure it's not null, contains a specified number of elements, etc. I would put all those assertions in a separate method. Basically the assertions that are tangential to what you are testing. For example, you're not specifically testing the list has one element above, the assertion you care most about is that the only element is fakeSportsRecord. You can't really do something like that so you have to assert that the first element is fakeSportsRecord. Anyways, I tend to put these tangential assertions in methods like this:

private static void verifyListExistsAndHasSingleElement(List<?> list) {
    assertNotNull(list);
    assertEquals(1, list.size());
}

Naming might be a little poor.

The name are mostly fine, but I felt like I needed to add more.

• fakeList → resultList or outputList
• testRecordFeed → incomingRecordFeed or givenRecordFeed
• fakeSportsRecord → expectedSportsRecord
• list, this is by far the worst. As shown in the note I don't know if this variable is even necessary. Even just function is better because it's a function, not a list.