Is this test for callout wrong? How to make a bulk?

Question

I wrote mock and test for callout, but I don't think it's right. I think I check wrong things with asserts and maybe call the same methods a few times, while have another ways to get 100%. Have I?

I also still don't know how to make a bulk test for my batch. Thanks for any help.

Callout:

public class CallThePepperoni {
public List<Pepperoni__c> pepperoniList;
public HttpResponse responseService;

public static HttpRequest createRequestForToken(){
    HttpRequest ourRequest = new HttpRequest();
    ourRequest.setBody(myTokenString);
    ourRequest.setMethod('GET');
    ourRequest.setEndpoint('https://myorg.salesforce.com/services/oauth2/token');
    return ourRequest;
}

public static HttpRequest createRequestForService(String token){
    HttpRequest finalRequest = new HttpRequest();
    finalRequest.setHeader('Authorization','Bearer ' + token);
    finalRequest.setHeader('Content-Type','application/json');
    finalRequest.setHeader('accept','application/json');
    finalRequest.setMethod('GET');
    finalRequest.setEndpoint('https://myorg.salesforce.com/services/apexrest/endpoint');
    return finalRequest;
}

public class OAuth2{
    public String TOKEN{get;set;}
}

public List<Pepperoni> getCalloutResponseContents() {
    Http ourHttp = new Http();
    HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
    HttpResponse responseToken = ourHttp.send(requestForToken);
    OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

    if(objAuthenticationInfo.TOKEN != null) {
        HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
        responseService = ourHttp.send(requestForService);
        pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
        for(Pepperoni pep : pepperoniList) {
            pep.Id = null;
        }
        return pepperoniList;
    }
}

public List<ContentVersion> getContentVersionList(Id pepId){
    List<ContentVersion> contentVersionList = [SELECT Id, ContentDocumentId FROM ContentVersion WHERE FirstPublishLocationId =: pepId];
    return contentVersionList;
}

public void insertCurrentContentVersion(){
    for(Pepperoni temporaryP : pepperoniList) {
        if(temporaryP.VersionData__c != null) {
            deletePreviousFiles(temporaryP.Id);

            ContentVersion currentContentVersion = new ContentVersion(
                Title = temporaryP.Title__c,
                PathOnClient = temporaryP.PathOnClient__c,
                VersionData = EncodingUtil.base64Decode(temporaryP.VersionData__c),
                FirstPublishLocationId = temporaryP.Id
            );
            insert currentContentVersion;
            temporaryCandidate.VersionData__c = null;
            update temporaryP;
        }
    }
}

public void deletePreviousFiles(Id pepId) {
    List<ContentVersion> contentVersionList = getContentVersionList(pepId);
    List<ContentDocument> contentDocumentList = new List <ContentDocument>();

    for(ContentVersion temporaryContentVersion : contentVersionList){
        ContentDocument thisContentDocument = [SELECT Id FROM ContentDocument WHERE Id =: temporaryContentVersion.ContentDocumentId];
        contentDocumentList.add(thisContentDocument); 
    }
    delete contentDocumentList;
} 
}

Batch:

global class BatchPep implements Database.Batchable<Integer>, Database.AllowsCallouts {
CallThePepperoni callThePepperoni = new CallThePepperoni();

global Integer operationsQuantity = 1;
global Iterable<Integer> start(Database.BatchableContext BC){
    List<Integer> scope = new List<Integer>();
    for(integer i = 1; i <= operationsQuantity; i++){
        scope.add(i);
    }
    return scope;
}

global void execute(Database.BatchableContext BC, List<Integer> scope){
    List<Pepperoni__c> mainList = new List<Pepperoni__c>();
    for(Integer i : scope){
        List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
        mainList.addAll(pepList);
    }
    upsert mainList Email__c;
    callThePepperoni.insertCurrentContentVersion(); 
}

global void finish(Database.BatchableContext BC){
}
}

Mock:

@isTest
global class PepMock implements HttpCalloutMock {


String token;
String externalId;

public CalloutJobAdvertisementMock(String token, String externalId) {
    this.token = token;
    this.externalId = externalId;
}

global HTTPResponse respond(HTTPRequest request) {
    HttpResponse response = new HttpResponse();
    response.setHeader('Content-Type', 'application/json');
    String serialized;
    if(request.getEndpoint().contains('services/oauth2/token')) {
        serialized = '{"'+ token +'":"FAKE-TOKEN"}';
    } else {
        //Here I serialized an object from another side:
        serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';
    }
    response.setBody(serialized);
    return response;
}
}

Test:

@isTest
private class CalloutPepTest {

static CallThePepperoni callThePepperoni = new CallThePepperoni();

@isTest
static void testGetCallout() {
    Pepperoni__c pep = new Pepperoni__c();
    insert pep;
    ContentVersion cv = new Contentversion(Title = 'TestTitle', PathOnClient = 'TestPath', VersionData = EncodingUtil.base64Decode('veryLoooongRealCode'), FirstPublishLocationId = pep.Id);
    List<ContentVersion> cvListTest = new List<ContentVersion>{ cv };
    insert cvListTest;
    String request = someFakeTokenIsHere;
    Test.setMock(HttpCalloutMock.class, new CalloutJobAdvertisementMock(200, 'ACCESS_TOKEN', ''));

    Test.startTest();
        List<Pepperoni__c> pepList = callThePepperoni.getCalloutResponseContents();
        HttpResponse response = callThePepperoni.responseService;
        List<ContentVersion> cvList = callThePepperoni.getContentVersionList(pep.Id);
        callThePepperoni.deletePreviousFiles(pep.Id);
        //Is bulk-test for batch work like this? Or not?
        Id batchJobId = Database.executeBatch(new BatchPep(), 100);
    Test.stopTest();

    System.assertEquals(response.getBody(), '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';);
    System.assertEquals(pepList.size(), 1);
    System.assertEquals(cvListTest.get(0).Id, cvList.get(0).Id);
}
}

Answer 1

I think you are on the right track. The code you're testing here is inherently somewhat more difficult to test, because you make multiple callouts and run an unusual batch class that isn't dependent on the test data you provide in test context. However, I think the class and mock you've built is along the right track.

Bulk Testing

To perform a bulk test, what you'll need to do is work on your Mock. You construct a return value here, by simply returning canned JSON:

    serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6yAAB"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';

In order to perform a relevant bulk test, you'd want to include a small number of records in that JSON. So you'd have something like

    serialized = '[{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6XXXX"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, ' 
+ '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6YYYY"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}, '
+ '{"attributes":{"type":"AnotherPep__c","url":"/services/data/v44.0/sobjects/AnotherPepFromOtherSide__c/a01f600000Mji6ZZZZ"},"Name__c":"My","VersionData__c":"veryLoooongRealCode"}]';

(Lines broken for clarity).

Inherent platform restrictions prevent you from actually executing more than one batch iteration in test context, so you'll have to make sure that operationsQuantity is always less than or equal to your batch size when you run your tests. You could set it higher than one, however, to validate the behavior of your loop. A counter in the mock class can track how many times it's called, and you can write assertions against that.

What you can do immediately, though, is configure your Mock to return multiple objects, validating that your code handles that correctly.

Testing Other Code Paths

You may want to create multiple Mocks so that you can test your code's response to several different situations:

• A single record returned.
• Multiple records returned.
• No records returned.
• An HTTP error returned.
• Something nonsensical returned due to problems with the remote server.

You can write multiple unit tests, and utilize a different one of those mocks in each test case to ensure that your code handles each type of return properly. Because you're also building your Mock to be responsive to different endpoints, you might want to build your Mock class so that the unit test case itself can supply the JSON to return - that way, you don't need to build many different classes. Alternately, you could create an abstract base Mock class that knows how to handle your OAuth callout, and subclass it to add the specific behaviors you want for the different test cases.

You may also want to take a look at MultiStaticResourceCalloutMock, a standard class you can use to create a Mock that returns JSON stored in a static resource based upon the endpoint that is called. This could be useful depending on how you decide you want to structure your unit test collection and their associated Mocks.

While it's not the same use case as what you're working on, I have an example from one of my test projects that shows this kind of approach to testing callouts. Here is one example. I'm calling an unreliable and rather weird API from the transit agency SEPTA, and I test my callout class in a bunch of different ways using different mocks:

• A good response, using a StaticResourceMock to return JSON stored in a Static Resource (this is inherently a bulk test, in a sense, because the JSON includes multiple results).
• A response where the API returns its own error structure.
• A response where I get an HTTP result code other than 200 OK.
• A response where I get back something that doesn't make any sense and/or isn't valid JSON.
• A response where an unexpected exception gets thrown at the callout.

This might be overkill, but because I'm working with an unreliable service and trying to handle every possible failure condition, I like the overkill on these tests.

Refactoring Code and Multiple Tests

One thing that's making your life a little more difficult is that you've built your callout class to make a token call and a REST call in the same method. If you factor those elements out, you can streamline things so you don't have to mock both callouts at the same time.

Starting from here:

public List<Pepperoni> getCalloutResponseContents() {
    Http ourHttp = new Http();
    HttpRequest requestForToken = RequestClass.createRequestForToken('GET');
    HttpResponse responseToken = ourHttp.send(requestForToken);
    OAuth2 objAuthenticationInfo = (OAuth2)JSON.deserialize(responseToken.getbody(), OAuth2.class);

    if(objAuthenticationInfo.TOKEN != null) {
        HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
        responseService = ourHttp.send(requestForService);
        pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
        for(Pepperoni pep : pepperoniList) {
            pep.Id = null;
        }
        return pepperoniList;
    }
}

what if instead you did something like this:

public List<Pepperoni> getCalloutResponseContents(String token) {
    Http ourHttp = new Http();
    HttpRequest requestForService = RequestClass.createRequestForService(objAuthenticationInfo.TOKEN);
    responseService = ourHttp.send(requestForService);
    pepperoniList = (List<Pepperoni>)JSON.deserialize(responseService.getBody(), List<Pepperoni>.class);
    for(Pepperoni pep : pepperoniList) {
        pep.Id = null;
    }
    return pepperoniList;
}

and build yourself a second method to perform the OAuth callout? That nets you a couple of things:

• You can write separate unit tests just for the callouts, each independently, using multiple simpler mocks and not worrying about the batch structure. Because you can pass a token string directly to getCalloutResponseContents(), you can test it as a unit with a simple mock that only knows how to return one kind of response.
• Then, you can test your batch class as a unit, using a very simple mock just to validate that the batch class makes the calls into your CallThePepperoni class that you expect it to.
• Finally, you can have a larger integration test, where you set a "real" mock and run your batch class against it to validate end-to-end behavior.

You can refactor your tests, too. Right now, I think your test class might be trying to do a bit too much, without smaller unit tests to "support" it, so to speak, by demonstrating that its component pieces of functionality work. You can do this by writing tests just for methods like insertCurrentContentVersion(), for example. A test for that method could populate pepperoniList (with records deserialized from your stock JSON, perhaps) and then call it, making assertions to check that it works as expected all by itself.

A principle that you'll find helpful in attempting this kind of refactoring, both in the tests and in the code, is that hiding state makes things tricky. Look at insertCurrentContentVersion(), for example. It does not accept a parameter. Instead, it uses an instance variable on its class to do its work. That makes it ever so slightly more difficult to test in isolation, because you have to be able, in your test, to control the value of that instance variable.

But if instead it accepts a parameter instead, and doesn't depend on internal state, you have a much easier time controlling it in a test. You can call it and feed it whatever data you want to observe the results, and you never get caught in a bind where you have to restructure your code just to get a test in there.

Now, one can probably go too far in this direction, and we could have a fun discussion about where the line is between different imperatives in code structure, but here I think it's pretty reasonable to decouple those methods from class state and just pass parameters.

As a general rule, decomposing functionality into smaller, unitary methods makes a huge difference in your ability to isolate and test each piece - and it often also makes it easier to test the pieces in assemblage (as an integration test) too. When I'm satisfied with a unit test suite, I often find that for N lines of Apex code, I have N*2 or N*3 lines of tests. Many of those test lines are boilerplate, because you're testing many different variations of a single piece of functionality. No test should be very big - big tests are harder to understand, so their failures don't tell you very much.

While the code you're working on is inherently somewhat complex, I think some small refactoring efforts like this could be very helpful.

Answer 2

You have bulkification problems all over your code. Never put any governor consumption in a loop. Mostly, you have DML operations you need to move outside your for loops.

public void insertCurrentContentVersion(){
    for(Pepperoni temporaryP : pepperoniList) {
        if(...) {
            deletePreviousFiles(temporaryP.Id); //                               <-- NO
            // ...
            insert currentContentVersion; //                                     <-- NO
            // ...
            update temporaryP; //                                                <-- NO
        }
    }
}

public void deletePreviousFiles(Id pepId) {
    // ...
    for(ContentVersion temporaryContentVersion : contentVersionList){
        ContentDocument thisContentDocument = [SELECT ... FROM ... WHERE ...]; // <-- NO
        // ...
    }
    delete contentDocumentList; // DML and you call this method in a loop
}

Typically to bulkify you simply instantiate a collection before your loop, populate it during your loop, and then act or query after your loop.

List<MyObject__c> records = new List<MyObject__c>();
for (...)
{
    records.add(...);
}
insert records;

This pattern will work for your currentContentVersion collection. As for updating VersionData__c, you are just clearing it out so not much work necessary here (though your posted code won't compile since you don't declare temporaryCandidate.

---

In addition to the above, the query in your deletePreviousFiles can be removed entirely. If you have the Id already, use the Database.delete(List<Id>) method.

Simply update your logic to collect Id instead of SObject:

List<Id> recordIds = new List<Id>();
for(ContentVersion temporaryContentVersion : contentVersionList){
    recordIds.add(temporaryContentVersion.ContentDocumentId);
}
Database.delete(recordIds);