+ Start a Discussion
Pavan Kumar PPavan Kumar P 

I need help to write a test class for trigger

trigger opptoacc on Opportunity (after insert,after update) {

    list<Account> acctList = new list<Account>();
    Set<Id> accId = new Set<Id>();
    
    for(Opportunity opp: Trigger.New){
        accId.add(opp.accountId);
        
    }
    
    
    for(Account acc:[SELECT Id,Name,Industry FROM Account WHERE Id IN: accId]){
        for(Opportunity op1:Trigger.New){

       if(Trigger.isInsert && op1.Name=='something'){

                acc.Rating='hot';

                acctList.add(acc);


            }
            
            
           
        }
    
    }
    
}

I need test class that covers more than 75% of code.

Thanks in Advance.

Regards,
Pavan Kumar P.
 
Best Answer chosen by Pavan Kumar P
Andrew GAndrew G
ok,
question?  did you do a test to see what parts weren't covered? 
If you had, you would have seen that your loop was not being executed in the trigger? why?  because the Id for a record does not exist until it is inserted.  You were creating the Opportunities in the same loop as the Accounts, and then doing the insert (for both Accounts and Opportunity)  when the loop finished.

Next, we read the test code, and we wonder why you are creating multiple Oppty with the Same Account Id, yet created 200 Accounts?  Now, it is a valid test to have 1 account with multiple Oppty - by doing so we revealed an issue with the code in the trigger.  Your code, which looped the Opportunity, but kept the same Account, kept adding the same Account to your account list which then reveals an error on the DML event as the same record is written over and over.  And, when we get to a proper loop of multiple accounts and multiple Oppty, we get a too many writes error , as the list is 200 accounts x 200 oppty - 400000 DML events (records) to try and write to disk.

So, discovering the DML issues associated with the construction of the trigger, lets revist the trigger to resolve it's issues:
How to remove duplicate values in the list?  Two easiest options - use Sets or Maps.  Either way , you will need to loop the result, as neither Sets nor Maps are directly able to be inserted/updated.  You will note that I love using DEBUG statements. And yes, you could skip the parentsAccts by doing the SELECT directly in the loop.  The other addition, is the check for the Account already having a Rating of Hot - why do a DML when we don't need to ?
trigger opptoacc on opportunity (after insert, after update) {

    Set<Id> accId = new Set<Id>();
    List<Account> parentAccts     = new List<Account>();
    Map<Id,Account> accountMap     = new Map<Id,Account>();
    List<Account> accttoUpdate    = new List<Account>();

    if (Trigger.isInsert) {
        for(Opportunity opp: Trigger.New){
            accId.add(opp.accountId);
        }
        System.debug('DEBUG: Opp Trigger: accId size ' + accId.size());
        
        parentAccts = [SELECT Id,Name,Rating FROM Account WHERE Id IN :accId];
        System.debug('DEBUG: Opp Trigger: parentAccts size ' + parentAccts.size());
        
        for(Account acc: parentAccts){
            System.debug('DEBUG: Opp Trigger: acc Id ' + acc.Id);
            for(Opportunity op1:Trigger.New){
                System.debug('DEBUG: Opp Trigger: op1 Id ' + op1.Id);
                if(op1.AccountId == acc.Id && op1.Name == 'something' && acc.Rating <> 'Hot'){
                    System.debug('DEBUG: Opp Trigger: update Account ' + acc.Name);
                    acc.Rating = 'Hot';
                    accountMap.put(acc.Id,acc);
                }
            }
        }

        for (Account acc : accountMap.values()) {
            accttoUpdate.add(acc);
        }
        System.debug('DEBUG: Opp Trigger: # number accounts to update ' + accttoUpdate.size());

        update accttoUpdate;
    }    
}
So, we have a trigger that is now good to go with variations of the bulk.  So to the test class.  I note that you seem to want to use one method to test this trigger, yet the variations of how the trigger could be invoked are more than one.  I mentioned previously about a Negative test. A Negative test is one where the trigger does not execute or it performs something unexpected. So in this case, I want to check that it doesn't fire if the name on the Opportunity is not "something". 
Additionally, by retaining the single test I first suggested, you would have realised the single record invoked the loop, but not you "bulk test", hence pointing to the bulk test being incorrect.
SO..... here is your test class with 5 x methods - hopefully each method name is descriptive enough - alternate to the method name, the insertion of some comments about what the test is trying to achieve is an excellent idea.
 
@isTest
private class OpportunityTest {

	@isTest static void test_Single_record_positive() {
		Account acc = new Account(Name='Test Account', Industry = 'Industrial', Rating = 'Cold'
			//other values as required
			);
		Insert acc;

		Opportunity opp = new Opportunity(Name = 'something', AccountId = acc.Id, stageName='Prospecting',closeDate=System.today()+5
			//other values as required
			);
		Insert opp;

		List<Account> insertedAccounts = [SELECT Id, Rating FROM Account WHERE Id = :acc.Id];
		System.assertEquals(insertedAccounts[0].Rating,'Hot');

	}
	
	@isTest static void test_Single_record_negative() {
		// now, check that the rating is not changes if the criteria are not met
		Account acc = new Account(Name='Test Account', Industry = 'Industrial', Rating = 'Cold'
			//other values as required
			);
		Insert acc;

		Opportunity opp = new Opportunity(Name = 'nothing', AccountId = acc.Id, stageName='Prospecting',closeDate=System.today()+5
			//other values as required
			);
		Insert opp;

		List<Account> insertedAccounts = [SELECT Id, Rating FROM Account WHERE Id = :acc.Id];
		System.assertEquals(insertedAccounts[0].Rating,'Cold');

	}

	@isTest static void test_Single_Account_Multi_records_positive(){
		// test for single account with multiple oppty - to ensure we have resolved the same account in list DML error
		Integer numToCreate = 200;
		List<Account> acctlist=new List<Account>();
		List<Opportunity> opplist=new List<Opportunity>();
		Set<Id> acctSet=new Set<Id>();

		Account newAccount= new Account(Name='Test Account', Rating='Cold');
		insert newAccount;

		List<Account> insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Id = :newAccount.Id];
		System.assertEquals(insertedAccounts.size(),1);

		for(Integer i=0;i<numToCreate;i++){
			Opportunity op = new Opportunity(Name ='something',	AccountId= newAccount.Id, 
				stageName='Prospecting', closeDate=System.today()+5);
			opplist.add(op);
		}
		System.assertEquals(oppList.size(),numToCreate);
		insert opplist;
		
		insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account'];
        System.assertEquals(insertedAccounts.size(),1);
		for(Account a1: insertedAccounts){
			System.assertEquals(a1.Rating,'Hot');
		}

	}

	@isTest static void test_Multi_Account_with_Single_Oppty_positive(){
		// test for multiple accounts with single oppty - to ensure bulk updates are covered
		Integer numToCreate = 200;
		List<Account> acctlist=new List<Account>();
		List<Opportunity> opplist=new List<Opportunity>();
		Set<Id> acctSet=new Set<Id>();
		for(Integer i=0;i<numToCreate;i++){
			Account acc= new Account(Name='Test Account'+i, Rating='Cold');
			acctlist.add(acc);
		}
		insert acctlist;

		List<Account> insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account%'];
		System.assertEquals(insertedAccounts.size(),numToCreate);

		for(Integer i=0;i<numToCreate;i++){
			Opportunity op = new Opportunity(Name ='something',	AccountId= insertedAccounts[i].id, 
				stageName='Prospecting', closeDate=System.today()+5);
			opplist.add(op);
		}
		System.assertEquals(oppList.size(),numToCreate);
		insert opplist;
		
		insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account%'];
		for(Account a1: insertedAccounts){
			System.assertEquals(a1.Rating,'Hot');
		}

	}

	@isTest static void test_Multi_Account_with_Multi_Oppty_positive(){
		// test for multiple accounts with multi oppty each - to ensure bulk updates are covered
		Integer numToCreate = 20; //keep and round to tens values
		List<Account> acctlist=new List<Account>();
		List<Opportunity> opplist=new List<Opportunity>();
		Set<Id> acctSet=new Set<Id>();
		for(Integer i=0;i<numToCreate;i++){
			Account acc= new Account(Name='Test Account'+i, Rating='Cold');
			acctlist.add(acc);
		}
		insert acctlist;

		List<Account> insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account%'];
		System.assertEquals(insertedAccounts.size(),numToCreate);

		for (Account acc : insertedAccounts ) {
			for(Integer i=0;i<numToCreate/10;i++){
				Opportunity op = new Opportunity(Name ='something',	AccountId= acc.id, 
					stageName='Prospecting', closeDate=System.today()+5);
				opplist.add(op);
			}
		}
		System.assertEquals(oppList.size(),numToCreate*(numToCreate/10));
		insert opplist;
		
		insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account%'];
		for(Account a1: insertedAccounts){
			System.assertEquals(a1.Rating,'Hot');
		}

	}
}

And that is a test class of which to be proud.

Regards
Andrew


 

All Answers

Andrew GAndrew G
it would be interesting to know if you had a crack at writing your own test class. and if you have tested the trigger in the UI.  Refer notes in code
 
@isTest
private class OpportunityTest {
	
	@isTest static void test_method_one() {
		// Implement test code
		Account acc = new Account(Name='Test Account', Industry = 'Industrial', Rating = 'cold'
			//other values as required
			);
		Insert acc;

		Opportunity opp = new Opportunity(Name = 'something', AccountId = acc.Id
			//other values as required
			);
		Insert opp;

		//now, since the trigger doesn't actually do anything, we could stop here with no asserts or anything  
		// and we would have a rubbish test class that does 100% coverage
		// but if the trigger actually did the insert of the list of accounts....then we would do something like

		List<Account> insertedAccounts = [SELECT Id, Rating FROM Account WHERE Id = :acc.Id];
		System.assertEquals(insertedAccounts[0].Id,'hot');

	}
	
	@isTest static void test_method_one() {
		// Implement test code
		// now, if we are good we would also create a negative test here
	}
}

 
Pavan Kumar PPavan Kumar P
Hi Andrew,

Thanks you. Its working fine.

Could please also share the test class for inserting few dummy records (say 200 records) for the same trigger mentioned above.

Regards,
Pavan Kumar P.

 
Andrew GAndrew G
I would suggest testing your knowledge and giving it a whirl yourself.

Basically, create a number limited loop, create an Oppty for each iteration, add the new Oppty to a list, insert the list (outside the loop).
Then do a select and do list.size() test and perhaps test the value of the first record in the list.

And if you are expecting to do a lot of coding (and hence test code) i would suggest creating a Test Helper Class.

Regards
Andrew
Pavan Kumar PPavan Kumar P
Hi Andrew,

Thanks for the suggestion. I have tried to write the test class for creating dummy 200 records. But, Unfortunately I am ableto get code coverage of around 50% only.

Please find my test clss below:

@isTest
public class opptest {       
        
      @isTest static void triggerTest1(){
          List<Account> acctlist=new List<Account>();
          List<Opportunity> opplist=new List<Opportunity>();
          Set<Id> acctSet=new Set<Id>();
          for(Integer i=0;i<200;i++){
          Account acc= new Account(Name='Test Account'+i, Rating='hot');
          
          acctlist.add(acc);
          acctSet.add(acc.Id);
          Opportunity op = new Opportunity(Name='something', stageName='Prospecting',closeDate=System.today()+5, AccountId= acc.id);
          opplist.add(op);
          }
          insert acctlist;
          insert opplist;
          if(opplist.size()>0){
           System.debug('Size of List is: '+ opplist.size());
          
          }
          
           List<Account> insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Id IN : acctSet];
          for(Account a1: insertedAccounts){
          System.assertEquals(a1.Rating,'hot');
         
          }
    }
    }


Thanks,
Pavan Kumar P.
Andrew GAndrew G
ok,
question?  did you do a test to see what parts weren't covered? 
If you had, you would have seen that your loop was not being executed in the trigger? why?  because the Id for a record does not exist until it is inserted.  You were creating the Opportunities in the same loop as the Accounts, and then doing the insert (for both Accounts and Opportunity)  when the loop finished.

Next, we read the test code, and we wonder why you are creating multiple Oppty with the Same Account Id, yet created 200 Accounts?  Now, it is a valid test to have 1 account with multiple Oppty - by doing so we revealed an issue with the code in the trigger.  Your code, which looped the Opportunity, but kept the same Account, kept adding the same Account to your account list which then reveals an error on the DML event as the same record is written over and over.  And, when we get to a proper loop of multiple accounts and multiple Oppty, we get a too many writes error , as the list is 200 accounts x 200 oppty - 400000 DML events (records) to try and write to disk.

So, discovering the DML issues associated with the construction of the trigger, lets revist the trigger to resolve it's issues:
How to remove duplicate values in the list?  Two easiest options - use Sets or Maps.  Either way , you will need to loop the result, as neither Sets nor Maps are directly able to be inserted/updated.  You will note that I love using DEBUG statements. And yes, you could skip the parentsAccts by doing the SELECT directly in the loop.  The other addition, is the check for the Account already having a Rating of Hot - why do a DML when we don't need to ?
trigger opptoacc on opportunity (after insert, after update) {

    Set<Id> accId = new Set<Id>();
    List<Account> parentAccts     = new List<Account>();
    Map<Id,Account> accountMap     = new Map<Id,Account>();
    List<Account> accttoUpdate    = new List<Account>();

    if (Trigger.isInsert) {
        for(Opportunity opp: Trigger.New){
            accId.add(opp.accountId);
        }
        System.debug('DEBUG: Opp Trigger: accId size ' + accId.size());
        
        parentAccts = [SELECT Id,Name,Rating FROM Account WHERE Id IN :accId];
        System.debug('DEBUG: Opp Trigger: parentAccts size ' + parentAccts.size());
        
        for(Account acc: parentAccts){
            System.debug('DEBUG: Opp Trigger: acc Id ' + acc.Id);
            for(Opportunity op1:Trigger.New){
                System.debug('DEBUG: Opp Trigger: op1 Id ' + op1.Id);
                if(op1.AccountId == acc.Id && op1.Name == 'something' && acc.Rating <> 'Hot'){
                    System.debug('DEBUG: Opp Trigger: update Account ' + acc.Name);
                    acc.Rating = 'Hot';
                    accountMap.put(acc.Id,acc);
                }
            }
        }

        for (Account acc : accountMap.values()) {
            accttoUpdate.add(acc);
        }
        System.debug('DEBUG: Opp Trigger: # number accounts to update ' + accttoUpdate.size());

        update accttoUpdate;
    }    
}
So, we have a trigger that is now good to go with variations of the bulk.  So to the test class.  I note that you seem to want to use one method to test this trigger, yet the variations of how the trigger could be invoked are more than one.  I mentioned previously about a Negative test. A Negative test is one where the trigger does not execute or it performs something unexpected. So in this case, I want to check that it doesn't fire if the name on the Opportunity is not "something". 
Additionally, by retaining the single test I first suggested, you would have realised the single record invoked the loop, but not you "bulk test", hence pointing to the bulk test being incorrect.
SO..... here is your test class with 5 x methods - hopefully each method name is descriptive enough - alternate to the method name, the insertion of some comments about what the test is trying to achieve is an excellent idea.
 
@isTest
private class OpportunityTest {

	@isTest static void test_Single_record_positive() {
		Account acc = new Account(Name='Test Account', Industry = 'Industrial', Rating = 'Cold'
			//other values as required
			);
		Insert acc;

		Opportunity opp = new Opportunity(Name = 'something', AccountId = acc.Id, stageName='Prospecting',closeDate=System.today()+5
			//other values as required
			);
		Insert opp;

		List<Account> insertedAccounts = [SELECT Id, Rating FROM Account WHERE Id = :acc.Id];
		System.assertEquals(insertedAccounts[0].Rating,'Hot');

	}
	
	@isTest static void test_Single_record_negative() {
		// now, check that the rating is not changes if the criteria are not met
		Account acc = new Account(Name='Test Account', Industry = 'Industrial', Rating = 'Cold'
			//other values as required
			);
		Insert acc;

		Opportunity opp = new Opportunity(Name = 'nothing', AccountId = acc.Id, stageName='Prospecting',closeDate=System.today()+5
			//other values as required
			);
		Insert opp;

		List<Account> insertedAccounts = [SELECT Id, Rating FROM Account WHERE Id = :acc.Id];
		System.assertEquals(insertedAccounts[0].Rating,'Cold');

	}

	@isTest static void test_Single_Account_Multi_records_positive(){
		// test for single account with multiple oppty - to ensure we have resolved the same account in list DML error
		Integer numToCreate = 200;
		List<Account> acctlist=new List<Account>();
		List<Opportunity> opplist=new List<Opportunity>();
		Set<Id> acctSet=new Set<Id>();

		Account newAccount= new Account(Name='Test Account', Rating='Cold');
		insert newAccount;

		List<Account> insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Id = :newAccount.Id];
		System.assertEquals(insertedAccounts.size(),1);

		for(Integer i=0;i<numToCreate;i++){
			Opportunity op = new Opportunity(Name ='something',	AccountId= newAccount.Id, 
				stageName='Prospecting', closeDate=System.today()+5);
			opplist.add(op);
		}
		System.assertEquals(oppList.size(),numToCreate);
		insert opplist;
		
		insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account'];
        System.assertEquals(insertedAccounts.size(),1);
		for(Account a1: insertedAccounts){
			System.assertEquals(a1.Rating,'Hot');
		}

	}

	@isTest static void test_Multi_Account_with_Single_Oppty_positive(){
		// test for multiple accounts with single oppty - to ensure bulk updates are covered
		Integer numToCreate = 200;
		List<Account> acctlist=new List<Account>();
		List<Opportunity> opplist=new List<Opportunity>();
		Set<Id> acctSet=new Set<Id>();
		for(Integer i=0;i<numToCreate;i++){
			Account acc= new Account(Name='Test Account'+i, Rating='Cold');
			acctlist.add(acc);
		}
		insert acctlist;

		List<Account> insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account%'];
		System.assertEquals(insertedAccounts.size(),numToCreate);

		for(Integer i=0;i<numToCreate;i++){
			Opportunity op = new Opportunity(Name ='something',	AccountId= insertedAccounts[i].id, 
				stageName='Prospecting', closeDate=System.today()+5);
			opplist.add(op);
		}
		System.assertEquals(oppList.size(),numToCreate);
		insert opplist;
		
		insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account%'];
		for(Account a1: insertedAccounts){
			System.assertEquals(a1.Rating,'Hot');
		}

	}

	@isTest static void test_Multi_Account_with_Multi_Oppty_positive(){
		// test for multiple accounts with multi oppty each - to ensure bulk updates are covered
		Integer numToCreate = 20; //keep and round to tens values
		List<Account> acctlist=new List<Account>();
		List<Opportunity> opplist=new List<Opportunity>();
		Set<Id> acctSet=new Set<Id>();
		for(Integer i=0;i<numToCreate;i++){
			Account acc= new Account(Name='Test Account'+i, Rating='Cold');
			acctlist.add(acc);
		}
		insert acctlist;

		List<Account> insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account%'];
		System.assertEquals(insertedAccounts.size(),numToCreate);

		for (Account acc : insertedAccounts ) {
			for(Integer i=0;i<numToCreate/10;i++){
				Opportunity op = new Opportunity(Name ='something',	AccountId= acc.id, 
					stageName='Prospecting', closeDate=System.today()+5);
				opplist.add(op);
			}
		}
		System.assertEquals(oppList.size(),numToCreate*(numToCreate/10));
		insert opplist;
		
		insertedAccounts = [SELECT Id,Name,Rating FROM Account WHERE Name LIKE 'Test Account%'];
		for(Account a1: insertedAccounts){
			System.assertEquals(a1.Rating,'Hot');
		}

	}
}

And that is a test class of which to be proud.

Regards
Andrew


 
This was selected as the best answer