You need to sign in to do that
Don't have an account?
kathybb
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.
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:
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
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:
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
Thank you very much for your explanation of where I was going wrong and how to correct it. This was very helpful.
Kathy