Skip to main content

Command Palette

Search for a command to run...

Debugging Flakey Tests

Static Parallelization

Updated
•5 min read•View as Markdown
Debugging Flakey Tests
J
I am a developer in Seattle with interests in Security (cyber and IRL), machine learning, and distributed systems.

A co-worker reached out to me to help debug some test failures. These were actually integration tests that were run with in-memory webserver:

Factory = ConfigureWebApplicationFactory();
HttpClient = Factory!.CreateClient();

...

private WebApplicationFactory<Program> ConfigureWebApplicationFactory()
{
    return new WebApplicationFactory<Program>().WithWebHostBuilder(
        builder =>
        {
            builder.UseEnvironment(CfsEnvironmentUtils.Local);

            if (_settings is { MockService1: true, } or { MockService2: true, } or { MockService3: true, })
            {
                builder.ConfigureTestServices(
                    services =>
                    {
                        if (_settings.MockService1)
                        {
                            services.AddSingleton<IPipelineBehavior<Service1Request, Service1Response>, Service1MockingBehavior>();
                        }

                        if (_settings.MockService2)
                        {
                            services.AddSingleton<IPipelineBehavior<Service2Request, Service2Response>, Service2MockBehavior>();
                        }

                        if (_settings.MockService3)
                        {
                            services.AddSingleton<IPipelineBehavior<Service3Request, User>, Service3MockBehavior>((sp) => Service3Mock);
                        }

                        if (_settings.MockKafka)
                        {
                            var descriptor = services.FirstOrDefault(d => d.ServiceType == typeof(IKafkaProcessor));
                            if (descriptor != null)
                            {
                                services.Remove(descriptor);
                            }

                            services.AddSingleton(A.Fake<IKafkaProcessor>());
                            services.AddSingleton(typeof(IPipelineBehavior<,>), typeof(KafkaRequestMockBehavior<,>));
                        }

                        services.AddMemoryCache()
                        .AddSingleton<ISomeCache>((sp) =>
                        {
                            IMemoryCache cache = sp.GetRequiredService<IMemoryCache>();
                            ILogger logger = sp.GetRequiredService<ILogger>();

                            return new SomeMemoryCache(cache, logger);
                        })
                        .AddSingleton<IPolicyEvaluator, DisableAuthorizationPolicyEvaluator>();
                    }
                );
            }
        }
    );
}

We use MediatR which we plan to rip out, hence the mocking of Behaviors. We also disable JWT validation/authorization when in-memory tests are run since we don’t want to store any secrets within the build system and the that logic will get tested after deployment with actual integration test run. This is done with the DisableAuthorizationPolicyEvaluator:

public class DisableAuthorizationPolicyEvaluator : IPolicyEvaluator
{
    private static readonly ClaimsPrincipal AuthenticatedUser =
        new(new ClaimsIdentity("Test"));

    // This method is called to authenticate the user.
    public Task<AuthenticateResult> AuthenticateAsync(AuthorizationPolicy policy, HttpContext context)
    {
        var ticket = new AuthenticationTicket(AuthenticatedUser, "Test");
        var result = AuthenticateResult.Success(ticket);
        return Task.FromResult(result);
    }

    // This is the updated method that now accepts the optional resource.
    public Task<PolicyAuthorizationResult> AuthorizeAsync(AuthorizationPolicy policy, AuthenticateResult authenticationResult, HttpContext context, object? resource)
    {
        // Simply return a successful authorization result, ignoring the resource.
        return Task.FromResult(PolicyAuthorizationResult.Success());
    }
}

Note that these tests were passing before and the test harness used was xUnit. I had recently upgraded from xUnit 2.9.3 to xUnit.v3 3.2.2. A non-breaking migration/upgrade for us. The pull-request build passed as did the CI/CD build.

Initially, I thought it was just the changes in the feature branch.

HowTo avoid flaky tests in Cypress with examples? | by Vikash Anand Singh |  Medium

Checking out main branch and running them locally as a sanity check, three tests fail consistently when run together. When run individually, they succeed. This leads me to think that it’s some sort of state mixing in the tests. Looking closer at the data generation I can see:

private static ShippingInfo ShippingInfo => new()
{
    CurrencyCode = "CAD",
    ShippingCharge = 0.0m,
    RecipientAddress = SampleAddress,
    SenderAddress = SampleAddress,
};

private static List<StoreBasketItem> StoreBasketItems =>
[
    new StoreBasketItem
    {
        Amount = 5.00m,
        CurrencyCode = "CAD",
        OrderItemId = "1234",
    },
];

...

private static Request<StoreTransaction> CreateFullStoreRequest(string agentName = ""
    , DeviceInfo? deviceInfo = null)
{
    return new Request<StoreTransaction>
    {
        RequestModel = new StoreTransaction
        {
            CurrencyCode = "CAD",
            CcAgentName = agentName,
            DeviceReputation = deviceInfo ?? DeviceInfo,
            Amount = 100.00m,
            TransactionId = "123456789",
            TransactionDateTime = TransactionDateTimeOffset,
            UserId = "USER123",
            ReferenceTransactionId = "REF123",
            Platform = "Web",
            Market = "CAD",
            BillingInfo = CarrierBillingInfo,
            Shipping = ShippingInfo,
            StoreBasket = StoreBasketItems
        }
    };
}

One of the failing tests:

[Fact]
public async Task PostStoreAsync_WithMissingCarrierDetails_ThrowsWithBadRequest()
{
    Request<StoreTransaction> request = CreateFullStoreRequest();
    request.RequestModel.BillingInfo!.CarrierDetails = null;

    Response<FraudCheckResult, ErrorResponse> response =
        await _client.PostStoreAsync(
            request,
            nameof(PostStoreAsync_WithMissingShippingInfo_ThrowsWithBadRequest)
        );

    AssertResponseCheck(response, new AssertOptions
    {
        ExpectSuccess = response != null && response.IsSuccess,
        ExpectedErrorCode = ErrorCodes.InvalidRequestObject.Code,
        ExpectedErrorMessages = ["Shipping"],
        ExpectedStatusCode = HttpStatusCode.BadRequest
    });
}

[Fact]
public async Task PostStoreAsync_WithMissingShippingInfo_ThrowsWithBadRequest()
{
    Request<StoreTransaction> request = CreateFullStoreRequest();
    request.RequestModel.Shipping = null;

    Response<FraudCheckResult, ErrorResponse> response =
        await _client.PostStoreAsync(
            request,
            nameof(PostStoreAsync_WithMissingShippingInfo_ThrowsWithBadRequest)
        );

    AssertResponseCheck(response, new AssertOptions
    {
        ExpectSuccess = response != null && response.IsSuccess,
        ExpectedErrorCode = ErrorCodes.InvalidRequestObject.Code,
        ExpectedErrorMessages = ["Shipping"],
        ExpectedStatusCode = HttpStatusCode.BadRequest
    });
}

When the two tests above are selected to run together via the Test Runner/Explorer, one of them will fail consistently (may fail in different order on another machine). We can see that the CreateFullStoreRequest() is called from both tests and that method is static.

The root cause is that some previous developer created static methods for test generation, but xUnit and MSTest will run tests in parallel and the request object is not thread-safe. The properties are also populated with static methods so they retain shared state between the tests. Since the properties are mutable, each test is changing one referenced object that is class scoped. A classic mistake. This can lead to race-conditions where one test passes and the other doesn’t or all tests pass, which explains why my builds succeeded previously.

There is nothing wrong with static methods, just how you use them. In this case, it should not be done. Each test should create it’s own object or call a non-static method to do so. I prefer using AutoFixture for test data generation.

We could also run the tests sequentially and that would fix some tests, but it would also slow down the run if we had lots of tests. This can be done with xUnit’s class attribute [Collection("Collection Name")] or with MSTest’s class attribute [DoNotParallelize]. MSTest, by default will run tests sequentially in the same assembly which is why issues like this may not pop up. Running tests sequentially may be faster in some instances when the test class initialization logic is slow, but usually best to run unit tests in parallel as they should have no dependencies. Even in this case when they are actually integration tests, they should still not share state.

I tend to use static methods for situations where performance matters and they are just helper or utility functions. I tend to keep that logic simple and have NO state or mutable resources/internals. The most common usage is Extension methods to extend class functionality or with dependency injection (DI) helpers.

I limit them to these scenarios since they can’t be mocked or overridden. So top level usage is fine, but using them within other classes and methods will be an issue if you want to change their behavior. Limit using them to reduce issues with thread-safety or if they could hold large amount of data since they are stored on the heap and lasts the lifetime of the application so not typically not garbage collected until the application is stopped. This can be an issue with webservices.

These tests have been flakey for at least 3 years…

References