Batch Class Test with History Object to Production problem

Question

I'm a new salesforce developer, and i'm facin my first real world problem in my company.

We have a custom object named "Solicitudes__c", is the detail side of a master relationship of another custom object.

I was asked to calculate the time between stages (Picklist field named "Etapas_de_solicitud__c" with 10 values). My solution is use History object records to calculate the first date to enter one stage, and the last day touching that same stage for every stage. I had to create some sample records and move them through all stages.

Later i came out with this apex class:

    public with sharing class SolicitudesTiempos {

    public static void tiempos(List<Solicitudes__c> solicitudes) {
        
        //transform the List to Set of Ids
        set<id> solicitudesIds = new set<Id>();
        for(solicitudes__c s : solicitudes){
            solicitudesIds.add(s.id);
        }

        //list used to update the Solicitudes__c records lateer
        List<Solicitudes__c> solicitudesActualizar = new List<Solicitudes__c>();

        //gather all Solicitudes__History records.
        list<Solicitudes__History> historialBuscado = [SELECT ParentId, NewValue, OldValue, CreatedDate
                                                        FROM Solicitudes__History 
                                                        WHERE ParentId =: solicitudesIds
                                                        AND Field = 'Etapas_de_Solicitud__c'
                                                        WITH SECURITY_ENFORCED
                                                        ORDER BY CreatedDate ASC];
                                               
                                                        
        // lists to store the history changes, accordingly.
        list<Solicitudes__History> historialCotizacion = new list<Solicitudes__History>();
        list<Solicitudes__History> historialAceptacion = new list<Solicitudes__History>();
        list<Solicitudes__History> historialPreanalisis = new list<Solicitudes__History>();
        list<Solicitudes__History> historialDocumentos = new list<Solicitudes__History>();
        list<Solicitudes__History> historialCarga = new list<Solicitudes__History>();
        list<Solicitudes__History> historialPendienteAnalizar = new list<Solicitudes__History>();
        list<Solicitudes__History> historialComite = new list<Solicitudes__History>();
        list<Solicitudes__History> historialCondicionada = new list<Solicitudes__History>();
        list<Solicitudes__History> historialAutorizada = new list<Solicitudes__History>();
        list<Solicitudes__History> historialPendientedeFirma = new list<Solicitudes__History>();


        // loop throughout all solicitudes__history to store the appropiate history record to a list.
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'Cotización' || s.OldValue == 'Cotización'){
                historialCotizacion.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'Aceptación' || s.OldValue == 'Aceptación'){
                historialAceptacion.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'Preanálisis' || s.OldValue == 'Preanálisis'){
                historialPreanalisis.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'Documentos' || s.OldValue == 'Documentos'){
                historialDocumentos.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'Aviso a sebas' || s.OldValue == 'Aviso a sebas'){
                historialCarga.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'pendiente de analizar' || s.OldValue == 'pendiente de analizar'){
                historialPendienteAnalizar.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'comite' || s.OldValue == 'comite'){
                historialComite.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'condicionado' || s.OldValue == 'condicionado'){
                historialCondicionada.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'autorizada' || s.OldValue == 'autorizada'){
                historialAutorizada.add(s);
            }
        }
        for(Solicitudes__History s : historialBuscado){
            if(s.NewValue == 'pendiente de firma' || s.OldValue == 'pendiente de firma'){
                historialPendientedeFirma.add(s);
            }
        }

        // loop to calcule every Solicitudes__c record time on each stage.
        for(Id s : solicitudesIds){

            Decimal tiempoCotizacion = SolicitudesTiempos.calcularTiempo(historialCotizacion, s);
            Decimal tiempoAceptacion = SolicitudesTiempos.calcularTiempo(historialAceptacion, s);
            Decimal tiempoPreanalisis = SolicitudesTiempos.calcularTiempo(historialPreanalisis, s);
            Decimal tiempoDocumentos = SolicitudesTiempos.calcularTiempo(historialDocumentos, s);
            Decimal tiempoCarga = SolicitudesTiempos.calcularTiempo(historialCarga, s);
            Decimal tiempoPendienteAnalizar = SolicitudesTiempos.calcularTiempo(historialPendienteAnalizar, s);
            Decimal tiempoComite = SolicitudesTiempos.calcularTiempo(historialComite, s);
            Decimal tiempoCondicionado = SolicitudesTiempos.calcularTiempo(historialCondicionada, s);
            Decimal tiempoAutorizada = SolicitudesTiempos.calcularTiempo(historialAutorizada, s);
            Decimal tiempoPendienteFirma = SolicitudesTiempos.calcularTiempo(historialPendientedeFirma, s);

            //store each calculation to the record's field and add it to a collection to update later.
            Solicitudes__c solicitud = new Solicitudes__c(Id = s, 
                                                            Tiempo_cotizacion__c=tiempoCotizacion,
                                                            Tiempo_Aceptacion__c=tiempoAceptacion,
                                                            Tiempo_preanalisis__c=tiempoPreanalisis,
                                                            Tiempo_Documentos__c=tiempoDocumentos,
                                                            Tiempo_Carga__c=tiempoCarga,
                                                            Tiempo_Por_analizar__c=tiempoPendienteAnalizar,
                                                            Tiempo_Comite__c=tiempoComite,
                                                            Tiempo_Condicionada__c=tiempoCondicionado,
                                                            Tiempo_Autorizada__c=tiempoAutorizada,
                                                            Tiempo_Pendiente_Firma__c=tiempoPendienteFirma);
            
            //add the new record to a collection to update later.
            solicitudesActualizar.add(solicitud);
        }
        
        //update the records
        update solicitudesActualizar;
    }


    //method to calculate time between stages.
    private static Decimal calcularTiempo (list<Solicitudes__History> historial, Id solicitudId){

        if(historial.size()>0 && historial != null){
            dateTime date1;
            dateTime date2;
            Decimal minutos;
            for(Solicitudes__History hist : historial){
                if(hist.ParentId == solicitudId && date1 == null){
                    date1 = hist.CreatedDate;
                }else if(hist.ParentId == solicitudId && date1 != null){
                    date2 = hist.CreatedDate;
                }
            }
            if(date1 != null && date2 != null){
                Long firstDate = date1.getTime();
                Long finalDate = date2.getTime();
                Decimal segundos = (finalDate-firstDate)/1000;
                minutos = segundos/60;
                return minutos;
            }else{
                return 0.00;
            }
        }else{
            return 0.00;
        }   
    }
}

Then i came out with an batch apex class:

public class SolicitudesTiemposBatch implements Database.Batchable<SObject> {

    public Database.QueryLocator start(Database.BatchableContext bc) {
        String query = 'SELECT Id FROM Solicitudes__c '+
                'WHERE CreatedDate >= 2022-01-01T00:00:00Z '+
                'ORDER BY CreatedDate ASC';
                return DataBase.getQueryLocator(query);
    }

    public void execute(Database.BatchableContext bc, List<Solicitudes__c> scope){
        SolicitudesTiempos.tiempos(scope);
    }

    public void finish(database.BatchableContext bc){
    }
}

And create a Test class:

@isTest
public class SolicitudesTiemposBatchTest {
    @isTest(SeeAllData=true)
        static void test(){

            Test.startTest();
            SolicitudesTiemposBatch solicitudesBatch = new SolicitudesTiemposBatch();
            Database.executeBatch(solicitudesBatch, 200);
            Test.stopTest();
            
            System.assertEquals(199,[SELECT count() FROM Solicitudes__C], '199 solicitudes');
        }
}

The problem is that when i'm trying to validate in production i see this problem: "System.UnexpectedException: No more than one executeBatch can be called from within a test method. Please make sure the iterable returned from your start method matches the batch size, resulting in one executeBatch invocation. Stack Trace: External entry point"

What i can read searching online, is that you cannot mock History records, OldValue and NewValue are not writable.

What can i do, any improvements/suggestions? Thanks a lot in advanced.

Answer 1

I imagine that the error is a result of you using seeAllData=true (which is normally a red flag, but testing history is one of the few exceptions where it can be appropriate) and having more than 200 Solicitudes__c records matching your query filter in your production org.

There are two easy ways out of this problem:

1. Have your batch class detect that you're in a test context or allow it to receive a string that you can append onto your query
2. Call your batch class methods directly instead of going through Database.executeBatch()

Option 1

could look something like this

public class SolicitudesTiemposBatch implements Database.Batchable<SObject> {
    // We move the query string to be a class variable to make it easy to use in
    //   the constructors
    private String queryString;

    // We need to provide our own no-argument constructor if we define any
    //   constructors in a class
    public SolicitudesTiemposBatch(){
        this('');
    }

    public SolicitudesTiemposBatch(String limiteDelTest){
        queryString = 'SELECT Id FROM Solicitudes__c ' +
                'WHERE CreatedDate >= 2022-01-01T00:00:00Z ' +
                'ORDER BY CreatedDate ASC ' +
                limiteDelTest;
    }

    public Database.QueryLocator start(Database.BatchableContext bc) {
        return DataBase.getQueryLocator(queryString);
    }

    public void execute(Database.BatchableContext bc, List<Solicitudes__c> scope){
        SolicitudesTiempos.tiempos(scope);
    }

    public void finish(database.BatchableContext bc){
    }
}

which you could still run in production using the no-arg constructor

SolicitudesTiemposBatch solicitudesBatch = new SolicitudesTiemposBatch();
Database.executeBatch(solicitudesBatch, 200);

but in your test you can specifically add a limit statement

Test.startTest();
SolicitudesTiemposBatch solicitudesBatch = new SolicitudesTiemposBatch('LIMIT 200');
Database.executeBatch(solicitudesBatch, 200);
Test.stopTest();

Option 2

is mostly just a shift in thinking. We generally only want to test things that we have control over, things that we're responsible for. The responsibility for making sure that the batch execution mechanism works rests with Salesforce, not us. Therefore, we should just trust that it works.

Instead, you can just call start(), execute(), and finish() directly. You'll probably call just one of those per test method, though you can call 2 (or all 3) in a single test.

BatchableContext is an interface that we can implement, but people don't use the batchable context 99.9% of the time. If it isn't being used, then you can just pass null as its argument.

@isTest
public class SolicitudesTiemposBatchTest {
    // No need for seeAllData=true in this particular test method
    @isTest
    static void test(){
        insert new Solicitudes__c();

        Test.startTest();
        SolicitudesTiemposBatch solicitudesBatch = new SolicitudesTiemposBatch();
        Database.queryLocator locator = solicitudesBatch.start(null);
        Test.stopTest();
        
        List<Solicitudes__c> results = new List<Solicitudes__c>();
        while(locator.hasNext()){
            results.add((Solicitudes__c)locator.next());
        }

        System.assertEquals(1, results.size(), 'No ha devuelto exactamente 1 registro');
    }

    // When you are actually working with history records, seeAllData=true
    @isTest(SeeAllData=true)
    static void test2(){
        // You can still control which records you send into the execute method
        List<Solicitudes__c> results = [SELECT Id FROM Solicitudes__c LIMIT 10];

        Test.startTest();
        SolicitudesTiemposBatch solicitudesBatch = new SolicitudesTiemposBatch();
        Database.queryLocator locator = solicitudesBatch.execute(null, results);
        Test.stopTest();

        // assertions here
    }
}

There is a medium-effort way as well

By using dependency injection, you can sidestep this issue.
Your batch class itself isn't working with history records. The main things that you're testing here are that:

• Your start method runs, and returns some results
• Your execute() method runs, and calls a particular method of a particular class

Strictly speaking, that's really all that your test for your batch class should be doing. Testing the behavior of SolicitudesTiempos should be made into its own test class.

By making SolicitudesTiempos either a virtual class, or having it implement an interface, and making tiempos() an instance method, you can make your own "implementation" of it in your test.

public with sharing virtual class SolicitudesTiempos {
    public virtual void tiempos(List<Solicitudes__c> solicitudes) {
        // interesting code here
    }
}

Your batch class would change so that we could "inject" the test instance of the class

public class SolicitudesTiemposBatch implements Database.Batchable<SObject> {
    private SolicitudesTiempos instance;

    public SolicitudesTiemposBatch(){
        this(new SolicitudesTiempos());
    }

    public SolicitudesTiemposBatch(SolicitudesTiempos givenInstance){
        instance = givenInstance;
    }

    public Database.QueryLocator start(Database.BatchableContext bc) {
        return DataBase.getQueryLocator('SELECT Id FROM Solicitudes__c ' +
                'WHERE CreatedDate >= 2022-01-01T00:00:00Z ' +
                'ORDER BY CreatedDate ASC'
        );
    }

    public void execute(Database.BatchableContext bc, List<Solicitudes__c> scope){
        instance.tiempos(scope);
    }

    public void finish(database.BatchableContext bc){
    }
}

Then, in your test, you can make and use your own implementation

@isTest
public class SolicitudesTiemposBatchTest {
    private class SolicitudesTiemposTest extends SolicitudesTiempos {
        public Integer tiemposLlamados = 0;

        override public void tiempos(List<Solicitudes__c> solicitudes){
            tiemposLlamados += 1;
        }
    }

    @isTest
    static void test(){
        SolicitudesTiempos testInstance = new SolicitudesTiemposTest();

        Test.startTest();
        // And we finally perform the "injection" here
        SolicitudesTiemposBatch solicitudesBatch = new SolicitudesTiemposBatch(testInstance);
        solicitudesBatch.execute(null, new List<Solicitudes__c>());
        Test.stopTest();

        System.assertEquals(1, testInstance.tiemposLlamados);
    }

    // and other tests...
}

Finally, there is a "hard" way

I think the "hard" way is not particularly hard, it just involves more work than the other approaches. It does minimize the need for seeAllData=true though.

In a nutshell, this approach involves:

• Creating a generic Data Access Object (DAO) for History, HistoryDAO that has instance variables like public Object oldValue and public Object newValue
• Creating a concrete DAO class specifically for Solicitudes__History, SolicitudesHistoryDAO
• Taking a page from FFlib (Financial Force's Apex Enterprise Patterns) and creating a selector class for SolicitudesHistoryDAO
  • That also allows you to inject data in your tests that the selector should return

Usually, using JSON serialization and deserialization is the way to get around the issue of working with read-only fields. This trick does not work with history objects though.

The DAO classes act as an abstraction layer, and give us more control over what we can do.
Having a "selector" class gives us a centralized location where we can define how other code receives these DAO instances.
Allowing data to be injected into the selector makes it (relatively) easy to test.

It is more work, and it involves writing more tests. It'll be a pain to set up, but once you do it'll make testing things that depend on history easier. The only test that would need seeAllData=true would be a test (maybe two) in the selector class that would ensure that __History records are correctly transformed into a DAO instance.

I've already spent too long on this, so I'll point you to James Simone's article on the topic (I am not affiliated with that site, nor with James) for a bit more in-depth explanation.