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
kathybbkathybb 

Problem with code coverage when testing a before insert trigger

I'm working on a trigger that copies some fields from an Account record into the related Opportunity record.  Here's the trigger code:

 

trigger FillFieldsFromAcct on Opportunity (before insert) 
{
    List<Account> myAccts = new List<Account>();
    RecordType rt = [select id from RecordType where Name = 'PubDev Opportunities' and SobjectType = 'Opportunity' limit 1];
    Account acctToCopy = new Account();
    For(Opportunity oppty : Trigger.New)
    {
        oppty = (Opportunity)Oppty;
       	if(oppty != NULL ) {
            if(oppty.RecordTypeId == rt.id){
                string myAcct = oppty.AccountId;
                string qry = 'select id, name, Display__c, Mobile__c, Video__c, Monthly_Display_Imps__c, Monthly_Mobile_Imps__c, Monthly_Video_Imps__c,' + 
                	'uniques__c, vertical__c, Pageviews__c  from Account where id = :myAcct limit 1';
                acctToCopy = database.query(qry);
				if(acctToCopy != null)
                acctToCopy = (Account)acctToCopy;
                oppty.Display__c = acctToCopy.Display__c;
               	oppty.Mobile__c = acctToCopy.Mobile__c;
               	oppty.Video_PubDev__c = acctToCopy.Video__c;
               	oppty.Monthly_Display_Impressions__c = acctToCopy.Monthly_Display_Imps__c;
                oppty.Monthly_Video_Impressions__c = acctToCopy.Monthly_Video_Imps__c;
               	oppty.Monthly_Mobile_Impressions__c = acctToCopy.Monthly_Mobile_Imps__c;
               	oppty.Uniques__c = acctToCopy.Uniques__c;
                oppty.Pageviews__c = acctToCopy.Uniques__c;
                oppty.Vertical__c = acctToCopy.Vertical__c;
            }
    	}
    }
}

 

This works as expected, but I need to get the coverage on my Test Class higher than 47%.  Here is the Test Class:

 

@isTest
private class TestOppTrigger {
static testMethod void verify_Acct_update() {
    List<Opportunity> oppsToInsert = new List<Opportunity>();
    List<Account> acctsToInsert = new List<Account>();
    RecordType rt = [select id from RecordType where Name = 'PubDev Opportunities' and SobjectType = 'Opportunity' limit 1];
    integer y = 0;
    string myString = 'test';
    Account testAcct1 = new Account(Name='TestAcct1', Vertical__c = 'Education',Uniques__c = 30000, 

Pageviews__c=20000,Display__c=TRUE, Video__c =TRUE,
    Mobile__c = TRUE, Monthly_Display_Imps__c = 500000,Monthly_Mobile_Imps__c = 500003, Monthly_Video_Imps__c = 500002, 
                                   UGC_Provider__c = myString);
    acctsToInsert.add(TestAcct1);
    Account testAcct2 = new Account(Name='TestAcct2', UGC_Provider__c = myString);
    acctsToInsert.add(TestAcct2);
    try{
    database.insert(acctsToInsert);
    } catch(DMLException e) {
        system.debug('ERROR:  ' + e);
    }
    
        Account testAcct = new Account();
        
        string qry = 'Select id, name from Account where UGC_Provider__c = :myString';
        List<Account> testAccounts = database.query(qry);
        
                for(y=0;y<30;y++){
                    if(math.mod(y,2)== 0) {
                    Opportunity TestOppFilled = new Opportunity(RecordTypeID = rt.id, Name='TestOpp' + x, Account = (Account)
                        testAcct1,  testString__c = myString, StageName = '3 - On Hold',
                        CloseDate = date.parse('3/30/2033'), Pub_Dev_Rep__c = 'Jason');
                    oppsToInsert.add(TestOppFilled);
                    
                    } else {
                    Opportunity TestOppEmpty = new Opportunity(RecordTypeID = rt.id, Name= 'TestOpp' + 1000 + x, 
                        testString__c =myString, Account = (Account)testAcct2, StageName = '3 - On Hold',
                        CloseDate = date.parse('3/30/2033'), Pub_Dev_Rep__c = 'Jason');
                    oppsToInsert.add(TestOppEmpty);
                    }
                    system.debug('Size of list = ' + oppsToInsert.size() + ' at bottom of loop');
                }
                system.debug ('just before try-catch');
                try{
                    system.debug ('trying to insert Opportunities');
                    database.insert(oppsToInsert);
                } catch(DMLException e1) {
                    system.debug('ERROR:  ' + e1);
                    }
            
        string qry1 = 'Select Account.id, Name, Vertical__c,Uniques__c, Pageviews__c,Display__c, Video_PubDev__c, ' +
            'Mobile__c, Monthly_Display_Impressions__c,Monthly_Mobile_Impressions__c, Monthly_Video_Impressions__c from Opportunity ' + 
            'where testString__c = :myString limit 100';
    
        List<Opportunity> Results = new List<Opportunity>();
        Results = database.query(qry1);
       
		for (Opportunity Oppty : Results){
            if (Oppty != NULL) {
            	Oppty = (Opportunity)Oppty; 
            }
        system.assertEquals(Oppty.Vertical__c,Oppty.Account.Vertical__c );
        system.assertEquals(Oppty.Uniques__c,Oppty.Account.Uniques__c );
		system.assertEquals(Oppty.Pageviews__c,Oppty.Account.Pageviews__c );
        system.assertEquals(Oppty.Display__c,Oppty.Account.Display__c );
        system.assertEquals(Oppty.Video_PubDev__c,Oppty.Account.Video__c );
        system.assertEquals(Oppty.Mobile__c,Oppty.Account.Mobile__c );      
        system.assertEquals(Oppty.Monthly_Display_Impressions__c,Oppty.Account.Monthly_Display_Imps__c );
        system.assertEquals(Oppty.Monthly_Video_Impressions__c,Oppty.Account.Monthly_Video_Imps__c);
        system.assertEquals(Oppty.Monthly_Mobile_Impressions__c,Oppty.Account.Monthly_Mobile_Imps__c ); 
             
     
        }
        
      }
    }

 From the display of code coverage, it looks like I am doing something wrong that is causing the system to believe that the actual copying is not being tested.  Is there a better way to test this than system.assertEquals?  Any help from a more experienced developer would be most welcome.

 

Best Answer chosen by Admin (Salesforce Developers) 
crop1645crop1645

kathybb:

 

1. Your Trigger has a conceptual design error - you are fetching Accounts within the Trigger.new for loop - this will cause you SOQL governor limit issues when you update in batches. You can avoid this by fetching all Accounts into a Map at the beginning of the trigger. Using something like this:

 

Set<ID> aIdSet = new Set<ID> ();
for (Opportunity o: Trigger.new) 
   aIdSet.add(o.accountId);

Map<ID,Account> aIdToAccountMap = new Map<ID,Account> ([select id, display__c /*, ..rest of fields */ from Account where id IN: aIdSet]);

In addition, in the trigger, Oppty will never be null in Trigger.new so you can remove that if test; you can also remove the cast from Oppty to oppty

 

 

 2. System.asserts are used to verify that your code worked

 

3. I think you are missing a {..} in the trigger after the if (acctToCopy != null); furthermore, an Opportunity can never exist without its account so testing for null here is unnecessary

 

 

 

 

All Answers

crop1645crop1645

kathybb:

 

1. Your Trigger has a conceptual design error - you are fetching Accounts within the Trigger.new for loop - this will cause you SOQL governor limit issues when you update in batches. You can avoid this by fetching all Accounts into a Map at the beginning of the trigger. Using something like this:

 

Set<ID> aIdSet = new Set<ID> ();
for (Opportunity o: Trigger.new) 
   aIdSet.add(o.accountId);

Map<ID,Account> aIdToAccountMap = new Map<ID,Account> ([select id, display__c /*, ..rest of fields */ from Account where id IN: aIdSet]);

In addition, in the trigger, Oppty will never be null in Trigger.new so you can remove that if test; you can also remove the cast from Oppty to oppty

 

 

 2. System.asserts are used to verify that your code worked

 

3. I think you are missing a {..} in the trigger after the if (acctToCopy != null); furthermore, an Opportunity can never exist without its account so testing for null here is unnecessary

 

 

 

 

This was selected as the best answer
kathybbkathybb

Thank you very much for your explanation of where I was going wrong and how to correct it.  This was very helpful.

Kathy