Method Does Not Create New Child Record Correctly

Question

I am trying to write a test class to test my handler class that has 3 methods. One handles creation of new records. One handles if the record was edited, and one deletes related records. I'm trying to test that when the record is updated the handler's actions actually work.

I can see that if I use a real record it works as expected. However, in code it does not. There are no error messages just my assert does not equal what I expect.

System.AssertException: Assertion Failed: Expected: 50, Actual: 100.00

Example Works, record is created and generates schedule as expected:

@isTest static void testCreateSchedule() {

    // Create Revenue Pipeline record
    Revenue_Pipeline__c revPipe = new Revenue_Pipeline__c();
        revPipe.Weighted_Product_Price__c = 1200;
        revPipe.Product_Start_Date__c = date.newInstance(2016, 08, 1);
        revPipe.Product_End_Date__c = date.newInstance(2017, 07, 1);
        revPipe.Name = 'Test Pipeline';
        revPipe.CurrencyIsoCode = 'USD';
    insert revPipe; 

    // Get the Revenue Pipeline record with the schedule
    Revenue_Pipeline_Schedule__c revPipeS = [SELECT Id, Amount__c
                                             FROM Revenue_Pipeline_Schedule__c
                                             WHERE Date__c = :date.newInstance(2016, 10, 1)
                                             LIMIT 1];  

    // Schedule record should be 1200/12 or 100
    System.assertEquals(100, revPipeS.Amount__c);
}

Example does not work, record is created but does not update the test record:

@isTest static void testCheckRecord() {

    // Create Revenue Pipeline record
    Revenue_Pipeline__c revPipe = new Revenue_Pipeline__c();
        revPipe.Weighted_Product_Price__c = 1200;
        revPipe.Product_Start_Date__c = date.newInstance(2016, 08, 1);
        revPipe.Product_End_Date__c = date.newInstance(2017, 07, 1);
        revPipe.Name = 'Test Pipeline';
        revPipe.CurrencyIsoCode = 'USD';
    insert revPipe;

    // Edit Revenue Pipeline record to change the Price
    Test.startTest();
    revPipe.Weighted_Product_Price__c = 600;
    update revPipe;
    Test.stopTest();

    // Get the Revenue Pipeline record with the schedule
    Revenue_Pipeline_Schedule__c revPipeS = [SELECT Id, Amount__c
                                             FROM Revenue_Pipeline_Schedule__c
                                             WHERE Date__c = :date.newInstance(2016, 10, 1) AND
                                             Revenue_Pipeline__c = :revPipe.Id
                                             LIMIT 1];      
    // Schedule record should be 600/12 or 50
    System.debug('Amount is:: ' + revPipeS.Amount__c);
    //System.assertEquals(50, revPipeS.Amount__c);
}

On After Insert this creates Schedule records.

public static void createRevSchedule(Map<Id, Revenue_Pipeline__c> newMap)
{
    List<Revenue_Pipeline_Schedule__c> newSchedule = new List<Revenue_Pipeline_Schedule__c>();
    for(Revenue_Pipeline__c revPipe : newMap.values()) {
        Integer numOfMonths = revPipe.Product_Start_Date__c.monthsBetween(revPipe.Product_End_Date__c) + 1;
        Integer n = revPipe.Subscription_Type__c == 'Subscription' ? 1 : 0;
        for(Integer i=0; i < numOfMonths; i++) {
                Revenue_Pipeline_Schedule__c revSchedule = new Revenue_Pipeline_Schedule__c();
                revSchedule.Amount__c = revPipe.Weighted_Product_Price__c/numOfMonths;
                revSchedule.Date__c = revPipe.Product_Start_Date__c.addMonths(i + n);
                revSchedule.Name = revPipe.Name + ' - ' + revSchedule.Date__c.year() + '-' + revSchedule.Date__c.month();
                revSchedule.Revenue_Pipeline__c = revPipe.Id;
                revSchedule.CurrencyIsoCode = revPipe.CurrencyIsoCode;
                newSchedule.add(revSchedule);           
        }
    }
    if(newSchedule.size() > 0){
        try{
            insert newSchedule;             
        } catch (DmlException ex) {
            ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.INFO, ex.getMessage()));
        }
    }
}

On After Update this checks if one of the fields changed and sends to delete method.

// Checks if the fields were changed in the newMap compared to OldMap
public void checkUpdate(Map<Id, Revenue_Pipeline__c> newMap, Map<Id, Revenue_Pipeline__c> oldMap)
{
    Map<Id, Revenue_Pipeline__c> revPipeline = new Map<Id, Revenue_Pipeline__c>();

    // Check if the fields changed
    for(Revenue_Pipeline__c revPipe : newMap.values())
    {
        if(
            revPipe.Product_Start_Date__c       != oldMap.get(revPipe.Id).Product_Start_Date__c ||
            revPipe.Product_End_Date__c         != oldMap.get(revPipe.Id).Product_End_Date__c ||
            revPipe.Product_Price__c            != oldMap.get(revPipe.Id).Product_Price__c ||
            revPipe.Weighted_Product_Price__c   != oldMap.get(revPipe.Id).Weighted_Product_Price__c)
            {
                revPipeline.put(revPipe.Id, revPipe);
            }
    }
    if(revPipeline.size() > 0)
    {
        deleteSchedule(revPipeline);
    }
}

If there is a change to the the fields this deletes the schedules and sends back to the create method to create new schedule records.

// Delete Revenue_Pipeline_Schedule records 
public void deleteSchedule(Map<Id, Revenue_Pipeline__c> revPipeline)
{
    List<Revenue_Pipeline_Schedule__c> scheduleToDelete = [SELECT Id,
                                                                 Revenue_Pipeline__r.Id
                                                         FROM Revenue_Pipeline_Schedule__c
                                                         WHERE Revenue_Pipeline__r.Id In :revPipeline.keyset()];
    try{
        delete scheduleToDelete;        
    }   
    catch (DmlException ex){
        ApexPages.addMessage(new ApexPages.Message(ApexPages.Severity.INFO, ex.getMessage()));
    }
    createRevSchedule(revPipeline);
}

Answer 1

Big Picture

When you are implementing complex functionality, Separation Of Concerns and a proper suite of Unit Tests can really help you pinpoint critical aspects of your functionality that can be difficult to catch when you're only looking at the big picture.

Individual Concerns

Right now, you simply test that the trigger does what you expect. But really, there are a bunch of small steps, not just one big one.

1. Fire your after update trigger
2. Filter parent records that should be processed
3. Query child records
4. Delete child records
5. Handle delete exceptions
6. Create new child records
7. Handle create exceptions?

You have actually co-mingled 2-5, (and chained 5 to 6) which doesn't help you write simple, effective tests. For all we know, the error is really in step 3. But you never directly test that concern!

---

Filter

I recommend the first step for you is to separate out the filter and test it rigorously:

public Map<Id, Revenue_Pipeline__c> needsNewSchedules(/*parameters*/) { /* filter logic */ }

Now you can test your filter mechanism very rigorously:

static testMethod void testNeedsNewSchedule_WeightedPriceChanged() { /* test logic */ }
static testMethod void testNeedsNewSchedule_StartDateChanged() { /* test logic */ }
static testMethod void testNeedsNewSchedule_EndDateChanged() { /* test logic */ }
static testMethod void testNeedsNewSchedule_PriceChanged() { /* test logic */ }

---

Delete

I also recommend you decouple the delete and create methods, and call them separately from your trigger/handler.

public void afterInsert()
{
    List<Revenue_Pipeline__c> filtered = RevenuePipelineService.needsNewSchedules(
        newMap, oldMap
    );
    RevenuePipelineService.deleteSchedule(filtered);
    RevenuePipelineService.createSchedule(filtered);
}

Now testing the delete mechanism is much simpler.

static testMethod void testDeleteSchedule()
{
    // setup data
    Test.startTest();
        RevenuePipelineService.deleteSchedule(pipelineRecords);
    Test.stopTest();
    system.assertEquals(0, [SELECT count() FROM ChildObject__c], '<assertion_message>');
}

This pattern will make it much easier for you to test different execution paths for each method.

---

Handle Exceptions

If you move out your error handling, testing becomes much easier, because instead of passing queried records, you can pass some you know will fail to delete (using dummy ids).

static testMethod void testSafeDelete()
{
    List<MyObject__c> records = new List<MyObject___c>
    {
        new MyObject__c(Id=SObjectType.MyObject__c.getKeyPrefix().rightPad(15, '0')
    };

    DmlException expectedException;
    Test.startTest();
        try { MyObjectService.safeDelete(records); }
        catch (DmlException d) { expectedException = d; }
    Test.stopTest();

    system.assertNotEquals(null, expectedException, '<assertion_message>');
}

You can then simplify the body of your deleteSchedule method to:

safeDelete[/*child query*/]);

Also note that ApexPages.addMessages is not an appropriate error handling mechanism from within a trigger. You should map errors back to the parent.

try
{
    delete children;
}
catch (DmlException d)
{
    for (Integer i = 0; i < d.getNumDml(); i++)
    {
        Id parentId = children[i].Parent__c;
        parentId.addError(d);
    }
}

---

Closing Thoughts

There's more yet to explain, but this post is already practically a book, and I think by now you get the idea. Separate your functionality into smaller chunks, test each rigorously, and you will find the breakdown. One last note, though, is that if you really want to stick with a delete-then-create strategy, you should strongly consider Transaction Control