function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
Gayatri KarthikGayatri Karthik 

Hi I m new to sales force development ,I wrote Apex trigger for cross objects and finally trigger working ,now I should move to production ,I tried to write test class ,code coverage was zero ,Kindly help me

Trigger code

trigger Ag_PurchaseContract_UpdateTradeConfirmation_Trigger on Ag_Purchase_Contract__c (after insert,after update) {

    set<string> tradeConfirmationNumSet = new set<String>();
    string purchaseContractName = null;
    for (Ag_Purchase_Contract__c purchase : Trigger.new)    
    {
      list<Ag_Purchase_Contract__c> purlist = [select id, Trade_Confirmation_No__c,name from Ag_Purchase_Contract__c where Trade_Confirmation_No__c = :purchase.Trade_Confirmation_No__c and name != :purchase.name];
   
      if (purlist != null && purlist.size() > 0 && purchase.Trade_Confirmation_No__c != null)
       {
         purchase.adderror('The TradeNumber is used in a different Purchase contract. The purchase contract number is ' + purlist[0].name  );
       }
       else
       {
         purchaseContractName = purchase.name;
         if(purchase.Trade_Confirmation_No__c != null )
         {
            if (purchase.Trade_Confirmation_No__c == Trigger.NewMap.get(purchase.Id).Trade_Confirmation_No__c)
            {
               tradeConfirmationNumSet.add(Trigger.NewMap.get(purchase.Id).Trade_Confirmation_No__c);
              
            }
            
            List<Trade_Confirmation__c> tcList = [Select ID,purchase_contract__c,name from Trade_Confirmation__c where id IN :tradeConfirmationNumSet];
          
            for (Trade_Confirmation__c tc: TClist)
            {
                tc.purchase_contract__c = purchaseContractName;
            }
             update tcList;
        }
     }
  }
}

the above trigger is working and 
test class is 

@isTest
private class Test_Ag_PurchaseContract {
    @istest static void testSuccess(){
    set<string> tradeConfirmationNumSet = new set<String>();
        
    string purchaseContractName = 'AGP/19/03/16599';
     String tradeConfirmationnum  = 'AGC0000012';
    Ag_Purchase_Contract__c purchase = new  Ag_Purchase_Contract__c();
     

      list<Ag_Purchase_Contract__c> purlist = [select id, Trade_Confirmation_No__c,name from Ag_Purchase_Contract__c where Trade_Confirmation_No__c = :tradeConfirmationnum and name=:purchaseContractName limit 1];
   
      if (purlist != null && purlist.size() > 0)
       {
         purchase.adderror('The TradeNumber is used in a different Purchase contract. The purchase contract number is ' + purlist[0].name  );
       }
       else
       {
        if(purchase.Trade_Confirmation_No__c != null )
         {
           
               tradeConfirmationNumSet.add(tradeConfirmationnum);
         }
            
         List<Trade_Confirmation__c> tcList = [Select ID,purchase_contract__c,name from Trade_Confirmation__c where id IN :tradeConfirmationNumSet limit 1];
         system.debug('purchaseContractName '+  purchase.name);
         system.debug('purchaseContractName After assign '+  purchaseContractName);
         system.debug('TClist :'+tcList.size());
           
           if(TClist!=null && TClist.size()>0)
           {
                for (Trade_Confirmation__c tc: TClist)
         {
           tc.purchase_contract__c = purchaseContractName;
           system.debug('tc.purchase_contract__c '+ tc.purchase_contract__c);
         }
           }
            
        
         try
         {
             update tcList;
         }
         catch(Exception e)
         {
             system.assertEquals(e.getMessage(), e.getMessage()) ;
         } 
       }
  }
}
With this test class Code coverage is zero.how to write pls help me , 

 
Best Answer chosen by Gayatri Karthik
Andrew GAndrew G
Hi Gayatri
Apologies - busy day at work as I wrap up this contract.
@apuroop
that looks much better - i wil cast an eye over it shortly.

Here is my version of a solution -
first a trigger Handler - I like handlers as the make the trigger easy to read, and allow full control over how things are executed.  My handler is also quite structured. 
public with sharing class PurchaseContractTriggerHandler {
	//Created by Andrew G - 02/07/2019
	//Purpose - 
	//Notes - 

	public class MyException extends Exception {}

	public void handleBeforeInsert(List<Ag_Purchase_Contract__c> newPurchaseContracts){
		System.debug('DEBUG: handle Before Insert') ;
		checkTradeNumber(newPurchaseContracts);
	}

	public void handleAfterInsert(List<Ag_Purchase_Contract__c> newPurchaseContracts){
		System.debug('DEBUG: handle After Insert') ;
		writePurchaseName(newPurchaseContracts);
	}

	public void handleBeforeUpdate(List<Ag_Purchase_Contract__c> oldPurchaseContracts, List<Ag_Purchase_Contract__c> updatedPurchaseContracts, Map<Id,Ag_Purchase_Contract__c> oldValues, Map<Id,Ag_Purchase_Contract__c> newValues){
		System.debug('DEBUG: handle Before Update') ;
		checkTradeNumber(updatedPurchaseContracts);
	}

	public void handleAfterUpdate(List<Ag_Purchase_Contract__c> oldPurchaseContracts, List<Ag_Purchase_Contract__c> updatedPurchaseContracts, Map<Id,Ag_Purchase_Contract__c> oldValues, Map<Id,Ag_Purchase_Contract__c> newValues) {
		System.debug('DEBUG: handle After Update') ;
		writePurchaseName(updatedPurchaseContracts);
	}

	public void checkTradeNumber(List<Ag_Purchase_Contract__c> newPurchaseContracts) {
		List<Id> tcIds = new List<Id>();
		list<Ag_Purchase_Contract__c> purlist = new List<Ag_Purchase_Contract__c>();

		for (Ag_Purchase_Contract__c pc : newPurchaseContracts ) {
			tcIds.add(pc.Trade_Confirmation_No__c);
		}

		purlist = [SELECT id, Trade_Confirmation_No__c, Name  
					FROM Ag_Purchase_Contract__c 
					WHERE Trade_Confirmation_No__c in :tcIds];

		if(purlist.size() > 0 ) {
			if (Trigger.isInsert )  {
				System.debug('DEBUG - new insert and found records - TC is used');
				Trigger.new[0].addError(new MyException('The TradeNumber is used in a different Purchase contract.'));
			}
			if (Trigger.isUpdate && purlist.size() > 1 ) {
				System.debug('DEBUG - update and more than one - TC is used elsewhere');
				Trigger.new[0].addError(new MyException('The TradeNumber is used in a different Purchase contract.'));
			} else if (purlist.size() == 1 && (purlist[0].Name <> newPurchaseContracts[0].Name)) {
				System.debug('DEBUG - update and other records found - TC is used elsewhere');
				Trigger.new[0].addError(new MyException('The TradeNumber is used in a different Purchase contract.'));
			}
		}
	}

	public void writePurchaseName(List<Ag_Purchase_Contract__c> newPurchaseContracts) {
		List<Id> tcIds = new List<Id>();
		List<Trade_Confirmation__c> toUpdate = new List<Trade_Confirmation__c>();

		for (Ag_Purchase_Contract__c pc : newPurchaseContracts ) {
			tcIds.add(pc.Trade_Confirmation_No__c);
		}

		Map<Id, Trade_Confirmation__c> mapTradeConfirmations = new Map<Id,Trade_Confirmation__c>([SELECT Id, purchase_contract__c 
								FROM Trade_Confirmation__c
								WHERE Id IN :tcIds ]);

		for (Ag_Purchase_Contract__c pc : newPurchaseContracts ) {
			if (mapTradeConfirmations.containsKey(pc.Trade_Confirmation_No__c)) {
				Trade_Confirmation__c tempTC = mapTradeConfirmations.get(pc.Trade_Confirmation_No__c);
				tempTc.purchase_contract__c = pc.Name;
				toUpdate.add(tempTc);
			}
		}
		if (!toUpdate.isEmpty()) {
			update toUpdate;
		}
	}

}
Note that we have no SELECT statements in FOR loops.  And we have methods that clearly perform one action (or a limited number of actions ) .  Whilst it makes the Handler longer, it provides clarity around what is happening where.  In the Handler above, we have two clear methods - one to ensure we don't duplicate the TradeConfirmation and one to update the TradeConfirmation.  The other reason to split them is that a validation method will occur in the Before Trigger and updating of other records will happen in the After Trigger.

The heavy lifting is done in the methods.  By having the handler call a method which calls another method it allows clarity of what is happening in the Handler:  Example only (not part of solution)  Where it makes sense to group actions in a method, then do so.  
public void handleBeforeInsert(List<Ag_Purchase_Contract__c> newPurchaseContracts){
		System.debug('DEBUG: handle Before Insert') ;
		checkTradeNumber(newPurchaseContracts);
        validatePurchase(newPurchaseContracts);
        checkAccounts(newPurchaseContracts);
        checkContactNumbers(newPurchaseContracts);
  // and so on.- example only           
	}
And by having the methods doing the work, we write the logic in one place but use in in multipe areas - example above , we call checkTradeNumber  from both the before insert and before update.

Next we have the simple trigger structure which calls the Handler.  This is fairly standard trigger structure for me (except here I have not included the delete call) 
trigger PurchaseContractTrigger on Ag_Purchase_Contract__c ( before insert, before update, after insert, after update ) {
	System.debug('DEBUG - In Trigger - PurchaseContractTrigger');

	PurchaseContractTriggerHandler objHandler = new PurchaseContractTriggerHandler();

	if ( Trigger.isBefore && Trigger.isInsert ) {
		System.debug('In Trigger - about to call on before insert');         
		objHandler.handleBeforeInsert(Trigger.New);
	} 

	if ( Trigger.isBefore && trigger.isUpdate ) {
		System.debug('In Trigger - about to call on before update');         
		objHandler.handleBeforeUpdate(Trigger.Old, Trigger.New, Trigger.OldMap, Trigger.NewMap);
	} 

	if ( Trigger.isAfter && Trigger.isInsert ) {
		System.debug('In Trigger - about to call on after insert');
		objHandler.handleAfterInsert(Trigger.New);
	} 

	if ( Trigger.isAfter && Trigger.isUpdate ) {
		System.debug('In Trigger - about to call on after update');
		objHandler.handleAfterUpdate(Trigger.Old, Trigger.New, Trigger.OldMap, Trigger.NewMap);
	}

}

Yes, we can nest the IF statements like below: - personal preference - i use either 
if ( trigger.isBefore ) {
        if ( trigger.isInsert ) {
//call before insert method
        } else if ( trigger.isUpdate ) {
//call before update method
        } 
        else if (trigger.isDelete ) {
      //      call before delete method
    } else {  //trigger is after
        if ( trigger.isInsert ) {
//call after insert method
        } else if (trigger.isUpdate)  {
//call after update method
        }
    }

Now for the test class.
Test class should test all the methods that are called or expected to action.
So we test:
1. Successful insert
2. Successful update
3. Unsuccessful insert
4. Unsuccessful update

I run a separate test method for each item that I want to test.  In that way, if a test fails, I get a test method name which helps narrow the issue - if the other tests are good, and only one test fails, its just one test to fix / investigate.
  
@isTest
private class PurchaseContractTriggerHandlerTest {
	

	@testSetup static void setup() {
		List<Trade_Confirmation__c> testTradeCons = new List<Trade_Confirmation__c>();
		//Create multiple TC -Trade_Confirmation_No__c
		for (Integer i = 0; i <200; i++) {
			Trade_Confirmation__c tradeCon = new Trade_Confirmation__c();
			testTradeCons.add(tradeCon);
		}
		insert testTradeCons;
	}

	@isTest static void test_PurchaseContract_insert() {
		// Implement test code
		List<Trade_Confirmation__c> tradeCons = new List<Trade_Confirmation__c>();
		tradeCons = [SELECT Id, Name FROM Trade_Confirmation__c ORDER BY Name];

		Ag_Purchase_Contract__c newPC = new Ag_Purchase_Contract__c();
		newPC.Trade_Confirmation_No__c = tradeCons[0].Id;
		Insert newPC;

		List<Ag_Purchase_Contract__c> insertedPCs = new List<Ag_Purchase_Contract__c>();
		insertedPCs = [SELECT Id, Name FROM Ag_Purchase_Contract__c WHERE Id = :newPC.Id];
		System.assertEquals(1,insertedPCs.size());  //sanity check 

		List<Trade_Confirmation__c> insertedTCs = new List<Trade_Confirmation__c>();
		insertedTCs = [SELECT Id,Name, purchase_contract__c FROM Trade_Confirmation__c WHERE Id = :tradeCons[0].Id ];
		System.assertEquals(1,insertedTCs.size());  //sanity check 

		System.assertEquals( insertedPCs[0].Name, insertedTCs[0].purchase_contract__c );
	}
	
	@isTest static void test_PurchaseContract_update_newTC() {
		// Implement test code
		List<Trade_Confirmation__c> tradeCons = new List<Trade_Confirmation__c>();
		tradeCons = [SELECT Id, Name FROM Trade_Confirmation__c ORDER BY Name];

		Ag_Purchase_Contract__c newPC = new Ag_Purchase_Contract__c();
		newPC.Trade_Confirmation_No__c = tradeCons[0].Id;
		Insert newPC;

		//now change the record we inserted
		List<Ag_Purchase_Contract__c> insertedPCs = new List<Ag_Purchase_Contract__c>();
		insertedPCs = [SELECT Id, Name, Trade_Confirmation_No__c FROM Ag_Purchase_Contract__c WHERE Id = :newPC.Id];
		System.assertEquals(1,insertedPCs.size());  //sanity check 

		insertedPCs[0].Trade_Confirmation_No__c = tradeCons[1].Id;
		update insertedPCs;

		//now test the changed record
		insertedPCs = [SELECT Id, Name, Trade_Confirmation_No__c FROM Ag_Purchase_Contract__c WHERE Id = :newPC.Id];
		System.assertEquals(1,insertedPCs.size());  //sanity check 

		List<Trade_Confirmation__c> insertedTCs = new List<Trade_Confirmation__c>();
		insertedTCs = [SELECT Id,Name, purchase_contract__c FROM Trade_Confirmation__c WHERE Id = :tradeCons[1].Id ];
		System.assertEquals(1,insertedTCs.size());  //sanity check 

		System.assertEquals( insertedPCs[0].Name, insertedTCs[0].purchase_contract__c );
	}

	@isTest static void test_PurchaseContract_insert_duplicateTC() {
		// Implement test code
		List<Trade_Confirmation__c> tradeCons = new List<Trade_Confirmation__c>();
		tradeCons = [SELECT Id, Name FROM Trade_Confirmation__c ORDER BY Name];

		Ag_Purchase_Contract__c newPC = new Ag_Purchase_Contract__c();
		newPC.Trade_Confirmation_No__c = tradeCons[0].Id;
		Insert newPC;

		Ag_Purchase_Contract__c newPC2 = new Ag_Purchase_Contract__c();
		newPC2.Trade_Confirmation_No__c = tradeCons[0].Id;
		try {
			Insert newPC2;
		} catch (Exception ex) {
			Boolean expectedExceptionThrown = ex.getMessage().contains('The TradeNumber is used in a different Purchase contract.') ? true : false;
			System.assertEquals(true, expectedExceptionThrown);
		}

	}

	@isTest static void test_PurchaseContract_update_duplicateTC() {
		// Implement test code
		List<Trade_Confirmation__c> tradeCons = new List<Trade_Confirmation__c>();
		tradeCons = [SELECT Id, Name FROM Trade_Confirmation__c ORDER BY Name];

		Ag_Purchase_Contract__c newPC = new Ag_Purchase_Contract__c();
		newPC.Trade_Confirmation_No__c = tradeCons[0].Id;
		Insert newPC;

		Ag_Purchase_Contract__c newPC2 = new Ag_Purchase_Contract__c();
		newPC2.Trade_Confirmation_No__c = tradeCons[1].Id;
		Insert newPC2;

		//now change the second record we inserted
		List<Ag_Purchase_Contract__c> insertedPCs = new List<Ag_Purchase_Contract__c>();
		insertedPCs = [SELECT Id, Name, Trade_Confirmation_No__c FROM Ag_Purchase_Contract__c WHERE Id = :newPC2.Id];
		System.assertEquals(1,insertedPCs.size());  //sanity check 

		insertedPCs[0].Trade_Confirmation_No__c = tradeCons[0].Id;

		try {
			update insertedPCs;
		} catch (Exception ex) {
			Boolean expectedExceptionThrown = ex.getMessage().contains('The TradeNumber is used in a different Purchase contract.') ? true : false;
			System.assertEquals(true, expectedExceptionThrown);
		}

	}
}

I hope the above proves beneficial.


Regards
Andrew​​​​​​​
 

All Answers

ApuroopApuroop
Hi Gayatri,

You are missing an insert statement in your test class. You did a great job being totally new to Apex.

Try this: 
@isTest
private class Test_Ag_PurchaseContract {
    @istest static void testSuccess(){
        set<string> tradeConfirmationNumSet = new set<String>();
        
        string purchaseContractName = 'AGP/19/03/16599';
        String tradeConfirmationnum  = 'AGC0000012';
        Ag_Purchase_Contract__c purchase = new  Ag_Purchase_Contract__c(Trade_Confirmation_No__c = tradeConfirmationnum, Name=purchaseContractName);
        insert purchase;
        
        list<Ag_Purchase_Contract__c> purlist = [select id, Trade_Confirmation_No__c,name from Ag_Purchase_Contract__c where Trade_Confirmation_No__c = :tradeConfirmationnum and name=:purchaseContractName limit 1];
        
        if (purlist != null && purlist.size() > 0)
        {
            purchase.adderror('The TradeNumber is used in a different Purchase contract. The purchase contract number is ' + purlist[0].name  );
        }
        else
        {
            if(purchase.Trade_Confirmation_No__c != null )
            {
                
                tradeConfirmationNumSet.add(tradeConfirmationnum);
            }
            
            List<Trade_Confirmation__c> tcList = [Select ID,purchase_contract__c,name from Trade_Confirmation__c where id IN :tradeConfirmationNumSet limit 1];
            system.debug('purchaseContractName '+  purchase.name);
            system.debug('purchaseContractName After assign '+  purchaseContractName);
            system.debug('TClist :'+tcList.size());
            
            if(TClist!=null && TClist.size()>0)
            {
                for (Trade_Confirmation__c tc: TClist)
                {
                    tc.purchase_contract__c = purchaseContractName;
                    system.debug('tc.purchase_contract__c '+ tc.purchase_contract__c);
                }
            }
            
            
            try
            {
                update tcList;
            }
            catch(Exception e)
            {
                system.assertEquals(e.getMessage(), e.getMessage()) ;
            } 
        }
    }
}

 
Andrew GAndrew G
@apuroop
OMG ... as a test class, just Oh my goodness
A test class should not simply repeat the class it is testing, it should be testing that the test class performs the expected action / outcome.
And then to assert with  system.assertEquals(e.getMessage(), e.getMessage())

But before the test class, why don't we actually point out the SELECT statement inside a FOR loop.  

@Gayatri
give me a little time and I will do a proper solution with explanation.

Regards
Andrew
 
ApuroopApuroop
@Andrew
Of course that's the way to test. I was just pointing out why the test class isn't covering at all. I guess then the trigger needs to be modified too, there's a DML statement inside a FOR loop.
Andrew GAndrew G
@gagyatri

I'm trying to read your code to determine your data structure and your intent of the trigger.

Structure
Object:  
    Ag_Purchase_Contract__c
Has fields: 
    Name - text field
    Trade_Confirmation_No__c - lookup field to object Trade_Confirmation__c

Object:
    Trade_Confirmation__c
Has fields:
    purchase_contract__c - text field.

Trigger intent:
   1.  A Trade Confirmation Number can only be used in one Purchase Contract.
   2.  If the Trade Confirmation Number is not used elsewhere, update the Trade Confirmation record with the PurchaseContractName (Ag_Purchase_Contract__c.Name)
  
   Can you confirm that the above is correct?

Regards
Andrew
Gayatri KarthikGayatri Karthik
Appreciate your response  Apuroop Naidu .but with above code also code coverage 0 percent. Is there any to write test class?
Gayatri KarthikGayatri Karthik
Andrew G


Thank you below information with few modifications

I'm trying to read your code to determine your data structure and your intent of the trigger.

Structure
Object:  
    Ag_Purchase_Contract__c
Has fields: 
    Name - text field <Auto generation number>
    Trade_Confirmation_No__c - lookup field to object Trade_Confirmation__c

Object:
    Trade_Confirmation__c
Has fields:
    purchase_contract__c - text field.

Trigger intent:
   1.  A Trade Confirmation Number can only be used in one Purchase Contract. < Yes, correct>
   2.  If the Trade Confirmation Number is not used elsewhere, update the Trade Confirmation record with the PurchaseContractName (Ag_Purchase_Contract__c.Name)
       <If the trade confirmation number is selected in purchase contract object, update the trade confirmation record with the purchasecontractname>

  

Regards
Gayatri

 
ApuroopApuroop
Shoot in the dark.. Is the trigger active?
Gayatri KarthikGayatri Karthik
@Apuroop Naidu
Shoot in the dark.. Is the trigger active? Yes 

 
Gayatri KarthikGayatri Karthik
Hi Andrew , I am waiting for your modified trigger and test code from you . Please let me know where I did mistake . Thank you . Regards, Gayatri
ApuroopApuroop
Hey Gayatri. I tried my best to do the testing and the following code works fine in my dev org. You might want to add your test cases and do some production level testing. Good luck!

Also, like Andrew suggested, you might want to add some more assert statements. I did my best! :)

TRIGGER: 
trigger Ag_PurchaseContract_UpdateTradeConfirmation_Trigger on Ag_Purchase_Contract__c (before insert, before update) {
    List<Id> tradeConfs = new List<Id>();
    Map<Id, String> tradeList = new Map<Id, String>();
    List<Trade_Confirmation__c> toUpdate = new List<Trade_Confirmation__c>();
    
    //Add the new tradeConfs to this list
    for(Ag_Purchase_Contract__c purchase : Trigger.New){
        tradeConfs.add(purchase.Trade_Confirmation_No__c);
    }
    
    //See if it already exists
    List<Ag_Purchase_Contract__c> purchaseList = [SELECT Id, Name, Trade_Confirmation_No__r.Name FROM Ag_Purchase_Contract__c WHERE Trade_Confirmation_No__r.Id =:tradeConfs];
    Map<Id, String> purchaseMap = new Map<Id, String>();
    for(Ag_Purchase_Contract__c apc : purchaseList){
        purchaseMap.put(apc.Trade_Confirmation_No__r.Id, apc.Name);
    }
    for(Ag_Purchase_Contract__c purchase2 : Trigger.new){
        if(purchase2.Trade_Confirmation_No__c != null){
            if(Trigger.isInsert){
                if(purchaseList.size() > 0 ){
                    purchase2.adderror('The TradeNumber is used in a different Purchase contract. The purchase contract number is ' + purchaseMap.get(purchase2.Trade_Confirmation_No__c)  );
                } else{
                    tradeList.put(purchase2.Trade_Confirmation_No__c, purchase2.Name);
                }
            } else if(Trigger.isUpdate){
                for(Ag_Purchase_Contract__c old_purchase2 : Trigger.old){
                    if((old_purchase2.Trade_Confirmation_No__c != purchase2.Trade_Confirmation_No__c) && purchaseList.size() > 0){
                        purchase2.adderror('The TradeNumber is used in a different Purchase contract. The purchase contract number is ' + purchaseMap.get(purchase2.Trade_Confirmation_No__c)  );
                    }else{
                        tradeList.put(purchase2.Trade_Confirmation_No__c, purchase2.Name);
                    }
                }
            }
        } else{
            continue;
        }
    }
    List<Trade_Confirmation__c> finalList = [SELECT Id, Purchase_Contract__c FROM Trade_Confirmation__c WHERE Id =:tradeList.keySet()];
    for(Trade_Confirmation__c tcfn : finalList){
        tcfn.Purchase_Contract__c = tradeList.get(tcfn.id);
        toUpdate.add(tcfn);
    }
    update toUpdate;
}

TEST CLASS: 
@isTest
public class Test_Ag_PurchaseContract {
    public static testMethod void testSuccess(){
        Set<String> tradeConfirmationNumSet = new set<String>();
        
        string purchaseContractName = 'AGP/19/03/16599';
        Trade_Confirmation__c tradeConfirmationnum = new Trade_Confirmation__c(Name = 'AGC0000012', Purchase_Contract__c = purchaseContractName);
        insert tradeConfirmationNum;
        
        System.assertNotEquals(null, tradeConfirmationnum.id);
        System.assertEquals('AGC0000012', tradeConfirmationnum.Name);
        System.assertEquals('AGP/19/03/16599', tradeConfirmationnum.Purchase_Contract__c);
        
        Ag_Purchase_Contract__c purchase = new  Ag_Purchase_Contract__c(Name=purchaseContractName, Trade_Confirmation_No__c = tradeConfirmationnum.id);
        insert purchase;
        
        System.assertNotEquals(null, purchase.id);
        System.assertEquals(purchaseContractName, purchase.Name);
        
        Ag_Purchase_Contract__c purchase2 = new  Ag_Purchase_Contract__c(Trade_Confirmation_No__c = tradeConfirmationnum.id, Name='AGP/19/03/16600');
        
        try{
            insert purchase2;            
        }catch(Exception e){
            System.debug('Exception message: '+e);
        }
        
        Trade_Confirmation__c tradeConfirmationnum2 = new Trade_Confirmation__c(Name = 'AGC000001234', Purchase_Contract__c = 'purchaseContractName');
        insert tradeConfirmationNum2;
        
        System.assertNotEquals(null, tradeConfirmationnum2.id);
        System.assertEquals('AGC000001234', tradeConfirmationnum2.Name);
        System.assertEquals('purchaseContractName', tradeConfirmationnum2.Purchase_Contract__c);
        
        Ag_Purchase_Contract__c purchase3 = new  Ag_Purchase_Contract__c(Name=purchaseContractName);
        insert purchase3;
        
        System.assertNotEquals(null, purchase3.id);
        System.assertEquals(purchaseContractName, purchase3.Name);
        
        purchase.Trade_Confirmation_No__r = tradeConfirmationNum2;
        update purchase;
        
        purchase3.Trade_Confirmation_No__c = tradeConfirmationNum.id;
        update purchase3;
    }
}
Andrew GAndrew G
Hi Gayatri
Apologies - busy day at work as I wrap up this contract.
@apuroop
that looks much better - i wil cast an eye over it shortly.

Here is my version of a solution -
first a trigger Handler - I like handlers as the make the trigger easy to read, and allow full control over how things are executed.  My handler is also quite structured. 
public with sharing class PurchaseContractTriggerHandler {
	//Created by Andrew G - 02/07/2019
	//Purpose - 
	//Notes - 

	public class MyException extends Exception {}

	public void handleBeforeInsert(List<Ag_Purchase_Contract__c> newPurchaseContracts){
		System.debug('DEBUG: handle Before Insert') ;
		checkTradeNumber(newPurchaseContracts);
	}

	public void handleAfterInsert(List<Ag_Purchase_Contract__c> newPurchaseContracts){
		System.debug('DEBUG: handle After Insert') ;
		writePurchaseName(newPurchaseContracts);
	}

	public void handleBeforeUpdate(List<Ag_Purchase_Contract__c> oldPurchaseContracts, List<Ag_Purchase_Contract__c> updatedPurchaseContracts, Map<Id,Ag_Purchase_Contract__c> oldValues, Map<Id,Ag_Purchase_Contract__c> newValues){
		System.debug('DEBUG: handle Before Update') ;
		checkTradeNumber(updatedPurchaseContracts);
	}

	public void handleAfterUpdate(List<Ag_Purchase_Contract__c> oldPurchaseContracts, List<Ag_Purchase_Contract__c> updatedPurchaseContracts, Map<Id,Ag_Purchase_Contract__c> oldValues, Map<Id,Ag_Purchase_Contract__c> newValues) {
		System.debug('DEBUG: handle After Update') ;
		writePurchaseName(updatedPurchaseContracts);
	}

	public void checkTradeNumber(List<Ag_Purchase_Contract__c> newPurchaseContracts) {
		List<Id> tcIds = new List<Id>();
		list<Ag_Purchase_Contract__c> purlist = new List<Ag_Purchase_Contract__c>();

		for (Ag_Purchase_Contract__c pc : newPurchaseContracts ) {
			tcIds.add(pc.Trade_Confirmation_No__c);
		}

		purlist = [SELECT id, Trade_Confirmation_No__c, Name  
					FROM Ag_Purchase_Contract__c 
					WHERE Trade_Confirmation_No__c in :tcIds];

		if(purlist.size() > 0 ) {
			if (Trigger.isInsert )  {
				System.debug('DEBUG - new insert and found records - TC is used');
				Trigger.new[0].addError(new MyException('The TradeNumber is used in a different Purchase contract.'));
			}
			if (Trigger.isUpdate && purlist.size() > 1 ) {
				System.debug('DEBUG - update and more than one - TC is used elsewhere');
				Trigger.new[0].addError(new MyException('The TradeNumber is used in a different Purchase contract.'));
			} else if (purlist.size() == 1 && (purlist[0].Name <> newPurchaseContracts[0].Name)) {
				System.debug('DEBUG - update and other records found - TC is used elsewhere');
				Trigger.new[0].addError(new MyException('The TradeNumber is used in a different Purchase contract.'));
			}
		}
	}

	public void writePurchaseName(List<Ag_Purchase_Contract__c> newPurchaseContracts) {
		List<Id> tcIds = new List<Id>();
		List<Trade_Confirmation__c> toUpdate = new List<Trade_Confirmation__c>();

		for (Ag_Purchase_Contract__c pc : newPurchaseContracts ) {
			tcIds.add(pc.Trade_Confirmation_No__c);
		}

		Map<Id, Trade_Confirmation__c> mapTradeConfirmations = new Map<Id,Trade_Confirmation__c>([SELECT Id, purchase_contract__c 
								FROM Trade_Confirmation__c
								WHERE Id IN :tcIds ]);

		for (Ag_Purchase_Contract__c pc : newPurchaseContracts ) {
			if (mapTradeConfirmations.containsKey(pc.Trade_Confirmation_No__c)) {
				Trade_Confirmation__c tempTC = mapTradeConfirmations.get(pc.Trade_Confirmation_No__c);
				tempTc.purchase_contract__c = pc.Name;
				toUpdate.add(tempTc);
			}
		}
		if (!toUpdate.isEmpty()) {
			update toUpdate;
		}
	}

}
Note that we have no SELECT statements in FOR loops.  And we have methods that clearly perform one action (or a limited number of actions ) .  Whilst it makes the Handler longer, it provides clarity around what is happening where.  In the Handler above, we have two clear methods - one to ensure we don't duplicate the TradeConfirmation and one to update the TradeConfirmation.  The other reason to split them is that a validation method will occur in the Before Trigger and updating of other records will happen in the After Trigger.

The heavy lifting is done in the methods.  By having the handler call a method which calls another method it allows clarity of what is happening in the Handler:  Example only (not part of solution)  Where it makes sense to group actions in a method, then do so.  
public void handleBeforeInsert(List<Ag_Purchase_Contract__c> newPurchaseContracts){
		System.debug('DEBUG: handle Before Insert') ;
		checkTradeNumber(newPurchaseContracts);
        validatePurchase(newPurchaseContracts);
        checkAccounts(newPurchaseContracts);
        checkContactNumbers(newPurchaseContracts);
  // and so on.- example only           
	}
And by having the methods doing the work, we write the logic in one place but use in in multipe areas - example above , we call checkTradeNumber  from both the before insert and before update.

Next we have the simple trigger structure which calls the Handler.  This is fairly standard trigger structure for me (except here I have not included the delete call) 
trigger PurchaseContractTrigger on Ag_Purchase_Contract__c ( before insert, before update, after insert, after update ) {
	System.debug('DEBUG - In Trigger - PurchaseContractTrigger');

	PurchaseContractTriggerHandler objHandler = new PurchaseContractTriggerHandler();

	if ( Trigger.isBefore && Trigger.isInsert ) {
		System.debug('In Trigger - about to call on before insert');         
		objHandler.handleBeforeInsert(Trigger.New);
	} 

	if ( Trigger.isBefore && trigger.isUpdate ) {
		System.debug('In Trigger - about to call on before update');         
		objHandler.handleBeforeUpdate(Trigger.Old, Trigger.New, Trigger.OldMap, Trigger.NewMap);
	} 

	if ( Trigger.isAfter && Trigger.isInsert ) {
		System.debug('In Trigger - about to call on after insert');
		objHandler.handleAfterInsert(Trigger.New);
	} 

	if ( Trigger.isAfter && Trigger.isUpdate ) {
		System.debug('In Trigger - about to call on after update');
		objHandler.handleAfterUpdate(Trigger.Old, Trigger.New, Trigger.OldMap, Trigger.NewMap);
	}

}

Yes, we can nest the IF statements like below: - personal preference - i use either 
if ( trigger.isBefore ) {
        if ( trigger.isInsert ) {
//call before insert method
        } else if ( trigger.isUpdate ) {
//call before update method
        } 
        else if (trigger.isDelete ) {
      //      call before delete method
    } else {  //trigger is after
        if ( trigger.isInsert ) {
//call after insert method
        } else if (trigger.isUpdate)  {
//call after update method
        }
    }

Now for the test class.
Test class should test all the methods that are called or expected to action.
So we test:
1. Successful insert
2. Successful update
3. Unsuccessful insert
4. Unsuccessful update

I run a separate test method for each item that I want to test.  In that way, if a test fails, I get a test method name which helps narrow the issue - if the other tests are good, and only one test fails, its just one test to fix / investigate.
  
@isTest
private class PurchaseContractTriggerHandlerTest {
	

	@testSetup static void setup() {
		List<Trade_Confirmation__c> testTradeCons = new List<Trade_Confirmation__c>();
		//Create multiple TC -Trade_Confirmation_No__c
		for (Integer i = 0; i <200; i++) {
			Trade_Confirmation__c tradeCon = new Trade_Confirmation__c();
			testTradeCons.add(tradeCon);
		}
		insert testTradeCons;
	}

	@isTest static void test_PurchaseContract_insert() {
		// Implement test code
		List<Trade_Confirmation__c> tradeCons = new List<Trade_Confirmation__c>();
		tradeCons = [SELECT Id, Name FROM Trade_Confirmation__c ORDER BY Name];

		Ag_Purchase_Contract__c newPC = new Ag_Purchase_Contract__c();
		newPC.Trade_Confirmation_No__c = tradeCons[0].Id;
		Insert newPC;

		List<Ag_Purchase_Contract__c> insertedPCs = new List<Ag_Purchase_Contract__c>();
		insertedPCs = [SELECT Id, Name FROM Ag_Purchase_Contract__c WHERE Id = :newPC.Id];
		System.assertEquals(1,insertedPCs.size());  //sanity check 

		List<Trade_Confirmation__c> insertedTCs = new List<Trade_Confirmation__c>();
		insertedTCs = [SELECT Id,Name, purchase_contract__c FROM Trade_Confirmation__c WHERE Id = :tradeCons[0].Id ];
		System.assertEquals(1,insertedTCs.size());  //sanity check 

		System.assertEquals( insertedPCs[0].Name, insertedTCs[0].purchase_contract__c );
	}
	
	@isTest static void test_PurchaseContract_update_newTC() {
		// Implement test code
		List<Trade_Confirmation__c> tradeCons = new List<Trade_Confirmation__c>();
		tradeCons = [SELECT Id, Name FROM Trade_Confirmation__c ORDER BY Name];

		Ag_Purchase_Contract__c newPC = new Ag_Purchase_Contract__c();
		newPC.Trade_Confirmation_No__c = tradeCons[0].Id;
		Insert newPC;

		//now change the record we inserted
		List<Ag_Purchase_Contract__c> insertedPCs = new List<Ag_Purchase_Contract__c>();
		insertedPCs = [SELECT Id, Name, Trade_Confirmation_No__c FROM Ag_Purchase_Contract__c WHERE Id = :newPC.Id];
		System.assertEquals(1,insertedPCs.size());  //sanity check 

		insertedPCs[0].Trade_Confirmation_No__c = tradeCons[1].Id;
		update insertedPCs;

		//now test the changed record
		insertedPCs = [SELECT Id, Name, Trade_Confirmation_No__c FROM Ag_Purchase_Contract__c WHERE Id = :newPC.Id];
		System.assertEquals(1,insertedPCs.size());  //sanity check 

		List<Trade_Confirmation__c> insertedTCs = new List<Trade_Confirmation__c>();
		insertedTCs = [SELECT Id,Name, purchase_contract__c FROM Trade_Confirmation__c WHERE Id = :tradeCons[1].Id ];
		System.assertEquals(1,insertedTCs.size());  //sanity check 

		System.assertEquals( insertedPCs[0].Name, insertedTCs[0].purchase_contract__c );
	}

	@isTest static void test_PurchaseContract_insert_duplicateTC() {
		// Implement test code
		List<Trade_Confirmation__c> tradeCons = new List<Trade_Confirmation__c>();
		tradeCons = [SELECT Id, Name FROM Trade_Confirmation__c ORDER BY Name];

		Ag_Purchase_Contract__c newPC = new Ag_Purchase_Contract__c();
		newPC.Trade_Confirmation_No__c = tradeCons[0].Id;
		Insert newPC;

		Ag_Purchase_Contract__c newPC2 = new Ag_Purchase_Contract__c();
		newPC2.Trade_Confirmation_No__c = tradeCons[0].Id;
		try {
			Insert newPC2;
		} catch (Exception ex) {
			Boolean expectedExceptionThrown = ex.getMessage().contains('The TradeNumber is used in a different Purchase contract.') ? true : false;
			System.assertEquals(true, expectedExceptionThrown);
		}

	}

	@isTest static void test_PurchaseContract_update_duplicateTC() {
		// Implement test code
		List<Trade_Confirmation__c> tradeCons = new List<Trade_Confirmation__c>();
		tradeCons = [SELECT Id, Name FROM Trade_Confirmation__c ORDER BY Name];

		Ag_Purchase_Contract__c newPC = new Ag_Purchase_Contract__c();
		newPC.Trade_Confirmation_No__c = tradeCons[0].Id;
		Insert newPC;

		Ag_Purchase_Contract__c newPC2 = new Ag_Purchase_Contract__c();
		newPC2.Trade_Confirmation_No__c = tradeCons[1].Id;
		Insert newPC2;

		//now change the second record we inserted
		List<Ag_Purchase_Contract__c> insertedPCs = new List<Ag_Purchase_Contract__c>();
		insertedPCs = [SELECT Id, Name, Trade_Confirmation_No__c FROM Ag_Purchase_Contract__c WHERE Id = :newPC2.Id];
		System.assertEquals(1,insertedPCs.size());  //sanity check 

		insertedPCs[0].Trade_Confirmation_No__c = tradeCons[0].Id;

		try {
			update insertedPCs;
		} catch (Exception ex) {
			Boolean expectedExceptionThrown = ex.getMessage().contains('The TradeNumber is used in a different Purchase contract.') ? true : false;
			System.assertEquals(true, expectedExceptionThrown);
		}

	}
}

I hope the above proves beneficial.


Regards
Andrew​​​​​​​
 
This was selected as the best answer
Andrew GAndrew G
@Apuroop

Your trigger is better, but as you read it, you can see that you are duplicating code.  Your if statment for trigger.isNew / trigger.isUpdate.  if you created a method, you would only need to write the code once, but call the method twice.

Your test method is better.  However, I steer away from constants in my test classes. 

he other thing to consider is that when you insert a record, the values that are being updated by the trigger will not exist as you insert.  So you would need to query the records again, including the field to be test and then run your assert on the returned field.
Asserts like:
System.assertEquals('AGC0000012', tradeConfirmationnum.Name); 
wont actually have a lot of value as you haven't really confirmed that the record has been written to the server. If you want to be precise, you are simply testing that you could write to that field.  If there was some validation around the field, then there could be value, but not for this test case. 

It is better to insert / update the record and then requery the record including the field and then doing the assert.

And you should also do both positive and negative tests.  In the above, yes we were succesful in inserting and then we also test where there was a conflict,  and as we are throwing an exception, try and catch the exception and assert that the exception was expected. 


Hope that is helpful

Regards
Andrew
 
ApuroopApuroop
WOW!! That's a beautiful answer and review on my code. I actually follow this Handler-Trigger style when am writing something of my own. 

The thing that you told about querying a record after insert for testing - I agree what you're saying but in my test class, lines like 10, 11, 12, 17, 18 work fine which means they're returning what we're expecting, correct? Are you saying it's not the ideal way of testing?

Thanks for the detailed explanation. :)
Andrew GAndrew G
Hi Apuroop

Well, if we think about why we write test code - it is to ensure the code we write does what it is supposed to do ... and to ensure what we write doesn't break the SF platform (why we bulkify).
So if we look at a snippet of your code 
06        string purchaseContractName = 'AGP/19/03/16599';
07        Trade_Confirmation__c tradeConfirmationnum = new Trade_Confirmation__c(Name = 'AGC0000012', Purchase_Contract__c = purchaseContractName);

08        insert tradeConfirmationNum;
09         
10        System.assertNotEquals(null, tradeConfirmationnum.id);
11        System.assertEquals('AGC0000012', tradeConfirmationnum.Name);
12        System.assertEquals('AGP/19/03/16599', tradeConfirmationnum.Purchase_Contract__c);
(numbers included to assist explanation)
Fairly straightforward - create a record, insert and then test.
But what are we testing?  that the record was saved as expected.  Well, guess what?  That is what Salesforce promises it will do - that if i put an entry in a field and hit save, the record will keep the value.  So testing the individual field values only checks that Salesforce did it's job.  It doesn't really test the custom code.

Now, the other element to consider is how the code works. When I declare creation of an record (tradeConfirmationnum ) , that record , and the values i assign, is held in memory.  So when we do an assert against that record, I am testing the "in memory" record, not the record that is written to the database.   So, if my code is doing something as I write the record to the database, I should grab that written record and test that the values that I expect my code to write have occurred.


One of the vagaries of test code is that the ID is available after insert.  It is an interesting question.  It would seem to indicate that the "insert" of the "in memory" record does update the "in memory" record.  I usually, even if doing a before insert trigger test, grab the record from the database with a query after the insert.  I might go play and see if a "before insert" trigger will update a non-defined field "in memory".

I hope my explanation is satisfactory.

Regards
Andrew
 
Andrew GAndrew G
Hi Gayatri
Sorry for stealing your thread, but hopefully you have found this educational.

@apuroop

Ok, here is a quick example of why we grab the written record in our Apex Test Classes
Simple Before Insert Trigger: 
trigger Lead_Testing on Lead (before insert){

	if (Trigger.isBefore) {
	    for (Lead lead : Trigger.new) {
	    	if (String.isEmpty(lead.Industry)) {
	    		lead.Industry = 'Other';
	    	}
	    }
	}
}
And a test class 
@isTest
private class Lead_Testing_Test {
	
	@isTest static void test_method_one() {
		// Implement test code
		Lead beforeLead = new Lead(LastName='Sample',
							FirstName ='A', 
							Company='ACME', 
							Status = 'Open - Not Contacted',
							Email = 'sample@acme.acme');
        System.assertEquals(null,beforeLead.Id);
		insert beforeLead;

		System.assertNotEquals(null,beforeLead.Id);
		System.assertEquals(null,beforeLead.Name);
		System.assertEquals(null,beforeLead.Industry);

		List<lead> insertedLeads = new List<Lead>();
		insertedLeads = [SELECT Id, Name, Industry FROM Lead WHERE Id = :beforeLead.Id];
		Lead insertedLead = insertedLeads[0];

		System.assertNotEquals(null,insertedLead.Id);
		System.assertEquals('A Sample',insertedLead.Name);
		System.assertEquals('Other',insertedLead.Industry);
	}
	
	@isTest static void test_method_two() {
		// Implement test code
		Lead beforeLead = new Lead(LastName='Sample',
							FirstName ='A', 
							Company='ACME', 
							Status = 'Open - Not Contacted',
							Email = 'sample@acme.acme',
							Industry = '');
		insert beforeLead;

		System.assertEquals('',beforeLead.Industry);
		
		List<lead> insertedLeads = new List<Lead>();
		insertedLeads = [SELECT Id, Name, Industry FROM Lead WHERE Id = :beforeLead.Id];
		Lead insertedLead = insertedLeads[0];

		System.assertEquals('Other',insertedLead.Industry);
	}
	
}

Test one:
So, we notice that in the first test the Industry value is null when we assert against 'in memory' value. 
But when we grab the written record, the asserted value is Other. (as set by the trigger).
Note that I also have an assert before we insert the newLead record, to show that the record Id has not been created before we do the insert.

Now, test two:
To check that the issue is not because I didn't declare the field in the record creation, when we set the value to '' (blank) that is what is asserted in the in memory record. System.assertEquals('',beforeLead.Industry);  This is because it is "in memory".
But when we grab the written record, the asserted value is Other. (as set by the trigger).

I included the Name field, as out of the box, the Name field is a concatenation of the FirstName and LastName fields.  So even for OoB SF functions, the 'in memory' record is not updated by the insert, but we see that the Id  is available to the in memory record after the insert.

Regards
Andrew
Gayatri KarthikGayatri Karthik
@Apuroop Thanks for explain and share the code . I learned how to write APEX Trigger and Test classes .Once again thanks for your time.
Gayatri KarthikGayatri Karthik
@Andrew G 

      First of all thank you for step by step explanation. Based on your code I modified my trigger and APEX Test class . NOw code coverage is 100% . As I am new to sales force development above mentioned article is very useful for my future Apex Classes development.Any doubts I will post the thread again.Thanks for your valuable time. 

regards,
Gayatri
Gayatri KarthikGayatri Karthik
@Andrew
        quick question : above scenario like cross object fields update Must use APEX Triggers or any other way we can handle in sales force?
Andrew GAndrew G
Hi Gayatri

Depending on the exact update and the relation to the first object, you could use Process Builder or Flows, or a combination of both.  And if you want to simply display data in a child, you could use a formula field.

Regards
Andrew