Flaky apex tests fail unreproducible with UNABLE_TO_LOCK_ROW when inserting Product2

Question

Recently, our tests started to fail randomly with the dreaded UNABLE_TO_LOCK_ROW. The only fix seems to "Restart on Failed" the pipeline until it succeeds.

This is the fixture code in question, line 31 is insert prods

public static List<Product2> InsertProductsWithStandardPrices() {
    List<Product2> prods = new List<Product2>();
    prods.add(CreateProduct('Test Base Product 1', 'A-1', 'Base Product', 'Each'));
    prods.add(CreateProduct('Test Base Product 2', 'A-2', 'Base Product', 'Each'));
    prods.add(CreateProduct('Test Base Product 3', 'A-3', 'Base Product', 'Each'));
    prods.add(CreateProduct('Test Service Product 1', 'C-1', 'Service', 'per Hour'));
    prods.add(CreateSubscriptionProduct('Test License (Yearly, 24 months)', 'B-1', 'Yearly', '24 months'));
    prods.add(CreateSubscriptionProduct('Test License (Yearly, 12 months)', 'B-2', 'Yearly', '12 months'));
    prods.add(CreateSubscriptionProduct('Test License (Monthly, 12 months)', 'B-3', 'Monthly', '12 months'));
    prods.add(CreateSubscriptionProduct('Test License (Monthly, 1 month)', 'B-4', 'Monthly', '1 month'));
    prods.add(CreateSubscriptionProduct('Test License (One-Time, 5 yrs)', 'B-6', 'One Time', '60 months', false));
    prods.add(CreateSubscriptionProduct('Test License (One-Time, 8 yrs)', 'B-6', 'One Time', '96 months', false));
    prods.add(CreateSubscriptionProduct('Test License (One-Time, Lifetime)', 'B-7', 'One Time', 'Lifetime', false));
    insert prods;
    InsertStandardPrices(prods);
    return prods;
}

The InsertStandardPrices method internally uses Test.getStandardPricebookId, but since it is not even reached, I assume the error is not with the pricebook setup.

This fixture is used in roughly 25 of our E2E test classes (in @TestSetup). Currently, we run slightly above 1100 apex tests, roughly 150-200 of them are E2E tests. Our Unit Tests --by definition-- are not allowed to use DML (we use mocks and stubs instead), but end-to-end tests --by definition-- cannot use a mocked selector layer (wouldn't qualify to be an end to end test, then).

When I researched the topic, most questions were almost 8 years old, and the solutions only included:

• Disable seeAllData
• Disable parallel testing
• Use FOR UPDATE

For obvious reasons, our tests do not use seeAllData=true, so this solution does not apply. We use either mocks or fixtures like this one.

Disabling parallel testing is not an option. The last time I tried to disable parallel for all E2E tests (isParallel=false), a test run that executes only slightly over 500 unit tests (usually completes in less than 3 minutes) took 30 minutes (!) and then timed out.

If code updates products in our tests, it already uses FOR UPDATE in the SOQL. We do not have any logic in our production code that updates products. We have lot's of fixtures that create accounts, contacts, orders, opportunities, service contracts, etc. None of them ever failed with this error. This only happens on the Product-fixtures.

I'm running out of ideas here. Anybody has a suggestion how to fix this?

1. Stop code reuse in fixtures and create duplicated fixtures?
2. Use try/catch to retry the insert at least 2 or 3 times?

Appreciate any suggestion!

Answer 1

Overview

If you hare hitting row locks consistently, there is exactly one way to guarantee you will never hit them. Bypass the database. Injection is relatively easy on both ends by using the Singleton pattern to allow mocking.

Should you test complex domain logic in your automated test suite that runs on platform? Maybe. There's an argument for it. If you can keep your write operations lean, maximize the value of @TestSetup, and combine as many tests as possible, then you might be able to make it work.

The fact you are already hitting row locks may mean that ship has sailed, although interacting with the standard Pricebook is older functionality that might have some edge case that lets you work around the problem at a smaller scale. In general, however, testing highly nuanced domains may be a task better suited to an external tool.

Scripting tests off platform and running them ad hoc can give you flow testing without forcing you to incur the run-time cost every time you validate production changes. Hopefully you validate at least some lower environments too... Since these scripts would not be "unit" tests, you can alter what you consider best practice. For example, maybe abandon data silos, allow hard coded ids, etc. These behaviors written into a local script, run against a specific org, would not have the downsides that would be associated with them were they written into your on-platform test suite.

Query Injection

The obvious interface for query mocking should look roughly as follows:

public virtual inherited sharing class Query
{
    public static SObject record(SObject record) { return instance.passThrough(records); }
    public static List<SObject> records(List<SObject> records) { return instance.passThrough(records);
    
    static Query instance = new Query();
    @TestVisible setMock(Query mock) { instance = mock; }
    protected SObject passThrough(SObject record) { return record; }
    protected List<SObject> passThrough(List<SObject> records) { return records; }
}

Talk about simple! Now in your tests, all you have to do is define your own mock query, much as you would with callouts. It should all be quite natural. You can define your own implementations in a QueryMocks class to keep things tidy, support count queries if you so desire, and so on.

DML Injection

Injecting writes rather than reads has perhaps slightly less obvious naming, but not too much. It is a bit more copy-paste boilerplate, but also higher value in the long run as it provides an easy central error handling mechanism. Having already written my own dml handling library, I had to choose differently, but ideally you'd just call your primary service DML.

public inherited sharing class DML
{
    public virtual inherited sharing class SaveLayer
    {
        public virtual void insertRecord(SObject record) { insert record; }
        public virtual void insertRecords(List<SObject> records) { insert records; }
        // etc.
    }
    public without sharing class Trampoline
    {
        // fun fact, to actually elevate access here you have to redefine the methods
        public override void insertRecord(SObject record) { raw.insertRecord(record); }
    }
    public inherited sharing class Logger
    {
        public override void insertRecord(SObject record)
        {
            try
            {
                raw.insertRecord(record);
            }
            catch (DmlException error)
            {
                MyLogService.register(error);
                // any logger hooked up to a domain should
                // ideally only buffer records to a local register
                // and flush at the end of the trigger handler execution
            }
        }
    }
    public virtual inherited sharing class Partial
    {
        public virtual SaveResult insertRecord(SObject record)
        {
            // due to the different signature it is likely simplest
            // to just define partial methods separately and not extend SaveLayer
        }
    }
    public class SaveResult { /* see below */ }
}

If you have a wide variety of error handling behaviors, it may be worth splitting out the error handling for more portability, as I did here. It's a bigger commitment to the architecture though.

Related reading: Do the *Result objects in Database have a common ancestor?

The Upshot

First of all, one great benefit of the above pattern is that you can insert the injection pattern, do nothing else with your code base, and it will behave exactly as it did before. So it is very low risk to implement. It is also very flexible. You can can mock out just reads, just writes, both, or get granular and mock only queries against certain tables. Let's take for example some code that takes some records as input, queries some related records, applies conditional logic, then updates those related records. All of that can be achieved without touching the database.

Futzing around with the names of common utility patterns a bit, but the test structure would look something like below.

@IsTest
static void testMyMethod()
{
    Parent__c mockParent = (Parent__c)MyTestDataBuilder.buildDummy(Parent__c.sObjectType);
    Query.setMock(new MyQueryMock(mockParent));
    
    DML.SaveLayer dmlMock = new DmlMocks.MyImplementation();
    DML.setMock(dmlMock);

    MyObject__c dummy = new MyObject__c(
        Parent__c = mockParent.Id,
        Foo__c = 'Bar'
    );
    
    Test.startTest();
        MyService.doStuff(dummy);
    Test.stopTest();
    
    system.assertEquals(1, dmlMock.updates.size(), 'One update should be performed');
    
    List<Parent__c> updatedRecords = dmlMock.updates[0];
    system.assertEquals(1, updatedRecords.size(), 'One record should be touched');
    
    Parent__c updatedParent = updatedRecords[0];
    system.assertEquals(dummy.Foo__c, updatedParent.Latest_Foo__c, 'Some informative message here');
}

Given that setting dummy ids on records is possible, you can test aspects of your functionality such as proper mappings, which records get updated, and how. You won't be able to test that complex SOQL filters are correct, but often times the query filters are simple and don't need such rigorous validation. If that's not the case, you might write a few tests against that functionality without using injection. But it can be the exception rather than the rule.