2
\$\begingroup\$

I want to write unit-test to my code. However, while writing it, it seems all I can verify is that one method or the other were called once.

My class is a manager that execute verification flow. It calls many checkers\verifiers - so they have the real logic. How can I test such "verification flow" code?

public AlternativeShortTotalChecker(SegmentsIdsChecker segmentsIdsChecker, InstructionsChecker instructionsChecker,
                                    RouteNameChecker routeNameChecker, AlternativeDistanceChecker
                                            alternativeDistanceChecker, AlternativeTimeChecker
                                            alternativeTimeChecker,
                                    IDeltaFromBaselineGenerator deltaFromBaselineGenerator,
                                    AStarVisitedChecker aStarVisitedChecker, GlobalSettings globalSettings) {
    this.segmentsIdsChecker = segmentsIdsChecker;
    this.instructionsChecker = instructionsChecker;
    this.routeNameChecker = routeNameChecker;
    this.alternativeDistanceChecker = alternativeDistanceChecker;
    this.alternativeTimeChecker = alternativeTimeChecker;
    this.deltaFromBaselineGenerator = deltaFromBaselineGenerator;
    this.aStarVisitedChecker = aStarVisitedChecker;
    this.globalSettings = globalSettings;
}


public List<String> checkAllAlternativesShort(CompleteRoutingResponseShort baselineRoutingResponse,
                                              CompleteRoutingResponseShort
                                                      routingResponse) {
    int numOfAlts = routingResponse.alternatives.size();
    List<String> testFailureMessages = new ArrayList<>();
    for (int i = 0; i < numOfAlts; i++) {
        ResponseShort currentResponse = getAlternativeShort(routingResponse, i).response;
        ResponseShort baselineResponse = getAlternativeShort(baselineRoutingResponse, i).response;
        AlternativeShort alternativeShort = getAlternativeShort(routingResponse, i);
        String testFailureMessage = checkAlternative(alternativeShort.alternative_regression_id, currentResponse,
                baselineResponse);
        testFailureMessages.add(testFailureMessage);
    }
    return testFailureMessages;
}

private AlternativeShort getAlternativeShort(CompleteRoutingResponseShort routingResponse, int i) {
    return routingResponse.alternatives.get(i);
}

private String checkAlternative(String currentAltId, ResponseShort currentResponse,
                                ResponseShort baselineResponse) {
    IndividualStat individualStat = new IndividualStat(currentAltId);
    aStarVisitedChecker.setForAltDiff(globalSettings.astarVisited_altDiffHighThreshold);
    individualStat.aStarVisitedDelta = aStarVisitedChecker.checkAStarCountPerAlt(currentResponse, baselineResponse);

    individualStat.segmentsIdsDelta = segmentsIdsChecker.checkSegmentsIdsUnorderedIdentity(currentResponse,
            baselineResponse);
    switch (individualStat.segmentsIdsDelta.delta) {
        case IDENTICAL: {
            individualStat.areInstructionsIdentical = instructionsChecker.checkTotalInstructionIdentity
                    (currentResponse, baselineResponse);
            individualStat.areRouteNamesIdentical = routeNameChecker.checkRouteNameIdentity(currentResponse,
                    baselineResponse);
            break;
        }
        case SMALL:
        case LARGE: {
            individualStat.distancesDelta = alternativeDistanceChecker.checkTotalDistanceIdentity
                    (currentResponse, baselineResponse);
            individualStat.timesDelta = alternativeTimeChecker.checkTotalTimeIdentity(currentResponse,
                    baselineResponse);
            break;
        }
    }
    return deltaFromBaselineGenerator.addDeltaForSpecificAlternative(individualStat);
}

My try for UT. Would you skip testing this code?

@Test
public void checkAltWith2() throws Exception {


    DescriptiveDelta descriptiveDelta = new DescriptiveDelta();
    descriptiveDelta.delta = Delta.IDENTICAL;
    when(segmentsIdsCheckerMock.checkSegmentsIdsUnorderedIdentity(any(ResponseShort.class), any(ResponseShort.class)))
            .thenReturn(descriptiveDelta);

    AlternativeShortTotalChecker alternativeShortTotalChecker =
            new AlternativeShortTotalCheckerBuilder()
                    .setAlternativeDistanceChecker(alternativeDistanceCheckerMock)
                    .setAlternativeTimeChecker(alternativeTimeCheckerMock)
                    .setAStarVisitedChecker(aStarVisitedCheckerMock)
                    .setDeltaFromBaselineGenerator(deltaFromBaselineGeneratorMock)
                    .setGlobalSettings(globalSettings)
                    .setInstructionsChecker(instructionsCheckerMock)
                    .setRouteNameChecker(routeNameCheckerMock)
                    .setSegmentsIdsChecker(segmentsIdsCheckerMock)
                    .createAlternativeShortTotalChecker();


    CompleteRoutingResponseShort currentResponse = routingResponsesInMemoryFactory.getOneAlternative();
    CompleteRoutingResponseShort baselineResponse = routingResponsesInMemoryFactory.getOneAlternative();

    alternativeShortTotalChecker.checkAllAlternativesShort(currentResponse, baselineResponse);

    verify(aStarVisitedCheckerMock).checkAStarCountPerAlt(any(ResponseShort.class), any(ResponseShort.class));
    verify(segmentsIdsCheckerMock).checkSegmentsIdsUnorderedIdentity(any(ResponseShort.class), any(ResponseShort
            .class));
    verify(instructionsCheckerMock).checkTotalInstructionIdentity(any(ResponseShort.class), any(ResponseShort.class));
    verify(routeNameCheckerMock).checkRouteNameIdentity(any(ResponseShort.class), any(ResponseShort.class));
}
\$\endgroup\$
5
\$\begingroup\$

I see no reason not to test this class. It exposes a public method that operates upon its arguments using non-trivial logic.

To decide how to test it, you must consider what the method does, step-by-step, and determine which parts are worth testing. Reading the checkAllAlternativesShort method from start to finish:

  1. It loops X times around a for-loop, based on the result of routingResponse.alternatives.size(). So your test will probably want to confirm that it did indeed process all the items correctly.

  2. After gathering a few bits of data, it then calls out to checkAlternative, which contains quite a lot of logic. It's beyond the scope of my review to explain quite how to test this, but presumably you would want to confirm this method operates correctly upon its inputs.

    In addition, this method changes behaviour depending upon the value of individualStat.segmentsIdsDelta.delta. I guess you should be testing what happens for each possible value (IDENTICAL, SMALL, LARGE).

    You might choose to expose checkAlternative as a package-private method, to allow you to test it directly. It really depends how unwieldy it is to pass the right information into checkAllAlternativesShort.

  3. The result of this method is a list of test failure messages. I would suggest that you test, for a given set of inputs, that you get an expected number of outputs. I concede that the real logic lies in different classes, but it's still the responsibility of this class to gather the messages together and return them correctly. Write a test that ensures this happens.

General observations:

  • You have a lot of direct field accesses (someVariable.field). I would consider adding more getter methods to be in line with OOP principles.

  • Your switch statement doesn't have a default class. That's not necessarily criminal behaviour, but it might flag up in some code quality scans. Your code might be more maintainable if you add a default clause, even if it's empty and commented:

    default:
      // No need to do anything
      break;
    
  • Your getAlternativeShort method seems unnecessary. It doesn't make your code any more compact or readable. I would consider removing it and just calling .alternatives.get(i) directly on those objects.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.