Scoped value formatters

[FLAMESpl]

Background and motivation

This feature is about allowing adding custom IValueFormatters to AssertionScope. In my development I discovered that I need to selectively customize formatting of an object based on the test's context. There is an option to remove once added formatter but it will not play nicely if tests would be executed simultanously. Having scoped formatters have a side bonus of reusable parametrizing formatter instances.

API Proposal

First make IFormatterCollection interface.

public interface IFormatterCollection : ICollection<IValueFormatter>
{
}

Then add it to AssertionScope:

public class AssertionScope
{
    public IFormatterCollection Formatters { get; } = // initialize from parent scope
}

Formatter API remains unchanged. Internally it will utilize IFormatterCollection for sake of coherence.

API Usage

using (var scope = new AssertionScope())
{
   scope.Formatters .Add(new MyCustomFormatter())
   // some assertions
}

Alternative Designs

Alternative would be to add a custom key-value store in AssertionScope that could be read inside a IValueFormatter.

Risks

None I can think of.

Are you willing to help with a proof-of-concept (as PR in that or a separate repo) first and as pull-request later on?

Yes, please assign this issue to me.

[dennisdoomen]

Maybe scope.UsingFormatter(new MyCustomFormatter()) is nicer?

[FLAMESpl]

It is also an option, this way collection of formatters could be internal.

[dennisdoomen]

I'm fine with that. What about you @jnyrup ?

[jnyrup]

I like the proposal of having scoped formatters 👍

As the signature is Formatter.ToString(object value, FormattingOptions options = null) placing the scoped formatters on FormattingOptions might be a good place?

When retrieving all formatters in Formatter.Format it will then search in scoped custom formatters, static custom formatters and lastly static default formatters.

Perhaps we should also move the current Formatter.CustomFormatters to FormattingOptions? This should be doable without introducing any breaking changes, by forwarding the existing AddFormatter and RemoveFormatter. The statically set custom formatters will then be set on the static AssertionOptions.FormattingOptions and the scoped ones will be set on AssertionScope.FormattingOptions.

To set a scoped formatter you'll only have to do

using var scope = new AssertionScope();
scope.FormattingOptions.AddFormatter(new MyCustomFormatter());

[dennisdoomen]

That sounds like a great idea. Let's go for that.

[dennisdoomen]

@FLAMESpl are you still interested in this one?

[ChristoWolf]

@dennisdoomen: I would certainly be interested 😃

[dennisdoomen]

I meant in implementing this.

[ChristoWolf]

Ah whoops, I misunderstood.

[IT-VBFK]

I am willing to take that, if @FLAMESpl doesn't want to.

[ITaluone]

Should a nested AssertionScope pick up all previous defined scoped formatters?

[dennisdoomen]

Everything that is associated with an AssertionScope flows in the nested scopes.

[ITaluone]

Ok, the tricky part is how to determine which scoped formatters to remove at scope dispose 🤔

[dennisdoomen]

Yeah, it means that the scope needs some state on which scope has added a particular formatter.

[ITaluone]

Since @FLAMESpl does not respond to this issue, I started the implementation. I have one remaining question: Should inner scope formatting options be allowed to modify outer scope scoped formatters?

For example: Should this be possible?

[Fact]
public void Test2()
{
    using var outerScope = new AssertionScope();

    var outerFormatter = new OuterFormatter();
    var innerFormatter = new InnerFormatter();
    outerScope.FormattingOptions.AddFormatter(outerFormatter);

    using var innerScope = new AssertionScope();
    innerScope.FormattingOptions.AddFormatter(innerFormatter);

    innerScope.FormattingOptions.RemoveFormatter(outerFormatter);
}

My main concern is, that this would become pretty "slow" when traversing back a lot of parent AssertionScopes, like:

[Fact]
public void Test2()
{
    using var outerScope = new AssertionScope();

    var outerFormatter = new OuterFormatter();
    var innerFormatter = new InnerFormatter();
    outerScope.FormattingOptions.AddFormatter(outerFormatter);

    using var innerScope = new AssertionScope();
    innerScope.FormattingOptions.AddFormatter(innerFormatter);

    using var innerScope1 = new AssertionScope();
    innerScope1.FormattingOptions.AddFormatter(innerFormatter);

    // ...
    using var innerScope100 = new AssertionScope();
    innerScope100.FormattingOptions.AddFormatter(innerFormatter);

    innerScope100.FormattingOptions.RemoveFormatter(outerFormatter);

}

That said: my current implementation does not allow such a thing.

[dennisdoomen]

Should inner scope formatting options be allowed to modify outer scope scoped formatters?

No, but inner scope options should be able to override the formatters used by the outer scope. So if the inner options remove a formatter that was added by the outer scope, that should only affect the inner scope.

[ITaluone]

I struggle to understand, why the scoped formatters are not tied to the AssertionScope?

The fact that the formatters are mainly static and we forward Formatter.AddFormatter() to FormattingOptions.AddFormatter I cannot distinguish whether this should go to the static custom formatters or to the scoped formatters.

Second, with the current implementation I am able to do the following: AssertionOption.Formatters.AddFormatter() which will then add a scoped formatter and not a custom formatter..

So: long story short I wonder what you are thinking about this:

using var outerScope = new AssertionScope();

var outerFormatter = new OuterFormatter();
outerScope.AddFormatter(outerFormatter);

which adds a scoped formatter, and

// as suggested forwarding the first to the second
Formatter.AddFormatter(); 
AssertionOptions.FormattingOptions.AddFormatter(); 

to add a custom formatter.

Edit: After a few times back-and-forth I finally managed this.. Ever day a new challenge 🎉🎉

[dennisdoomen]

This usually happens only if I run all tests. (Please note, that this couldn't be a issue on my side, because this obviously happens on GHA too: see here: https://github.com/fluentassertions/fluentassertions/actions/runs/9565943493/job/26370017856

I've never seen this before, but it does seem to be related to formatters.

[dennisdoomen]

I struggle to understand, why the scoped formatters are not tied to the AssertionScope?

Well, that's what you are trying to change, right?

The fact that the formatters are mainly static and we forward Formatter.AddFormatter() to FormattingOptions.AddFormatter I cannot distinguish whether this should go to the static custom formatters or to the scoped formatters.

I don't understand the question. When you use Formatter.AddFormatter, with the proposed changes, you're adding a global formatter to the AssertionOptions.FormattingOptions

Second, with the current implementation I am able to do the following: AssertionOption.Formatters.AddFormatter() which will then add a scoped formatter and not a custom formatter..

This should add a global formatter that is available to all assertion scopes.

So: long story short I wonder what you are thinking about this:

using var outerScope = new AssertionScope();

var outerFormatter = new OuterFormatter();
outerScope.AddFormatter(outerFormatter);

which adds a scoped formatter, and

// as suggested forwarding the first to the second
Formatter.AddFormatter(); 
AssertionOptions.FormattingOptions.AddFormatter(); 

to add a custom formatter.

I thought we said to use outerScope.FormattingOptions.AddFormatter(formatter)?

[ITaluone]

I have to admit, I had blinders on.. After I rethought the API it suddenly made sense.. Sorry for the spam..

I struggle to understand, why the scoped formatters are not tied to the AssertionScope?

Well, that's what you are trying to change, right?

What I meant was: why are the scoped formatters tied to FormattingOptions and not to AssertionScope directly. But like I said.. I rethought this and it made sense

[dennisdoomen]

Don't worry about that. I had to study the entire thread and look at the code to build a good understanding as well.

[ChristoWolf]

When will this be released?

[jnyrup]

When will this be released?

With v7 - we don't have an ETA