You need to sign in to do that
Don't have an account?
Jack Ins
Need help with code covereage
Hi all,
I need some help with covering a trigger. My test calss gets me 58% of the way there but I would relly like to get closer to 100%.
My trigger is not big and should be easy help. Just need pointed in the right direction.
Trigger that I am trying to cover:
OpportunityContactAssoc (Code Covered: 58%) line source 1 trigger OpportunityContactAssoc on Opportunity (before insert, before update) { 2 List<Contact> cntList = new List<Contact>(); 3 List<OpportunityContactRole> oppCntRole = new List<OpportunityContactRole>(); 4 public String cntID {get;set;} 5 6 for (Opportunity o: Trigger.new) { 7 if (o.Customer_Number__c != null) 8 { 9 try {Contact cnts = [select Id, cnt_plcsc_CustNmbr_ExtID__c, cnt_plcsc_Customer_Number__c 10 from Contact 11 where cnt_plcsc_Customer_Number__c =: o.Customer_Number__c 12 And cnt_plcsc_CustNmbr_ExtID__c =: o.Customer_Number__c + 'Ins01' limit 1]; 13 cntID = cnts.Id; 14 cntList.add(cnts);} catch (Exception e) {System.debug('Contact Record Does Not Exist.' + cntID);} 15 16 if(cntID != null) 17 { 18 OpportunityContactRole ocr = new OpportunityContactRole(ContactId=cntID, OpportunityID = o.Id, 19 Role = 'Primary Insured', IsPrimary = true); 20 upsert new List<OpportunityContactRole>{ocr}; 21 22 for(OpportunityContactRole oppcnts : [select Id, ContactId, OpportunityId, Role, IsPrimary 23 from OpportunityContactRole 24 where IsPrimary =: false And Role =: 'Primary Insured' And OpportunityId =: o.Id]) 25 { 26 delete oppcnts; 27 } 28 29 } 30 } 31 else { 32 try { Contact cnts = [select Id, MailingStreet, MailingState, cnt_plcsc_Customer_Number__c, 33 MailingPostalCode, MailingCity, LastName, FirstName, Email 34 from Contact 35 where Email =: o.Consumer_Email__c And MailingPostalCode =: o.Zip__c And RecordTypeId =: '01240000000DVfa' limit 1]; 36 cntID = cnts.Id; 37 o.Customer_Number__c = cnts.cnt_plcsc_Customer_Number__c; 38 cntList.add(cnts);} catch (Exception e) {System.debug('Contact Record Does Not Exist.');} 39 40 if(cntID != null){ 41 OpportunityContactRole ocr = new OpportunityContactRole(ContactId=cntID, OpportunityID = o.Id, 42 Role = 'Primary Insured', IsPrimary = true); 43 upsert new List<OpportunityContactRole>{ocr}; 44 } 45 } 46 } 47 }
test class (please don't laugh:smileywink:):
@isTest private class TestOpportunityContactRole { static testMethod void TestmyOpportunityContactRoleTrigger() { // TO DO: implement unit test Test.startTest(); Contact cntExisting = new Contact( FirstName = 'Test', LastName = 'Tester', RecordTypeId = '01240000000DVfaAAG', Email='test@hanover.com', cnt_plcsc_CustNmbr_ExtID__c = '123456789INS01', cnt_plcsc_Customer_Number__c='123456789'); insert cntExisting; Contact cntExisting2 = new Contact( FirstName = 'Test', LastName = 'Tester2', RecordTypeId = '01240000000DVfaAAG', Email='test2@hanover.com', cnt_plcsc_CustNmbr_ExtID__c = '2123456789INS01', cnt_plcsc_Customer_Number__c='2123456789'); insert cntExisting; Opportunity myOppPolicyTest = new Opportunity(StageName='Quoted', CloseDate=Date.today(), Name='Tester1', Policy_Id__c='ABC1234567', Pol_Sym__c='ABC', Pol_No__c='1234567',LOB__c='Auto', opp_plcsc_Integration_ID__c='A1234567', Customer_Number__c = '123456789'); insert myOppPolicyTest; OpportunityContactRole ocr = new OpportunityContactRole(OpportunityId = myOppPolicyTest.Id, ContactId = cntExisting.Id, Role = 'Primary Insured', IsPrimary = true); upsert new List<OpportunityContactRole> {ocr}; System.assert(myOppPolicyTest.Customer_Number__c == '2123456789'); Test.stopTest(); } }
@JackIns
To cover any lines that were not covered, you will need to ensure that the records you are working with meet the conditions in the if statements as well as the parent FOR SOQL statements.
Takle each missing line one at a time. Get them covered, then go back and look at your test class and consolidate where necessary.
Also, you do not have to cover all the lines in one method. You can have multiple tests methods and when run together they cover the whole code. (i.e. method A covers the first Positive sceniero, Method 2 covers the negative, etc.)
The previous poster is technically correct, but it will not solve your immediate issue. With the trigger as you currently have it, you will run into governor limits after a small amount of records. In order to handle bulk updates/inserts you must ensure that you SOQL and DML statement are outside of all loops. This will require the use of Sets and Maps and is one of those milestones in your learning process of Apex. Doing a search for "Bulkify" will yield a lot of information on these boards.
All Answers
Just a quick glance,
Your second insert should be giving you an error as you already inserted it.
change
to
What lines are not being covered?
Lines 31 to 43 will only be covered if the Customer_Number__c is = Null and I do not see you inserting a record without a customer number.
queries within a for loop?
That's a really bad practice, what if I insert bulk records, here I will be hitting the limits!
First thing you need to do here is avoid any queries within a for loop.
Same thing applies for DML statements, you have all the dml's within the for loop.
Starz26,
Being a new developer trying to learn to code and based on the sarcasm that I recieved from visha@force it seems that I may have taken the wrong direction in my trigger.
I did add code to my test class based on your suggestions and was able to increase my coverage to 70% but I am still not covering lines , 24, 26, 36, 37, 41 and 43.
Should I be looking to rewrite my trigger for better optimization?
Thanks for your help.
@JackIns
To cover any lines that were not covered, you will need to ensure that the records you are working with meet the conditions in the if statements as well as the parent FOR SOQL statements.
Takle each missing line one at a time. Get them covered, then go back and look at your test class and consolidate where necessary.
Also, you do not have to cover all the lines in one method. You can have multiple tests methods and when run together they cover the whole code. (i.e. method A covers the first Positive sceniero, Method 2 covers the negative, etc.)
The previous poster is technically correct, but it will not solve your immediate issue. With the trigger as you currently have it, you will run into governor limits after a small amount of records. In order to handle bulk updates/inserts you must ensure that you SOQL and DML statement are outside of all loops. This will require the use of Sets and Maps and is one of those milestones in your learning process of Apex. Doing a search for "Bulkify" will yield a lot of information on these boards.
Thank you very much. I will start with your recommendations and see where I can improve my code coverage and learning curve.
I appreciate your patience and tact with a beginner developer.
hey Jack,
that wasn't sarcastic. I just wanted you to know that it's a bad practice and being a new developer, it will help you a lot if you start avoiding such practices as early as possible. I may have sounded sarcastic but I only intended to make sure you know your code needs to be changed :)
* Whenever you are writing a trigger, you should bulkify it, reason being if data is inserted/updated/deleted through data loader, your trigger.new or trigger.old list can have thousands of records. In that case when you have dml and queries inside those trigger.new/trigger.old for loops, you are bound to hit the limits. So you need to avoid these things and always make sure your triggers are bulkified.
Thanks Vishal,
Thank you for the follow up. It's difficult to learn a new language when for so many years coding in a completely different language. The concepts are very much different. I will be looking into the Bulkify process and start learning how this works.
Thanks again for your response.
Vishal,
I took a crack at bulkifying my code and this is where I am at. It does not seem to being loading records as I would have expected. Can you look at it and help me identify where I am going wrong?
Thanks Dwayne
Starz26,
I have spent some time to get my trigger Bulkified and wondered if you can look at it to let me know if you see any issues. It is working great and I am so thankful for all the help. What a great little mile stone. :)
Thanks Dwayne
looks good at first glance....
congrats on the milestone