function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
Anoop Patel 11Anoop Patel 11 

Test Class Coverage Won't Go Above 72%

I have the test class below that only has 72% code coverage and I can't seem to increase it anymore. My trigger fires to make all Contacts related to an Account inactive, when a Account has been made inactive using a checkbox (this part seems to be covered). When the checkbox is unchecked then it reserves and makes all Contacts active (this part is not covered in bold below). I have tried various ways i.e. updating the Account, inserting a Contact with the inactive flag equals true with the latest version below. but still doesn't get past 72%.

Test Class

@isTest
private class TestUpdateContactToActiveOrInactive {

static testMethod void UpdateContactToActiveOrInactive() {
         
          Account acc = new Account();
          list<RecordType> rt = [SELECT Id FROM RecordType WHERE Name = 'Customer' and SobjectType='Account'];
          acc.RecordTypeId=rt[0].id;
          acc.Name = 'test acct';
          acc.Industry = 'Retail';
          try {
              insert acc;
          } catch (DmlException e) {}
   
          Contact c = new Contact();
          c.AccountId = acc.id;
          c.LastName='test cont';
          c.MobilePhone= '(0)00 0000 0000';
           try {
              insert c;
          } catch (DmlException e) {}
                   
          acc.Inactive__c = True;
          try {
          Update acc;         
          } catch (DmlException e) {}    
         
          Account acc1 = new Account();
          list<RecordType> rt1 = [SELECT Id FROM RecordType WHERE Name = 'Customer' and SobjectType='Account'];
          acc1.RecordTypeId=rt1[0].id;
          acc1.Name = 'test acct1';
          acc1.Industry = 'Retail';
          acc1.Inactive__c = True;
          try {
              insert acc1;
          } catch (DmlException e) {}    
         
          Contact c1 = new Contact();
          c1.AccountId = acc1.id;
          c1.LastName='test cont1';
          c1.MobilePhone= '(0)00 0000 0000';
          c1.Inactive_Contact__c = True;
           try {
              insert c1;
          } catch (DmlException e) {}         
         
          acc.Inactive__c = False;
          try {
          Update acc1;         
          } catch (DmlException e) {}           

}
}
Best Answer chosen by Anoop Patel 11
pconpcon
Hermes, you are correct.  That's what I get for typing it blindly and not testing it :)

Updated test class
 
@isTest
private class TestUpdateContactToActiveOrInactive {
    static testMethod void UpdateContactToActiveOrInactive() {
        RecordType rt = [
            select Id
            from RecordType
            where Name = 'Clearing Member' and
                SobjectType='Account'
        ];  

        Account testAccount = new Account(
            Name = 'Test Account',
            RecordTypeId = rt.Id,
            Industry = '2nd / 3rd Tier Bank',
            Inactive__c = false
        );
        insert testAccount;

        List<Contact> testContacts = new List<Contact>();

        testContacts.add(new Contact(
            AccountId = testAccount.Id,  
            LastName = 'Test Contact 1',
            MobilePhone= '+44(0)00 0000 0000';
        ));

        testContacts.add(new Contact(
            AccountId = testAccount.Id,
            LastName = 'Test Contact 2',
            MobilePhone= '+44(0)00 0000 0000';
        ));

        Test.startTest();

        testAccount.Inactive__c = true;
        update testAccount;

        Test.stopTest();

        for (Contact result: [
            select Inactive_Contact__c, 
                Name,
                Reason_For_Inactivity__c
            from Contact
            where AccountId = :testAccount.Id
        ]) {
            System.assert(
                result.Inactive_Contact__c,
                'The contact should be inactive ' + result.Name
            );
            System.assertEquals(
                'Account Inactive',
                result.Reason_for_Inactivity__c,
                'Did not get the right reason ' + result.Name
            );
        }   
    }
}

 

All Answers

pconpcon
Can you please include the code you are trying to test as well?  This will help to see what corner cases you are doing in your code that you are not testing for in your tests.

A couple of notes about your tests:
  • I would not wrap your inserts in try/catch blocks because it may be obfuscating any errors you get.  It is ok for exceptions to be un-handled in tests since you want to know when things fail
  • You should only make the RecordType call once and reuse it since it is the same for both accounts
  • You should use Test.startTest and Test.stopTest to wrap the parts that you are actually testing to reset the governor limits
  • You should also include System.assert* calls to ensure that your data was actually changed as expected.  This will also include querying your changed results.
Anoop Patel 11Anoop Patel 11
This is my trigger:

trigger UpdateContactToActiveOrInactive on Account (after update){

    Set<Id> AccIds = new Set<Id>();
    Set<Id> ActIds = new Set<Id>();

         for (Account a : trigger.new){
          if (a.Inactive__c!=false)
             AccIds.add(a.id);}
     
       List<Contact> ContactsToUpdate = new List<Contact>();
       ContactsToUpdate = [SELECT Inactive_Contact__c, Reason_for_Inactivity__c FROM Contact WHERE AccountId IN :AccIds];
        
         for(Contact c :ContactsToUpdate){
            
             if(c.Inactive_Contact__c == False){ // condition
                c.Inactive_Contact__c =True;   // field update 
                c.Reason_for_Inactivity__c ='Account Inactive';
                }                               
                }
                update ContactsToUpdate;
               
        for (Account acc : trigger.new){
          if (acc.Inactive__c==false)
             ActIds.add(acc.id);}
       
     List<Contact> ContactsToActivate = new List<Contact>();
    
       ContactsToActivate = [SELECT Inactive_Contact__c, Reason_for_Inactivity__c FROM Contact WHERE AccountId IN :ActIds];
        
         for(Contact con :ContactsToActivate){
            
             if(con.Inactive_Contact__c == True){ // condition
                con.Inactive_Contact__c =False;   // field update 
                con.Reason_for_Inactivity__c ='';
                }                               
                }
                update ContactsToActivate;
                                                    
}



 
Hermes Yan 11Hermes Yan 11
Your test class updates the account to false but in your trigger the filter conditions only account for when the account is not false.

Seems to be a logic error in your trigger. Should probably fire when Inactive has changed.
Anoop Patel 11Anoop Patel 11
I'm new to writting triggers so not sure if the trigger has been created correctly. What would be the best way to write this like?

 
Anoop Patel 11Anoop Patel 11
Weird.... I have changed the test class to the below leaving the trigger as is and its increased the code coverage to 86%!!!!!

@isTest
private class TestUpdateContactToActiveOrInactive {

static testMethod void UpdateContactToActiveOrInactive() {

Test.startTest();

          List<Contact> lstC = new List <Contact>();        
                    
          Account acc = new Account();
          List<RecordType> rt = [SELECT Id FROM RecordType WHERE Name = 'Clearing Member' and SobjectType='Account'];
          acc.RecordTypeId=rt[0].id;
          acc.Name = 'test acct';
          acc.Industry = '2nd / 3rd Tier Bank';
          acc.Inactive__c = False;
          
          insert acc;
    
          Contact c = new Contact();
          c.AccountId = acc.id;
          c.LastName='test cont';
          c.MobilePhone= '+44(0)00 0000 0000';
          lstC.add(c);
          
          Contact c1 = new Contact();
          c1.AccountId = acc.id;
          c1.LastName='test cont1';
          c1.MobilePhone= '+44(0)00 0000 0000';
          lstC.add(c1);
         
          insert lstC;
                    
          acc.Inactive__c = True;
          Update acc;           
          
          c.Reason_for_Inactivity__c ='Account Inactive';
          Update c;        
Test.stopTest();                        
}
}
pconpcon
Your test doesn't really "Test" anything.

Each test should follow the following structure:
  1. Setup of test data. This includes creation of any data needed by your class.  Account, Contacts etc
  2. Starting the test. This is calling Test.startTest() to reset the governor limits
  3. Calling your class / method
  4. Stopping the test.This is calling Test.stopTest() to reset the governor limits and allow for any async jobs to finish
  5. Asserting that your changes have worked
    1. If you have inserted/updated/deleted data, you need to query for the updates
    2. Run System.assert, System.assertEquals, System.assertNotEquals to verify that you got the correct data back

I have taken the liberty of updating your trigger and making it a little more streamlined.
 
trigger UpdateContactToActiveOrInactive on Account (after update){
    Set<Id> accountIds = new Set<Id>();
    Map<Id, List<Contact>> accountToContactMap = new Map<Id, List<Contact>>();

    for (Account account: Trigger.new) {
        Account oldAccount = Trigger.oldMap.get(account.Id);
        if (account.Inactive__c != oldAccount.Inactive__c) {
            accountIds.add(account.Id);
            accountToContactMap.put(account.Id, new List<Contact>());
        }
    }
    
    for (Contact contact: [
        select Inactive_Contact__c,
            Reason_for_Inactivity__c
        from Contact
        where AccountId in :accountIds
    ]) {
        accountToContactMap.get(contact.AccountId).add(contact);
    }  
    
        
    for (Id id: accountIds) {
        Account account = Trigger.newMap.get(id);

        if (account.Inactive__c == false) {
            for (Contact contact: accountToContactMap.get(id)) {
                contact.Inactive_Contact = false;
                contact.Reason_for_Inactivity__c = '';
            }
        } else {
            for (Contact contact: accountToContactMap.get(id)) {
                contact.Inactive_Contact = true;
                contact.Reason_for_Inactivity__c = 'Account Inactive';
            }  
        }
    }

    if (!accountIds.isEmpty()) {
        List<Contact> contactsToUpdate = new List<Contact>();
        contactsToUpdate.addAll(accountToContactMap.values());

        if (!contactsToUpdate.isEmpty()) {
            update contactsToUpdate;
        }
    }
}

Here is a single test that you can expand upon to cover the bulk case (updating multiple accounts at one time) as well as the Inactive to Active senario
 
@isTest
private class TestUpdateContactToActiveOrInactive {
    static testMethod void UpdateContactToActiveOrInactive() {
        RecordType rt = [
            select Id
            from RecordType
            where Name = 'Clearing Member' and
                SobjectType='Account'
        ];  

        Account testAccount = new Account(
            Name = 'Test Account',
            RecordTypeId = rt.Id,
            Industry = '2nd / 3rd Tier Bank',
            Inactive__c = false
        );
        insert testAccount;

        List<Contact> testContacts = new List<Contact>();

        testContacts.add(new Contact(
            AccountId = testAccount.Id,  
            LastName = 'Test Contact 1',
            MobilePhone= '+44(0)00 0000 0000';
        ));

        testContacts.add(new Contact(
            AccountId = testAccount.Id,
            LastName = 'Test Contact 2',
            MobilePhone= '+44(0)00 0000 0000';
        ));

        Test.startTest();

        testAccount.Inactive__c = true;

        Test.stopTest();

        for (Contact result: [
            select Inactive_Contact__c, 
                Name,
                Reason_For_Inactivity__c
            from Contact
            where AccountId = :testAccount.Id
        ]) {
            System.assert(
                result.Inactive_Contact__c,
                'The contact should be inactive ' + result.Name
            );
            System.assertEquals(
                'Account Inactive',
                result.Reason_for_Inactivity__c,
                'Did not get the right reason ' + result.Name
            );
        }   
    }
}

Additional Reading
[1] http://www.sfdc99.com/2013/05/14/how-to-write-a-test-class/
[2] http://pcon.github.io/presentations/testing/
[3] http://blog.deadlypenguin.com/blog/2014/07/23/intro-to-apex-auto-converting-leads-in-a-trigger/
Hermes Yan 11Hermes Yan 11
Oops I did not notice the logic is repeated again for the false case. Taking a second look your test cases should cover your trigger code. The first half already covers 72% so that indicates probably the second update is erroring.

I would take pcon's advice and remove the try catch blocks in the insert statements in your test class. I suspect that either your second account or contact is not actually inserting.

Moving the second part of your test class to it's on test method is also a good idea. Eliminates the case that the data is in conflict with the first set of data. 
 
Hermes Yan 11Hermes Yan 11
Just saw all the posts that came before mine.

Line 35 needs an update to the account in the unit test
pconpcon
Hermes, you are correct.  That's what I get for typing it blindly and not testing it :)

Updated test class
 
@isTest
private class TestUpdateContactToActiveOrInactive {
    static testMethod void UpdateContactToActiveOrInactive() {
        RecordType rt = [
            select Id
            from RecordType
            where Name = 'Clearing Member' and
                SobjectType='Account'
        ];  

        Account testAccount = new Account(
            Name = 'Test Account',
            RecordTypeId = rt.Id,
            Industry = '2nd / 3rd Tier Bank',
            Inactive__c = false
        );
        insert testAccount;

        List<Contact> testContacts = new List<Contact>();

        testContacts.add(new Contact(
            AccountId = testAccount.Id,  
            LastName = 'Test Contact 1',
            MobilePhone= '+44(0)00 0000 0000';
        ));

        testContacts.add(new Contact(
            AccountId = testAccount.Id,
            LastName = 'Test Contact 2',
            MobilePhone= '+44(0)00 0000 0000';
        ));

        Test.startTest();

        testAccount.Inactive__c = true;
        update testAccount;

        Test.stopTest();

        for (Contact result: [
            select Inactive_Contact__c, 
                Name,
                Reason_For_Inactivity__c
            from Contact
            where AccountId = :testAccount.Id
        ]) {
            System.assert(
                result.Inactive_Contact__c,
                'The contact should be inactive ' + result.Name
            );
            System.assertEquals(
                'Account Inactive',
                result.Reason_for_Inactivity__c,
                'Did not get the right reason ' + result.Name
            );
        }   
    }
}

 
This was selected as the best answer
Anoop Patel 11Anoop Patel 11
Thank you sincerly guys.

I receive this error when saving the trigger;

Error: Compile Error: Incompatible argument type LIST<LIST<Contact>> for All method on LIST<Contact> at line 41 column 9

trigger UpdateContactToActiveOrInactive on Account (after update){
    Set<Id> accountIds = new Set<Id>();
    Map<Id,List<Contact>> accountToContactMap = new Map<Id,List<Contact>>();

    for (Account account: Trigger.new) {
        Account oldAccount = Trigger.oldMap.get(account.Id);
        if (account.Inactive__c != oldAccount.Inactive__c) {
            accountIds.add(account.Id);
            accountToContactMap.put(account.Id, new List<Contact>());
        }
    }
   
    for (Contact contact: [
        select Inactive_Contact__c,
            Reason_for_Inactivity__c
        from Contact
        where AccountId in :accountIds
    ]) {
        accountToContactMap.get(contact.AccountId).add(contact);
    } 
   
       
    for (Id id: accountIds) {
        Account account = Trigger.newMap.get(id);

        if (account.Inactive__c == false) {
            for (Contact contact: accountToContactMap.get(id)) {
                contact.Inactive_Contact__c = false;
                contact.Reason_for_Inactivity__c = '';
            }
        } else {
            for (Contact contact: accountToContactMap.get(id)) {
                contact.Inactive_Contact__c = true;
                contact.Reason_for_Inactivity__c = 'Account Inactive';
            } 
        }
    }

    if (!accountIds.isEmpty()) {
        List<Contact> contactsToUpdate = new List<Contact>();
        contactsToUpdate.addAll(accountToContactMap.values());

        if (!contactsToUpdate.isEmpty()) {
            update contactsToUpdate;
        }
    }
}

 
pconpcon
That is my mistake.  I was trying to make it a little faster, and apparently List does not have an addAll method that works that way.  Below is the updated code
 
trigger UpdateContactToActiveOrInactive on Account (after update){
    Set<Id> accountIds = new Set<Id>();
    Map<Id, List<Contact>> accountToContactMap = new Map<Id, List<Contact>>();

    for (Account account: Trigger.new) {
        Account oldAccount = Trigger.oldMap.get(account.Id);
        if (account.Inactive__c != oldAccount.Inactive__c) {
            accountIds.add(account.Id);
            accountToContactMap.put(account.Id, new List<Contact>());
        }
    }
    
    for (Contact contact: [
        select Inactive_Contact__c,
            Reason_for_Inactivity__c
        from Contact
        where AccountId in :accountIds
    ]) {
        accountToContactMap.get(contact.AccountId).add(contact);
    }  
    
        
    for (Id id: accountIds) {
        Account account = Trigger.newMap.get(id);

        if (account.Inactive__c == false) {
            for (Contact contact: accountToContactMap.get(id)) {
                contact.Inactive_Contact = false;
                contact.Reason_for_Inactivity__c = '';
            }
        } else {
            for (Contact contact: accountToContactMap.get(id)) {
                contact.Inactive_Contact = true;
                contact.Reason_for_Inactivity__c = 'Account Inactive';
            }  
        }
    }

    if (!accountIds.isEmpty()) {
        List<Contact> contactsToUpdate = new List<Contact>();
        for (List<Contact> contacts: accountToContactMap.values()) {
            contactsToUpdate.addAll(contacts);
        }

        if (!contactsToUpdate.isEmpty()) {
            update contactsToUpdate;
        }
    }
}

 
Anoop Patel 11Anoop Patel 11
This is fantastic and a great help. I will use the links you have provided along with various resources avalible to me.

Thank you both.