3

While refactoring a test to make it more consistent with the style of our other tests, a colleague came upon something really weird.

The original test created the test account in @TestSetup, like so:

@IsTest
public TestClass2 {
    private static final Boolean doInsert = true;

    @TestSetup
    public static void beforeTest() {
        TestAccountFactory.create(doInsert);
    }

    @IsTest
    public static void testSomething() {
        User testUser = TestUserFactory.create(doInsert);
        Account testAccount = [SELECT Id FROM Account LIMIT 1];
        String expectedStatus = 'Active';

        System.runAs(testUser) {
            Test.startTest();
                SpagettiCode.abandonAllHopeAllYeWhoEnterHere();
                // Passes   
                System.assertEquals(
                         expectedStatus, 
                         [SELECT Status__c FROM Account 
                         WHERE Id = :testAccount.Id].Status__c]
                    ); 
            Test.stopTest();
        }
        // Passes   
        System.assertEquals(
                         expectedStatus, 
                         [SELECT Status__c FROM Account 
                         WHERE Id = :testAccount.Id].Status__c]
                    ); 
    }
}

As noted, the assertions pass.

In the modified form, the test created the Account within the test, like so:

@IsTest
public TestClass1 {
    private static final Boolean doInsert = true;

        @IsTest
        public static void testSomething() {
            User testUser = TestUserFactory.create(doInsert);
            Account testAccount = TestAccountFactory.create(doInsert);
            String expectedStatus = 'Active';

            System.runAs(testUser) {
                Test.startTest();
                    SpagettiCode.abandonAllHopeAllYeWhoEnterHere();
                    // Passes   
                    System.assertEquals(
                         expectedStatus, 
                         [SELECT Status__c FROM Account 
                         WHERE Id = :testAccount.Id].Status__c]
                    ); 
                Test.stopTest();
            }
            // Fails
            System.assertEquals(
                         expectedStatus, 
                         [SELECT Status__c FROM Account 
                         WHERE Id = :testAccount.Id].Status__c]
                    ); 
        }
    }

Now, the second assertion fails even though the only difference is that the Account was not created in @TestSetup.

This is obviously a simplification of the code and our path to discovering this discrepancy was a torturous one, but we are trying to understand how this difference would even be possible and whether this is something that we could have expected or reflects a defect in how SFDC is executing the test?

  • This code would not compile. Your first example references doInsert out of scope. – Adrian Larson Feb 16 at 17:51
  • Sorry, that's a transcription error... this is a recreation, not the actual code. I'll fix that. – Brian Kessler Feb 16 at 17:54
5

This is being caused by a "recursion blocker" (as mentioned by David Reed). The fix is to remove or correct the recursion blocker from your trigger logic. The reason why this happened is because static variables are reset between unit tests and test setup methods; as long as the trigger ran during test setup, it would have been reset for the unit tests. When the account is created within the unit test itself, the static variable is not reset, causing the behavior you see here.

To fix the problem, first try commenting out the recursion blocker logic and run some tests to make sure nothing breaks. If you get a "record is already in trigger" or "trigger depth exceeded" error, you'll need the recursion blocker, but you should modify it so it resets itself intelligently, or write "rising edge" trigger logic.


Here's my preferred form of recursion blocking:

static Boolean runOnce = false;
public static void beforeInsert(sObject[] records) {
  if(runOnce) {
    return;
  }
  runOnce = true;
  doBeforeInsertLogicWith(records);
  runOnce = false;
}

The secret sauce is the last line; by resetting the variable when the trigger contexts concludes, you'll fix the unit test problem and also allow the trigger to run correctly with batches of records larger than 200.


For rising edge triggers, you check to make sure the record will meet the condition, instead of checking if it already meets the condition.

Here's an example of this:

public static Boolean checkTransition(sobject oldRecord, sobject newRecord) {
  return oldRecord.get(someField) != newRecord.get(someField);
}
public static void handleRecords(sobject[] oldRecords, sobject[] newRecords) {
  sobject[] updates = new sobject[0];
  for(Integer i = 0, s = newRecords.size(); i < s; i++) {
    if(checkTransition(oldRecords[i], newRecords[i])) {
      updates.add(newRecords[i]);
    }
  }
  if(!updates.isEmpty()) {
    doUpdatesOn(updates);
  }
}

Obviously, these are meant more as pseudo-code, but hopefully you should be able to get to a fix using these ideas. Make sure you have a trigger unit test, and attempt to perform logic on at least 201 records in a single list, just to make sure that it is working appropriately either way.

  • Cheers for the feedback. We'll look into this! – Brian Kessler Feb 17 at 7:52
5

Side effects in your test data factory, in automation fired on Account insert, or interactions between those entities and your code under test can absolutely cause this kind of behavior change.

@testSetup executes in a mostly-isolated transaction, insulating you from many of the potential side effects and code interactions between your setup process and your actual code under test. Removing that barrier opens a wide range of potential sources of interference, such as static variables that are not reset after test initialization, limits that have already been partially consumed, enqueued asynchronous actions, possible Mixed DML traps, and so on.

I would find this behavior somewhat surprising but not unheard-of while working with legacy or poorly-architected code, as it sounds like you are here.

One specific example where you can get this kind of weirdness is low-quality trigger recursion guards based on static Booleans (people often call them something like runOnce). Insert the Account, the trigger runs once. Do something else with the Account.... the trigger doesn't run again, because you're still in the same transaction and the static didn't get reset.

  • Cheers for the feedback. We'll look into this! – Brian Kessler Feb 17 at 7:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.