Apex trigger not working - renewal opportunity creation

Question

I'm implementing an Apex trigger to generate a renewal upon a deal reaching Closed Won status in SFDC, but doesn't seem to be working:

trigger autoRenewal on Opportunity (after insert, after update) {
    List<Opportunity> listOppor = new List<Opportunity>();
    for(Opportunity opp : Trigger.new)
    {

       if(opp.StageName == 'Closed Won' && opp.Account != null)
           {
              Opportunity oppNew = opp.clone(false, true);
              oppNew.Type = 'Existing Business';
              listOppor.add(oppNew);
           }
         insert listOppor;    

    }
}

Which is to say - after a deal reaches 'Closed Won' status, a separate opportunity clone isn't generated. Additionally, I'm having trouble generating tests that meet the 75% criteria:

private class autoRenewalTest {

    static testMethod void autoRenewalTest() {       

        //Data Prep

        //Create Account, Opportunity, Product, etc.
        Account acct1 = new Account(name='test Account One1');
        insert acct1;

        //Create Opportunity on Account
        Opportunity Oppty1 = new Opportunity(name='test Oppty One1');
        Oppty1.AccountId = acct1.Id;
        Oppty1.StageName = 'Closed Won';
        Oppty1.CloseDate = Date.today();

        insert Oppty1;   

         //Create Opportunity on Account
        Opportunity Oppty2 = new Opportunity(name='test Oppty One2');
        Oppty2.AccountId = acct1.Id;
        Oppty2.StageName = 'Discovery';
        Oppty2.CloseDate = Date.today();


        insert Oppty2;              

 }
}

Any help would be greatly appreciated!

Thanks

Answer 1

Three things are wrong here:

Relationship Fields

Relationship fields (e.g. Account) are always null when you first enter a trigger context. You need to check the lookup field (AccountId) instead.

if(opp.StageName == 'Closed Won' && opp.AccountId != null)

DML Inside Loop

As commented on the question, the DML function needs to be outside the for loop.

Infinite Recursion

You're not resetting the StageName field to a new value, so the first closed won opportunity creates a new one, which in turn creates a new one... and so on until you crash from recursion:

Opportunity oppNew = opp.clone(false, true);
oppNew.Type = 'Existing Business';
oppNew.StageName = 'Discovery';
listOppor.add(oppNew);