How is it possible for @TestSetup to effect test result?

Question

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?

Answer 1

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.

---

As a concrete example of a legitimate use of recursion blocking with a flag, we can write code like this:

public class Accounting {
    static Boolean runOnce = false;
    public static void createDoubleBook(Book__c[] books) {
        if(runOnce) {
            return;
        }
        Book__c[] relatedBooks = new Book__c[0];
        for(Book__c book: books) {
            relatedBooks.add(new Book__c(Name=book.RelatedBookName__c));
        }
        runOnce = true;
        insert relatedBooks;
        runOnce = false;
        for(Integer i = 0, s = books.size(); i < s; i++) {
            books[i].RelatedBook__c = relatedBooks[i].Id;
        }
        // Note: incomplete example. We still need an after insert to also link
        // the related books back to the original.
    }
}

Here, we can create pairs of records that will eventually be linked to each other. Without the ability to block recursion, this would simply spiral into a governor limit, but with it, we can safely link records together. This design is also possible with afterInsert triggers and appropriate "rising edge" logic, so this example is for illustrative purposes only.

Answer 2

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.