mjhjmtu

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?

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.

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.