Test Coverage fail on simple before insert & update apex trigger

Question

Apex n00b here. This has been gnawing me up over past few weeks. I have this very simple before insert / update trigger on Opportunity that auto-selects the Price Book based on a dropdown value containing Sales Office (State) location info.

Here's my Trigger:

trigger SelectPriceBook on Opportunity ( before insert, before update ) {


    for( Opportunity opp : Trigger.new ) {
        // Change Price Book
        // New York
        if( opp.Campus__c == 'NYC' )
          opp.Pricebook2Id = PB_NYC; // contains a Pricebook's ID

        // Atlanta
        if( opp.Campus__c == 'ATL' )
          opp.Pricebook2Id = PB_ATL; // contains another Pricebook's ID

    }

}

Here's my Test Class:

@isTest (SeeAllData = true)
public class SelectPriceBookTestClass {

    static testMethod void validateSelectPriceBook() {

        // Pricebook IDs
        ID PB_NYC =  'xxxx';
        ID PB_ATL =  'xxxx';

        // New Opp
        Opportunity opp = new Opportunity();
        opp.Name = 'Test Opp';
        opp.Office__c = 'NYC';
        opp.StageName = 'Quote';       

        // Insert
        insert opp;

        // Retrive inserted opportunity
        opp = [SELECT Pricebook2id FROM Opportunity WHERE Id =:opp.Id];
        System.debug( 'Retrieved Pricebook Id: ' + opp.Pricebook2Id );

        // Change Campus
        opp.Office__c = 'ATL';
        // Update Opportunity
        update opp;

        // Retrive updated opportunity
        opp = [SELECT Pricebook2id FROM Opportunity WHERE Id =:opp.Id];
        System.debug( 'Retrieved Updated Pricebook Id: ' + opp.Pricebook2Id );        

        // Test
        System.assertEquals( PB_ATL, opp.Pricebook2Id );

    }
}

Test runs report 0% coverage.

Please suggest a fix for this.

Also on similar lines I have this other before insert trigger that sets the Owner of an Event same as the Owner of the parent Lead.

Here's the code:

trigger AutoCampusTourOwner on Event( before insert ) {

    for( Event evt : Trigger.new ) {
        // Abort if other kind of Event
        if( evt.Subject != 'Visit' )
            return;        

        // Set Owner Id
        Lead parentLead = [SELECT OwnerId FROM Lead WHERE Id = :evt.WhoId];
        evt.OwnerId = parentLead.OwnerId;

    }
}

This too is causing 0% coverage - my guess is it's got something to do with the for loops in both. I know I'm seriously flouting DML rules by invoking SOQL query inside a for loop - but for my purposes it should be fine, as these Events are created manually and only 1 at a time. So there are no scopes of governor limits kicking in due to bulk inserts.

The code in both cases works a 100%. PLEASE suggest a fix for the test cases.

Thank you.

Answer 1

Firstly it's good that you recognised the issue with the soql query inside the foor loop, however it is worth fixing, although they are being created manually you never know if later on down the track you need to data load or something changes to that effect, besides it's good practice to do it right the first time. Consider changing to something like this unless anyone else has something more efficient. This way we only have 1 query.

trigger AutoCampusTourOwner on Event( before insert ) 
{
    map<ID,ID> mapLeadtoOwnerId = new map<ID,ID>();

    for( Event evt : Trigger.new )
    {
        // Abort if other kind of Event
        if( evt.Subject != 'Visit' )
        {
            // put ownerId into map
            mapLeadtoOwnerId.put(evt.WhoId, null);
        }
    }

    // Add the ownerIds to the map
    for(Lead parentLead = [SELECT OwnerId FROM Lead WHERE Id IN mapLeadtoOwnerId.keyset()])
    {
        mapLeadtoOwnerId.put(parentLead.Id, parentLead.OwnerId);
    }

    // Loop through again now that we have the OwnerIds
    for( Event evt : Trigger.new)
    {
        if(evt.Subject != 'Visit')
        {
            evt.OwnerId = mapLeadtoOwnerId.get(evt.WhoId);
        }
        }
}

For the test coverage if at all possible I would suggest avoiding using (SeeAllData = true), instead create your test data in your method, also use test.starttest() and test.stoptest() annotation to signal where you want your test to start and end.

I would also probably change the first trigger to get the correct pricebook from a map as well rather than via an ID and check if the value has changed I.e.

trigger SelectPriceBook on Opportunity ( before insert, before update )
{

    map<String,Id> mapPriceBooks = new map<String,Id>();

    for(PriceBook2 sPricebook : [SELECT Id, Name FROM PriceBook2])
    {
        mapPriceBooks.add(sPriceBook.Name, sPriceBook.Id);
    }

      for( Opportunity opp : Trigger.new ) {
    // Change Price 

    if(Trigger.isInsert)
    {
        // New York
        if( opp.Campus__c == 'NYC' )
          opp.Pricebook2Id = mapPriceBooks.get(PB_NYC); // returns the correct Pricebook's ID

        // Atlanta
        if( opp.Campus__c == 'ATL' )
          opp.Pricebook2Id = mapPriceBooks.get(PB_ATL); // returns the correct Pricebook's ID
    }
    else if(Trigger.isUpdate)
    {
        Opportunity beforeUpdate = Trigger.oldMap.get(opp.Id);

        // Check if value has changed
        if(opp.Campus__c != beforeUpdate.Campus__c)
        {
            // New York
            if( opp.Campus__c == 'NYC' )
              opp.Pricebook2Id = mapPriceBooks.get(PB_NYC); // returns the correct Pricebook's ID

            // Atlanta
            if( opp.Campus__c == 'ATL' )
              opp.Pricebook2Id = mapPriceBooks.get(PB_ATL); // returns the correct Pricebook's ID
        }
    }
}

Maybe try those 3 things and see how you go. Also when you say that it is causing 0% test coverage is there any failures or is it just not covering any of the triggers lines?