0

I have below trigger which runs on Lead object and create a Task based on record type and lead source values( I have taken base code from stack exchange only did some modification to access the fields from custom settings). It's working fine without any issues. I have written a test class which has given code coverage as 95% as well.

The only thing is that I feel like the way I have written the test class is not the standards of writing test classes. can someone help me how to improve the writing test classes.

Apex Trigger:

trigger TriggerLeadSourceType on Lead (after insert) {

    /*Custom setting declaration to declare the variables for trigger*/
    CustomSettings__c cs = CustomSettings__c.getInstance();
    String LeadSourceType = cs.LeadSourceType__c;

     /*Custom setting field to Enable/Disbale the trigger*/
    if (cs.Disable_Task_Creation_on_Lead__c) {
        return ;
    }

    /*Map declaration to run the code for a particular record Type*/
     Map<Id,schema.RecordTypeInfo> leadRecType = lead.sObjectType.getDescribe().getRecordTypeInfosByID();

     List<Task> taskList = new List<Task>();

      If(Trigger.isAfter || trigger.isInsert) {

        for(Lead leadObj: Trigger.new) {

            /* checking the LeadSource Values from custom settings*/

            if(LeadSourceType != '' && leadObj.LeadSource != null && leadObj.LeadSource.containsIgnoreCase(LeadSourceType)){

                /*If condition to allow the Task creation based on Record Type declaration*/
                if(leadRecType.get(leadObj.RecordTypeID).getName() != 'Registration') continue;

                /*Task fields declaration*/

                task taskObj = new Task( 
                     WhoID = leadObj.Id,
                     Subject = 'Google',
                     Priority = 'Normal',
                     Status = 'Not Started',
                     Type = 'Action',
                     ownerID = leadObj.ownerID
                 );

                /*Adding Task related data to a list*/ 
                taskList.add(taskObj);      
            }
        }

        /*insert condition on task list which allows us to create the task for related Lead record */
        if(!taskList.isEmpty()){

            insert taskList;
        }   

      }
}

Apex Test Class:

@isTest
private class TestTriggerLeadSourceType {

    private static List<Lead> leadList;

    static testMethod void initData() {

        CustomSettings__c customSettingObj = new CustomSettings__c(
        Name = 'Default',
        LeadSourceType__c = 'Web',
        Disable_Task_Creation_on_Lead__c = false);
        insert  customSettingObj;

        String leadSourceStg = customSettingObj.LeadSourceType__c;


        Schema.DescribeSObjectResult cfrSchema = Schema.SObjectType.Lead; 
        Map<String,Schema.RecordTypeInfo> LeadRecordTypeInfo = cfrSchema.getRecordTypeInfosByName(); 
        Id rtId = LeadRecordTypeInfo.get('Registration').getRecordTypeId();

/**/

        leadList = new List<Lead>{new Lead(FirstName = 'LeadTest First Name', LastName = 'LeadTest LastName', 
        Company = 'Lead Company',recordtypeid = LeadRecordTypeInfo.get('Registration').getRecordTypeId(), LeadSource = leadSourceStg )};

        insert leadList;

        system.assertEquals(leadSourceStg, 'Web');
        system.assertNotEquals(leadSourceStg, 'Web Test');


    }

}
2
  • 1
    You don't need the second assert. If the first is true then the second has to be true. If the first is not true you will never hit the second. Also, no need to assert your custom setting value. You should be checking the results of your code like was a task created or not and was the lead updated or not all depending on what the goal of your test is – Eric Apr 26 '17 at 11:27
  • NB the syntax is system.assertEquals(expected_value, actual_value, failure_message) and the same goes for assertNotEquals. – Adrian Larson Apr 26 '17 at 12:01
2

Some General resources

How to write good unit tests

An introduction to apex code test methods

TL;DR Some practical information directly related to your code

The business requirements and logic in your code should drive the tests. Multiple tests covering a specific aspect of the business or code logic and asserting results. Test should cover both positive and negative cases.

Test methods imho are an art form. Doing them correctly when working on a large code base provides a lot of confidence that your changes did not break thing if your well written tests continue to pass.

Reusable code

This line can be placed in a utility class method and it can be used by both your trigger and you test methods. Seems nitpicky but illustrates a good practice especially since your tests and live code can reuse the same criteria a change in that criteria will self update tests (depending on the circumstances)

/*Custom setting declaration to declare the variables for trigger*/
    CustomSettings__c cs = CustomSettings__c.getInstance();
    String LeadSourceType = cs.LeadSourceType__c;

for example:

public static string sourceTypeSetting(){
    return CustomSettings__c.getInstance().LeadSourceType__c;
}

public static void setDisableFlag(boolean isDisabled){
    CustomSettings__c tmp = CustomSettings__c.getInstance();
    tmp.Disable_Task_Creation_on_Lead__c = isDisabled;
    upsert tmp; //using upsert in case CS does not exist yet
}

I realize not the best example, but lets say that you need to change the CS field used, placing in a utility method you can just update it there. In the test method if set during a testSetup this method can be called to check the current value and see if it needs changed rather than writing it again in the test method or having to update if the Type Setting location changes etc.

Test Utility Methods

We will be creating a bunch of leads in our tests, some that meet criteria and some that do not. Last thing you want to do if the criteria on the trigger changes is to have to update every place a lead is created when you could isolate that code as follows:

public static lead createLeadCauseTask(){
    return New Lead(FirstName = 'LeadTest First Name', LastName = 'LeadTest LastName', 
       Company = 'Lead Company',recordtypeid = LeadRecordTypeInfo.get('Registration').getRecordTypeId(), 
        LeadSource = ClassName.sourceTypeSetting() ); //see what I did with the CS
}

public static lead createLeadDoesNotCauseTask(){
    return New Lead(FirstName = 'LeadTest First Name', LastName = 'LeadTest LastName', 
       Company = 'Lead Company' 
    ); 
}

This way when you want to create a specific lead, simply call this method. If criteria in the trigger changes, you only have to update one line in your tests

Also, @testSetup method should be used to set up data that will be used throughout all tests

@TestSetup

    public static void setupCommonData(){
       CustomSettings__c customSettingObj = new CustomSettings__c(
            Name = 'Default',
            LeadSourceType__c = 'Web',
            Disable_Task_Creation_on_Lead__c = false);
            insert  customSettingObj;
    }

Multiple specific tests

Examine your business logic and logic in your trigger and you will be able to see the test methods you need to write. Find all the branches, possiblilities, etc and write methods for them

From the code (and it may be overkill but you get the idea) you can see the following possible test methods:

Custom setting logic

    //Custom setting field to Enable/Disbale the trigger
    if (cs.Disable_Task_Creation_on_Lead__c) {
        return ;
    }

Create lead with criteria where a task would be created and set the cs.Disable_Task_Creation_on_Lead__c to false in your test. Assert that no tasks were created

public static test method void disableTaskCreation(){
   //Set disabled flag
   ClassName.setDisableFlag(true); //see above comments
   //Create a lead that meets 
   Lead testLead = createLeadCauseTask();
   test.startTest(); //use start and stop test to isolate the functional parts of the test and various other reasons
   insert lead; //No task should have been created as the CS was not set.
   test.stoptest();

   //Query tasks and assert than there are none

}

Why go through all of this? Lets say someone updates your code to either remove the disabled check or changes the place where it is pulled from pr whatever. All other things being equal this method will catch that the original intention of the code is no longer working as expected as a task would be created.

Trigger Context

Our next test comes from If(Trigger.isAfter || trigger.isInsert) { and can be incorporated into the above by (reusing test methods)

public static test method void onInsertOnly(){
   disableTaskCreation(); //Execute the above test, no need to rewrite it
   //Cannot use test.start and stop again but that is ok for this test
   ClassName.setDisableFlag(false); //allow code to execute
   //Query for the lead
   Lead newLead = [Select Id From Lead]; //There is only one at this point could add some error handling but if there is an error we want this to fail, the how it fails it the question
   update lead; //Should NOT create a new task even though it meets lead criteria and disable flag is not set. (Insert only is criteria)
   //Query tasks and assert there are none
}

Why Let says you or someone else comes along and feels it is a good idea to add to your trigger and change the context to include After Update but does not modify your code. Now a task will be created on insert and on update. This test will catch that.

Specific criteria in a logic branch

if(LeadSourceType != '' && leadObj.LeadSource != null && leadObj.LeadSource.containsIgnoreCase(LeadSourceType)){

In this one set things up so each of the values would fail the comparison, insert a lead that other than the above would cause a task to be inserted, assert no tasks were created, change one of the criteria to pass the comparison, insert another lead, assert no tasks, repeat until all properties pass, at which point assert that a task was created

You could break this up into several methods but for the sake of brevity

public static testmethod void leadCriteria(){

   //CS is not disabled and LS is set up as wee need it due to testSetup method
   //But we need them to be null. Did not make reusable as so far only method to do this
   CustomSettings__c cs = CustomSettings__c.getInstance();
   cs.LeadSourceType__c = null;
   upsert cs;

   Lead originalLead = createLeadCauseTask(); //will use to create new leads later
   originalLead.LeadSource = 'Web'; //Since we set the CS to null this is null now so need to set its value. Could make Web a static property to use here and keep the value in one spot too
   Lead workingLead = originalLead.clone(false, true);
   workingLead.LeadSource = null; //this is a part of the condition

   //Now insert lead. Everything should be set up to hit the **IF** conditional but not pass
   insert workingLead;

   //Query tasks and assert that none were created

   workingLead = originalLead.clone(false, true); //clone lead so source on lead is set
   insert workingLead;

   //Query tasks and assert that none were created (CS is still null)

   cs.LeadSourceType__c = 'NOT ME'; //Set to a value that lead does not contain
   workingLead = originalLead.clone(false, true); //clone lead so source on lead is set but still Web so not equal to the CS value of NOT ME
   insert workingLead;

   //Query tasks and assert that none were created (CS does not match lead)   

   cs.LeadSourceType__c = 'Web'; //Set to a value that lead does match
   workingLead = originalLead.clone(false, true); //clone lead so source on lead is set equal to the CS value of Web
   insert workingLead;

   //Query tasks and assert that one **WAS** created (CS does matchs lead and is not null) and assert the values are set on the task appropriately 
}

Why If any of the criteria is updated in the trigger by accident or a logic issue is introduced this test will assure that the business requirement is still met. Note the test can be broken into several methods as well but that is your call

Lead Record Type

This line is the next logic to test if(leadRecType.get(leadObj.RecordTypeID).getName() != 'Registration') continue;

public static testmethod void leadRecordTypeIncorrect(){
   //CS is set properly to create a task
   //Get a proper lead - then remove / change record type so test is specific
   Lead workingLead = createLeadCauseTask();
   workingLead.RecordTypeId = null; //Or set to something other than registration
   insert workingLead;

   //Query tasks and assert one was NOT created
}

public static testmethod void leadRecordTypeCorrect(){
   //CS is set properly to create a task
   //Get a proper lead
   Lead workingLead = createLeadCauseTask();
   insert workingLead;

   //Query tasks and assert one WAS created and assert the values are set on the task appropriately 
}

Why If the record type criteria ever gets modified or bypassed this test will catch that.

Negative Test Case

We also need to ensure that a negative test passes. A negative test is one where the trigger does NOT do something. In this case we want to ensure that if a record that does not meet criteria does not create a task. I always like to have a simple method to test this in addition to all the above to attempt to catch things I did not account for in other tests

public static testmethod void simpleNegativeTest(){
   //CS settings created already in testSetup
   //Get a lead that does not match criteria
   Lead newLead = createLeadDoesNotCauseTask();
   insert newLead;
   //Query tasks and assert that none were created
}

I think that give you a good enough idea. Some other things to keep in mind:

  1. Use Test.Start and StopTest where appropriate
  2. You can reuse test methods in other tests if the code is duplicated
  3. When you assert do not just assert that a record was created but also assert that the values were set appropriately
  4. Loog at your logic, set up the test criteria so the logic in the code does what you are testing
  5. Incorporate business logic in your tests. Sometimes when doing so you will find that your code actually does not cover the business logic. both tests and code should meet business requirements

A note on bulk testing:

Simple inserting 200 leads is not bulk testing. You need to incorporate the logic in the records. In one go insert 200 records which contain a mix of leads that both should and should not create tasks. How you mix this and keep track of it is another subject. If you insert all leads of the same outcome you will miss logic issues in the code that break during bulk inserts.

I know there is a lot I missed, stuff I got wrong or typos etc but this should at least give you and idea and a starting point to begin to understand some fundamental art behind writing proper tests.

Properly written test not only ensure your code works as expected but gives you confidence that when making updates to your codebase if your tests continue to pass then the updates did not alter core logic

3
1


You can use a utility method for creating test data, I am giving you one example rest you can modify according to your need.

  1. Create a Utility class

    @isTest
    public with sharing class TestDataUtility {
    
    //Populates and returns a list of Contact records to be used in test classes
    public static List<Contact> getContactRecords(Integer pCount){
      List<Contact> lstContact = new List<Contact>();
    
      for(Integer contactCount = 1; contactCount <= pCount; contactCount ++){
        lstContact.add(new Contact(FirstName = 'Test' + contactCount,
                                   LastName = 'Contact'));
      }//for
        return lstContact;
      }//End getContactRecords
    }//TestDataUtility 
    
  2. In your Test class use that method like

    @isTest
    private class YourTestClass {
      static testMethod void yourTestMethod() {
        /*Create Test records for Contact*/
        List<Contact> contactList = TestDataUtility.getContactRecords(1);
        insert contactList;
        /*Use This List For Further Manipulations*/
      }//yourTestMethod
    }//YourTestClass
    

Advantages of creating test data using Utility Method:-

  1. This will give you flexibility such that, lets say in future a required field is added on Contact object, then in order to pass your test method you need to provide value in your test-data of that required field, if you simply update the method in utility this will resolve your problem, however if you are creating test data in your test class itself then you need to edit each test class of yours and provide value of required field, wherever you are using contact object.

  2. If you want to test on bulk data, you just need to pass Integer - Number as a argument which will return you list of records for the number you provided.

      List<Contact> contactList = TestDataUtility.getContactRecords(1);
    

    Here I have passsed "One" so it will return me one contact record, If I will pass 200 the it will give me list of 200 contact records, as it is always better to test your trigger for 200 records.

Hope this helps you!

0

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.