ArgumentOutOfRangeException in FluentAssertions.Equivalency.Tracing.StringBuilderTraceWriter.ToString()

[vturecek]

Description

I'm getting random ArgumentOutOfRangeException from an internal StringBuilder.ToString() call when using object equivalency (Should().BeEquivalentTo) with large object graphs during concurrent test runs with xUnit:

Error Message:
   System.ArgumentOutOfRangeException : Index was out of range. Must be non-negative and less than or equal to the size of the collection. (Parameter 'chunkLength')
  Stack Trace:
     at System.Text.StringBuilder.ToString()
   at FluentAssertions.Equivalency.Tracing.StringBuilderTraceWriter.ToString()
   at FluentAssertions.Equivalency.EquivalencyValidator.AssertEquality(Comparands comparands, EquivalencyValidationContext context)
   at FluentAssertions.Collections.GenericCollectionAssertions`3.BeEquivalentTo[TExpectation](IEnumerable`1 expectation, Func`2 config, String because, Object[] becauseArgs)
   at FluentAssertions.Collections.GenericCollectionAssertions`3.BeEquivalentTo[TExpectation](IEnumerable`1 expectation, String because, Object[] becauseArgs)

This only seems to occur sometimes when multiple tests are running concurrently. When run individually or sequentially, this error does not occur.

Is there any static or shared StringBuilder internally that might be used in overlapping test runs?

Reproduction Steps

This is difficult to reproduce in a controlled environment, since it only happens occasionally during concurrent test runs. It seems more likely to happen when evaluating large object graphs (depth > 15 or so) and only during concurrent test runs (xUnit concurrency settings "parallelizeAssembly": false)

Expected behavior

Not expecting an exception from StringBuilder.ToString()

Actual behavior

ArgumentOutOfRangeException

Regression?

No response

Known Workarounds

No response

Configuration

.NET 7
Fluent Assertions 6.12.0
xUnit 2.4.1

Other information

No response

Are you willing to help with a pull-request?

No

[jnyrup]

I assume you're using WithTracing somewhere, since StringBuilderTraceWriter otherwise shouldn't be used at all.
Where are you calling WithTracing from?

[jnyrup]

This reproduces the exception

using FluentAssertions;
using FluentAssertions.Execution;

AssertionOptions.AssertEquivalencyUsing(e => e.WithTracing());

Parallel.For(1, 10_000, (_, _) =>
{
    try
    {
        new { A = "a" }.Should().BeEquivalentTo(new { A = "b" });
    }
    catch (AssertionFailedException) { }
});

The problem is setting a single ITraceWriter, which per default is not thread-safe, which is then shared among tests run in parallel.

[vturecek]

@jnyrup thanks for the quick reply. Yes, I have a WithTracing() set globally, that would explain it.

If this uses a shared ITraceWriter internally then setting it globally is destined to have this problem? Is the guidance to add tracing per test with a new trace writer instance, something like this:

Should().BeEquivalentTo(foo, options => options.WithTracing(new StringBuilderTraceWriter()));

[jnyrup]

Setting WithTracing globally should not be causing issues, so this is a bug in Fluent Assertions.

The two workarounds I can currently think of are:

• setting WithTracing() per test
• use WithTracing() globally, but write some thread-safe class implementing ITraceWriter.

I have not dug further into what we could do in FA to make this (more) thread-safe.

[jnyrup]

With the current WithTracing(ITraceWriter writer) method, this implementation of ITraceWriter seems to work better.
It might not be bullet-proof, as it relies on ITraceWriter.ToString() always to be called to clear the current StringBuilderTraceWriter.

So use at your own risk, setting up WithTracing() per test is the safer choice.

AssertionOptions.AssertEquivalencyUsing(e => e.WithTracing(new StringBuilderTraceWriterPool()));

Parallel.For(1, 1_000, (_, _) =>
{
    new { A = "a" }.Should().BeEquivalentTo(new { A = "a" });
});

public sealed class StringBuilderTraceWriterPool : ITraceWriter
{
    private readonly AsyncLocal<StringBuilderTraceWriter> builders = new();

    private StringBuilderTraceWriter Builder => builders.Value ??= new();

    public void AddSingle(string trace) => Builder.AddSingle(trace);

    public IDisposable AddBlock(string trace) => Builder.AddBlock(trace);

    public override string ToString()
    {
        var result = Builder.ToString();
        builders.Value = null;
        return result;
    }
}

The underlying problem is that we reuse the same TraceWriter, i.e. assume it's safe to reuse across threads, which is not true.

fluentassertions/Src/FluentAssertions/Equivalency/SelfReferenceEquivalencyOptions.cs

Lines 95 to 102 in 7632b78

	ConversionSelector = defaults.ConversionSelector.Clone();
	selectionRules.AddRange(defaults.SelectionRules);
	userEquivalencySteps.AddRange(defaults.UserEquivalencySteps);
	matchingRules.AddRange(defaults.MatchingRules);
	OrderingRules = new OrderingRuleCollection(defaults.OrderingRules);
	TraceWriter = defaults.TraceWriter;

[dennisdoomen]

Although it's a bug, tracing is meant for individual diagnostic situations. Why are you enabling it globally?

[vturecek]

Thanks @jnyrup, for now I've disabled tracing globally. If I need to enable it globally again I'll try that version, but for now I'll turn it on only as needed.

@dennisdoomen it's useful to have as much information as possible for test failures during automated test runs (e.g., build pipelines), especially in cases where test failures are not consistently reproducible (like this one, ironically). Since trace output only happens during test failures, it seems like it makes sense to always have it on so that traces are available when a test fails during an automated test run.