I'm trying to write a test class for a trigger another developer on this forum showed me how write. Not getting 75%. Only 43%. Here is my test class:

private class MQLEventTest {

  @isTest static void TestMyMQLs() {
    Account myAcct = new Account();
    myAcct.Name = 'MQLTestAcct';

    Contact myContact = new Contact();
    myContact.AccountId = myAcct.Id;
    myContact.LastName = 'MQLTestContact';

    Event myMQL = new Event();
    myMQL.OwnerId = '00541000002eabg';
    myMQL.Subject = 'MQLtesting!';
    myMQL.ActivityDate = Date.today();
    myMql.ActivityDateTime = Datetime.now();
    myMQL.DurationInMinutes = 50;
    myMQL.Type = 'MQL';
    myMQL.WhoId = myContact.Id;

    Opportunity myOpp = new Opportunity();
    myOpp.AccountId = myAcct.Id;
    myOpp.Name = 'MyMQLTestOpp';
    myOpp.Amount = 5;
    myOpp.Type = 'New';
    myOpp.StageName = 'Discovery';
    myOpp.CloseDate = Date.today();

    insert myAcct;
    insert myContact;
    insert myMQL;
    insert myOpp;

    OpportunityContactRole myContactRole = new OpportunityContactRole();
    myContactRole.ContactId = myContact.Id;
    myContactRole.OpportunityId = myOpp.Id;
    myContactRole.Role = 'Decision Maker';

    insert myContactRole;



Here is the trigger. I've put comments above and below the lines which were not covered by my test class. What did I miss in my test class? Thank you.

trigger MQLEvent on Opportunity (after update, after insert) {

Set<Id> contactIds = new Set<Id>();
Map<Id, List<OpportunityContactRole>> ocrs = new Map<Id, List<OpportunityContactRole>>();
for (OpportunityContactRole ocr : [
    SELECT OpportunityId, ContactId FROM OpportunityContactRole
    WHERE OpportunityId IN :trigger.new
    if (!ocrs.containsKey(ocr.OpportunityId))
        ocrs.put(ocr.OpportunityId, new List<OpportunityContactRole>());

Map<Id, List<Event>> events = new Map<Id, List<Event>>();
for (Event event : [
    SELECT WhoId FROM Event
    WHERE WhoId IN :contactIds
    AND WhatId = null
    if (!events.containsKey(event.WhoId))
        events.put(event.WhoId, new List<Event>());

for (Opportunity record : trigger.new)
    if (!ocrs.containsKey(record.Id)) continue; //NEXT 5 LINES NOT COVERED
    for (OpportunityContactRole ocr : ocrs.get(record.Id))
        if (!events.containsKey(ocr.ContactId)) continue;
        for (Event event : events.get(ocr.ContactId))
            event.WhatId = record.Id;
List<Event> eventsToUpdate = new List<Event>();
for (List<Event> contactSpecificEvents : events.values())
update eventsToUpdate;

You have to update your Opportunity record. None of these lines can be hit in your insert trigger because they depend on an OpportunityContactRole record being associated.

That said, this may be a good time to learn a little bit about architecture and Separation Of Concerns. The core concept in Unit Tests is that you should test atomic units of code and ensure application performance. That last also requires assertions on the behavior, about which more later.

Anyway, the simplest architectural paradigm which can help you here is called a Service layer. It let's you perform small stateless actions where you filter/transform inputs (often record collections passed from the trigger). So in this case, you might separate your methods into these signatures:

public with sharing class OpportunityService
    public static Map<Id, List<OpportunityContactRole>> getContactRoles
        (List<Opportunity> records)
        // first for loop
    public static void setWhatIds
        (Map<Id, List<OpportunityContactRole>> contactRoles, Map<Id, List<Event>> events)
        // third for loop

public with sharing class EventService
    public static Map<Id, List<Event>> groupByWhoId(List<Event> events)
        // second for loop
    public static List<Event> flatten(Map<Id, List<Event>> groupedEvents)
        // final for loop
        // not the update that follows, though

A big advantage of this service layer pattern is that now, you can test most of your functionality without being forced to insert the data. This makes your tests on the application logic simpler and faster. For example, to test the event grouping:

class EventServiceTests
    static testMethod void testGroupByWhoId()
        final Integer whoCount = 3, recordsPer = 5;
        final String prefix = SObjectType.Contact.getKeyPrefix();
        List<Event> events = new List<Event>();
        for (Integer i = 0; i < whoCount; i++)
            Id whoId = prefix + String.valueOf(i).rightPad(15, '0');
            for (Integer j = 0; j < recordsPer; j++)
                events.add(new Event(WhoId = whoId));
            Map<Id, List<Event>> grouped = EventService.groupByWhoId(events);
        system.assertEquals(whoCount, grouped.size(), 'Each WhoId should have a grouping');
        for (Id whoId : grouped.keySet())
            system.assertEquals(recordsPer, grouped.get(whoId).size(),
                'Each grouping should have the correct number of records');

I haven't checked this code over very carefully, just trying to give you the idea. If you split your code out into units in this way, your trigger testing can then be a bit more cursory in its assertions on application behavior, simply verifying the happy path.

Note from How to Write Good Unit Tests (which you should read in its entirety):

Verify the results are correct

Verifying that your code works as you expect it to work is the most important part of unit testing. It’s also one of the things that Force.com developers commonly neglect. Unit tests that do not verify the results of the code aren’t true unit tests. They are commonly referred to as smoke tests, which aren’t nearly as effective or informative as true unit tests.

A good way to tell if unit tests are properly verifying results is to look for liberal use of the System.assert() methods. If there aren’t any System.assert() method calls, then the tests aren’t verifying results properly. And, no, System.assert(true); doesn’t count.

Always remember: code coverage is not the point of unit tests. The point is to verify application behavior. Never write a test without including assertions. They're the most important part!

| improve this answer | |

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.