0

I have an apex trigger which updates the description of cases with the concatenation of its child cases, if it has. If not, it fills the Description field with a default phrase. I'm trying to code the test class for this trigger and i don't know how to continue.

Apex trigger:

trigger CaseTrigger on Case (before update) {
   
    Map<Id, List<String>> idPadreYSusHijos = new Map<Id, List<String>>();

    for(Case c :[SELECT Id, ParentId, CaseNumber FROM Case WHERE ParentId IN : Trigger.newMap.keySet()]){
        // Comprobar si los Id ya están en el mapa, si no están se añaden. 
        // De esta forma se evitan duplicados
        if(!idPadreYSusHijos.containsKey(c.ParentId)){
            idPadreYSusHijos.put(c.ParentId, new List<String>());
        }
        // Añadir los casosHijos a sus respectivos padres mediante Id en el mapa
        idPadreYSusHijos.get(c.ParentId).add(c.CaseNumber);
    }
    for(Case caso : Trigger.New){
       List<String> numeroDeCasosHijos = idPadreYSusHijos.get(caso.Id);
        
        if(numeroDeCasosHijos != null){
            caso.Description = String.join(numeroDeCasosHijos, ', ');
        }else{
        // Lista vacía, no hay hijos, asignamos descripción por defecto
        caso.Description = 'Caso sin hijos';
        }
    }
}

Test class:

@isTest
public class CaseTriggerTest {
    
   @testSetup
   static void dataCreation() {
        Case padreConHijos =  new Case(Status = 'New',Origin='Phone');
        Case padreSinHijos =  new Case(Status = 'New',Origin='Phone');
        Case hijo1 = new Case(Status = 'New',Origin='Phone',ParentId=padreConHijos.Id, Description='hijo1');
        Case hijo2 = new Case(Status = 'New',Origin='Phone',ParentId=padreConHijos.Id, Description='hijo2');
        insert padreConHijos;
        insert padreSinHijos;
        insert hijo1;
        insert hijo2;    
    }
    
    @isTest
    static void testCaseTrigger() {
        List<Case> casosHijos = new List<Case>();
        List<Case> casosPadres = new List<Case>();
        for(Case c : [SELECT Id,CaseNumber FROM Case WHERE ParentId = null]) {
            casosPadres.add(c);
        }
        //List<String> nombresHijos = new List<String>([SELECT CaseNumber FROM Case WHERE Id IN :casosHijos]);
        update casosPadres;     
        for(Case c : [SELECT Id,CaseNumber FROM Case WHERE ParentId != null]) {
            casosHijos.add(c);
        }
        update casosHijos;
        
        Test.startTest();
        for(Case caso : casosHijos){
        System.assertEquals(caso.Description, 'Caso sin hijos');
            }
        Test.stopTest();
        
        /*Test.startTest();
        for(Case caso : casosHijos){
        System.assertEquals(caso.Description, String.join(casosHijos, ', '));
            }
        Test.stopTest();*/
    }
    
    @isTest
    static void testMapa() {
        Case hijo1;
        Case padreConHijos;
        
        Test.startTest();
        Map<Id,List<String>> lista = new Map<Id,List<String>>();
        lista.put(padreConHijos.Id,new List<String>());
        lista.get(padreConHijos.Id).add(hijo1.CaseNumber);
        Test.stopTest();
        
    }
    
}

With the test class i've made i cover 60% of the trigger. I don't know how to test the case where numeroDeCasosHijos !=null and the following lines :

if(!idPadreYSusHijos.containsKey(c.ParentId)){
    idPadreYSusHijos.put(c.ParentId, new List<String>());
}

1 Answer 1

3

Part of the issue is in your @testSetup method.

You need to perform a dml insert on the padreConHijos case (at the very least) before trying to set the ParentId on hijo1 and hijo2. Records don't have an Id before they're inserted. There are a few situations where you can manually set an Id on a test record, but this is not one of them (because you're using a query in your trigger).

    @testSetup
    static void dataCreation() {
        Case padreConHijos = new Case(Status = 'New', Origin = 'Phone');
        Case padreSinHijos = new Case(Status = 'New', Origin = 'Phone');
        // You only need to insert padreConHijos here, but might as well insert
        //   padreSinHijos too.
        // It's good practice to group things into fewer DML statements where possible
        insert new List<Case>{padreConHijos, padreSinHijos};

        // After the insert has finished, the inserted records automatically have their
        //   Id field populated.
        // This is the only field that is automatically populated.
        // If you have a trigger/flow/process builder that causes other fields to be populated,
        //   you need to execute a query to retrieve those fields.
        Case hijo1 = new Case(Status = 'New', Origin = 'Phone', ParentId = padreConHijos.Id, Description = 'hijo1');
        Case hijo2 = new Case(Status = 'New', Origin = 'Phone', ParentId = padreConHijos.Id, Description = 'hijo2');
        insert new List<Case>{hijo1, hijo2};
    }

You only get coverage for code executed as part of a unit test

So if you expect a certain piece of code to be covered, and it isn't, then you know something is wrong.

In testCaseTrigger(), the query in for(Case c : [SELECT Id,CaseNumber FROM Case WHERE ParentId = null]) was returning 4 cases (both parents, and both children) instead of 2 cases (both parents) because your test setup ended up populating ParentId on the child cases before padreConHijos had an Id.

When you got to the first query in your trigger, in for(Case c :[SELECT Id, ParentId, CaseNumber FROM Case WHERE ParentId IN : Trigger.newMap.keySet()]){, it returned no results, so the body of the for loop was not executed.

Fixing the test setup should fix that issue, and get you coverage for the body of that for loop. It'll also ensure that you have a case where numeroDeCasosHijos in your second loop is not null (thus gaining coverage for that as well).

Code coverage should not be the focus of unit tests

Code coverage is the metric that Salesforce uses, and we as programmers need to care about it to some degree, but coverage isn't what makes a test useful to us.

Instead, you want to focus on whether or not the your code behaves as intended. That's what assertions help us do. A method that returns the result of 2 + 2 could have 100% coverage, but if it returns 5 as a result then the method is doing something wrong.

As for what you should be writing assertions for, you should look at the results, the output of running your code. Things like

  • The return value from a method
  • SObject records that were created (or deleted)
  • Fields on SObject record(s) that were updated
  • Whether or not a particular method in a helper class was called
  • Changes to the "public state" (i.e. public class variables) in a class that you're testing

The main thing you're concerned about in this case is if the case Description is updated or not.

Try to write more, small tests instead of fewer, large tests

Smaller tests are generally easier to write, and having multiple tests makes it easier to see where issues are.

If you have a single, monolithic test method and it fails partway through, you'll only get a single error message. If your code has more than one issue, you'll need to fix the first error before you can see the second error.

With multiple tests, you have a chance to see multiple errors at once. You'll still probably end up fixing them one at a time, but having a better idea of the overall state of your code can help track down issues and make better decisions.

One of the other important reasons to write many, smaller tests is that if you test a wide range of scenarios, your code coverage will naturally be high.

What tests would I write for this?

  • One test where you update a parent Case with children (assert that the description changed)
  • One test where you update a parent Case with no children (assert that the description did not change)
  • Perhaps one test where you update a child Case (assert that neither the child nor parent's description changed)

For that first test, I'd write it along these lines

@isTest
static void caseTrigger_UpdateParentWithChildren_UpdatesParentDescription(){
    // Gather data from before running the trigger (or method) that you want to test
    //   so you have something to compare against

    // Self-lookups (like ParentId on Case, which points to another Case)
    //   tend to need some additional processing because we can't perform a
    //   semi-join on the same object as the base query
    Map<Id, Case> caseMapBefore = new Map<Id, Case>([SELECT Id, Description, ParentId FROM Case]);
    
    Set<Id> parentIds = new Set<Id>();
    for(Case c :caseMapBefore.values()){
        parentIds.add(c.ParentId);
    }

    // My personal preference is to execute one remove() outside of a loop instead
    //   of putting something like "if(String.isNotBlank(c.ParentId))" inside a loop
    parentIds.remove(null);

    // Removing cases that have no children (because cases with children is
    //   what we're interested in for this test)
    caseMapBefore.keySet().retainAll(parentIds);

    // Step 2 of unit testing: execute the target code
    Test.startTest();

    update caseMapBefore.values();

    Test.stopTest();

    // Step 3 of unit testing: gather results and make assertions
    Map<Id, Case> caseMapAfter = new Map<Id, Case>([SELECT Id, Description, ParentId FROM Case WHERE Id IN :caseMapBefore.keySet()]);

    // Another important part of testing is to notice that you're asserting against things
    //   that happened as a result of running your code.
    // If you were to set the case Description directly in this test method (and not call
    //   your trigger), an assertion that checks the description wouldn't tell you anything
    //   about how the code you're trying to test behaves.

    for(Case caseAfter :caseMapAfter.values()){
        Case caseBefore = caseMapBefore.get(caseAfter.Id);

        // The third argument is optional, but you should always set it.
        // It's the message that you'll be given if the assertion fails.
        // Having a helpful message really helps to understand why and where a test failed
        System.assertNotEquals(caseBefore.Description, caseAfter.Description, 'Updating a parent case with children should have caused the Description to be updated');
    }
}

Parting advice

Once you're comfortable with writing triggers like this (and writing tests), then the next step you should take is to use a trigger framework.

Best practice here is to have only one trigger per object (you can have multiple triggers, but they are not guaranteed to be run in any particular order), and to keep the trigger logic-free.

Triggers are harder to test than Apex classes. Triggers can only be executed by performing DML. As you continue to customize your org, the number of requirements that you have to satisfy to create your test data will increase.

Ideally, your trigger would look something like this

trigger case on Case (before insert, after insert, before update, after update, before delete, after delete, after undelete){
    MyFramework.run(trigger.old, trigger.new, trigger.operationType);
}

with MyFramework.run() using the trigger operation type to decide which handler method to call, and the handler method dictating the code to be run (or classes to be executed).

Calling a method in an Apex class is more flexible. If you have the handler methods take lists and/or maps instead of directly using trigger context variables, then your code is independent of the trigger (which can really help if you need to set up a situation for a test that would otherwise be difficult or impossible to do).

2
  • I was trying your code in the dev console and the following error appears: The inner and outer selects should not be on the same object type. Apr 4, 2022 at 17:25
  • @alexmartinez Bummer, I was hoping that would work so I didn't need to do more work. Updated my example test method.
    – Derek F
    Apr 4, 2022 at 18:28

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .