Provide opt-out to AssertionOptions(o => o.WithStrictOrdering())

[BrunoJuchli]

Breaking change in 5.5 (coming from 5.4.2)

We globally set the default assertion equivalency options using:

AssertionOptions.AssertEquivalencyUsing(options => options
	.WithStrictOrdering());

Because we believe it's safer to require strict ordering by default (I can provide a reasoning if anyone is interested).

Now, in special cases where the order did not matter, we used:

collection.Should().BeEquivalentTo(
    ...,
    options => options.WithoutStrictOrderingFor(info => true));

to counteract the effect of WithStrictOrdering(). For certain scenarios this has stopped working with 5.5.0 (we now have a test fail which didn't fail before).

Proposal

WithoutStrictOrderingFor(info => true)) was a bit of an ugly workaround in the first place.
Instead of "reverting" FluentAssertions to work as before it seems more sensible to actually support WithoutStrictOrdering as a proper counterpart to WithStrictOrdering.

SelfReferenceEquivalencyAssertionOptions<TSelf>.WithStrictOrdering()

performs

orderingRules.Add(new MatchAllOrderingRule());

The suggested SelfReferenceEquivalencyAssertionOptions<TSelf>.WithoutStrictOrdering() should perform something along the lines of:

var strictOrderingRules = orderingRules.OfType<MatchAllOrderingRule>().ToList();
foreach(var strictOrderingRule in strictOrderingRules)
{
    orderingRules.Remove(strictOrderingRule)
}

Complete minimal example reproducing the issue

I have taken the liberty of not providing a repro - for now. If necessary I can produce one, but since I don't really consider the 5.5 behavior a bug It seems more appropriate to focus on the feature request - which, I believe, does resolve the issue appropriately.

Note: I suspect this to be connected to #965
the reason is that this problem only surfaces in a very few tests even though we apply WithoutStrictOrderingFor(info => true) 166 times.

Please also note that I have tested the following cases:

Default WithStrictOrdering, override with WithoutStrictOrderingFor(info => true)

--> used to work in 5.4.2
--> doesn't work in 5.5 (i.E. breaking change)

Default WithStrictOrdering, no WithoutStrictOrderingFor, expected and actual sequence match

--> works in 5.5 (expected)

Default without strict ordering, no WithoutStrictOrderingFor

--> works in 5.5 (expected)

Versions

• 5.5 (before: 5.4.2)
• .Net Framework 4.7.2

[BrunoJuchli]

@dennisdoomen I could provide a PR. I'm not really sure how to implement the feature, though, because...

• should WithStrictOrdering be adapted to not add multiple copies of MatchAllOrderingRule?
• should OrderingRuleCollection be adapted to not allow adding of multiple instances of identical rules?
• should there be only one instance of MatchAllOrderingRule? (might provide an (imperfect) way to determine "identical" rules => identical reference, identical rule)
• should WithoutStrictOrdering remove all rules from OrderingRuleCollection which were added by WithoutStrictOrderingFor and WithStrictOrderingFor? (=> all instances of PredicateBasedOrderingRule)
• what should be the FluentAssertions behavior in case an insensible configuration method is called? For example: current configuration is WithoutStrictOrdering and one applies a WithoutStrictOrderingFor (=> shouldn't have an effect since we're not checking ordering of anything). Note: "sensible" or "not sensible" depends on the call sequence...
• What should be the result of the call sequence .WithStrictOrdering, .WithStrictOrderingFor(..), WithoutStrictOrdering. Should strict ordering be completely disabled or should WithStrictOrderingFor remain in effect?

I believe:

• throwing an exception is problematic because there's no way to check whether the method call will throw before execution. I.E. if I have some piece of generic code which should ignore strict ordering of a specific property it will fail if ever I first perform WithoutStrictOrdering for this test.
• not throwing an exception will probably result in some hard to understand use cases / effects. Hence I believe the simplest behavior should be preferable.

I currently believe the simplest behavior would be

• for WithStrictOrdering to remove all WithoutStrictOrderingFor
• for WithoutStrictOrdering to remove all WithStrictOrderingFor

and thus where the vision should lead us. However, how much of a breaking change is this? If this is too much of a breaking change, what would be the best acceptable change leading us in the right direction and resolving the issue?
... Since WithoutStrictOrdering doesn't exist yet, it could remove all WithStrictOrderingFor without representing a breaking change. (Yes, it's ugly because it doesn't behave like WithStrictOrdering)

[dennisdoomen]

What I don't understand is how this worked in previous versions. The code around ordering hasn't been changed for 9 months or so. That being said, I think multiple calls to WithStrictOrdering, WithStrictOrderingFor and WithoutStrictOrdering should allow chaining and be evaluated in order of execution.

[BrunoJuchli]

@dennisdoomen

(...). That being said, I think multiple calls to WithStrictOrdering, WithStrictOrderingFor and WithoutStrictOrdering should allow chaining and be evaluated in order of execution.

Allowed in the sense of not throwing an exception? But what is the expected effect on the comparison?

Do the two variants of the following examples always behave the same or is there a difference?

Example 1

WithoutStrictOrdering()
WithStrictOrderingFor(XYZ)
WithStrictOrdering()

versus

WithStrictOrdering()

Example 2

WithoutStrictOrdering()
WithStrictOrderingFor(XYZ)
WithStrictOrdering()
WithoutStrictOrdering()

versus

WithStrictOrderingFor(XYZ)

[dennisdoomen]

Example 2 should be the same as WithoutStrictOrdering. So the order is important.

[dennisdoomen]

Is it possible that the original issue that you raised is now fixed by #967 ?

[jnyrup]

@BrunoJuchli Can you check if the failing tests contain empty lists?
Otherwise please provide a minimal complete verifiable example.

[BrunoJuchli]

@dennisdoomen Thanks,yes, most likely it's fixed by #967

---

I believe it's still valuable to have the API allow overriding of defaults configured by AssertionOptions.AssertEquivalencyUsing(....). While it's seems that our current approach of using WithoutStrictOrdering(x => true) works it's not very concise.

The implementation of FluentAssertions, however, does not actually override the default but rather complement it by adding a "filter" which reverses the effect of - the still configured - WithStrictOrderingFilter. Performance wise that's sub-optimal. And I do think that applies to the concept, too. It make's it harder to understand - conceptually and at runtime (you might end up with a config that contains 20 filters but in the end just behaves like WithoutStrictOrdering) and has a higher risk of unintended side effects being introduced by changes.

Thus, if this is ok with you, I'd try to provide a PR, adapting the implementation to:

• WithStrictOrdering and WithoutStrictOrdering to reset the orderingRules collection to a single "strict" or "not strict" rule.
• If the current config is WithoutStrictOrdering and WithoutStrictOrderFor is called, it doesn't have any effect.
• If the current config is WithStrictOrdering and WithStrictOrderingFor is called, it doesn't have any effect.

And tests, of course:

• default ordering is WithoutStrictOrdering for all children (except byte arrays)
• WithStrictOrdering results in strict ordering for all children
• same goes for WithoutStrictOrdering().WithStrictOrdering().
• same goes for WithoutStringOrderingFor(FOO).WithStrictOrdering()

and vice versa for WithoutStrictOrdering.

[dennisdoomen]

Sure. I'll change the label.

[BrunoJuchli]

Should WithoutStrictOrdering() remove ByteArrayOrderingRule? It seems likely that strict byte array ordering is almost always demanded. This poses a design-problem though. Since it's possible it was removed from the default it's not clear whether it should be present after performing:

 .WithStrictOrdering()
 .WithoutStrictOrdering()

Personally, it seems risky to have WithoutStrictOrdering remove the ByteArrayOrderingRule. One might expect bytes still to be compared in order. And you'll only find out your expectation is wrong when a bug occurs which you believed covered by your tests...

Having WithoutStrictOrdering always clearing the OrderingRuleCollection and then adding a ByteArrayOrderingRule might not be expected - especially if the ByteArrayOrderingRule was not present on the default. The issue will make itself visible by throwing an exception, though.
To remove the ByteArrayOrderingRule I could add another method to the interface (WithoutStrictOrderingForBytes). Removing all ordering rules would then become:

.WithoutStrictOrdering()
.WithoutStrictOrderingForBytes()

from a usage perspective I'd prefer this.

[dennisdoomen]

I think you'll need to collect all ordering rules in some kind of collection and then evaluate the resulting ordering based on the order of the rules.

[BrunoJuchli]

Taken care of by #974