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

[BrunoJuchli]

This handles #966

Added the following configuration options:

• WithoutStrictOrdering()
• WithoutStrictOrderingForBytes()

I'm a bit unsure about WithoutStrictOrderingForBytes(). Reasoning:

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 we can live with the situation that byte arrays are always compared in order, irrespective to the options you provide. Do you agree @jnyrup ?

[jnyrup]

I agree that ByteArrayOrderingRule should still be present after calling WithoutStrictOrdering.

I'm not sure about WithoutStrictOrderingForBytes.
How would you express that functionality today?

WithoutStrictOrdering should be documentated in https://github.com/fluentassertions/fluentassertions/blob/master/docs/_pages/documentation.md.

[dennisdoomen]

I'm not sure about WithoutStrictOrderingForBytes.

I don't think we need this, do we really?

[BrunoJuchli]

@jnyrup

I'm not sure about WithoutStrictOrderingForBytes.
How would you express that functionality today?

IMHO you can't.
Originally I had another impression because IEquivalencyAssertionOptions.OrderingRules is public gettable and I thought there was a way to get to IEquivalencyAssertionOptions. It would have been possible to perform OrderingRules.Add(RuleWhichIgnoresByteOrdering). Since the last rule applying to a member wins the IgnoreByteOrdering rule would have won over the the default behavior.

---

Hence I agree that WithoutStrictOrderingForBytes should not be introduced. Strict ordering for bytes should always be kept (as is the current behavior).
Furthermore OrderingRuleCollection.Clear must be internal, otherwise it would be possible to remove strict ordering for bytes.

@jnyrup

WithoutStrictOrdering should be documentated in https://github.com/fluentassertions/fluentassertions/blob/master/docs/_pages/documentation.md.

Will do, thanks for the hint.

[jnyrup]

@BrunoJuchli Sounds good and thanks for thinking about making Clear() internal. No need to expose it.

[BrunoJuchli]

Breaking Change

As per review of @jnyrup I've introduced a breaking change.

Previously:

    .WithoutStrictOrderingFor(x => x.Foo)
    .WithStrictOrdering();

would result in strict ordering for everything but .Foo.

Now the same configuration results in strict ordering for everything.

Request for Decision

@fluentassertions
Could authority please decide among one of the following (or any other...) options:

1. WithoutStrictOrdering and WithStrictOrdering do override previous WithStrictOrderingFor / WithoutStrictOrderingFor configurations
2. WithoutStrictOrdering and WithStrictOrdering do not override previous WithStrictOrderingFor / WithoutStrictOrderingFor configurations
3. WithStrictOrdering does not override previous WithoutStrictOrderingFor, but WithoutStrictOrdering does override previous WithStrictOrderingFor (=> no breakinge change. WithoutStrictOrdering didn't previously exist. Downside: WithStrictOrdering and WithoutStrictOrdering don't behave the same).

Personally I'd like the 1st option as it's how I expected FA to behave, but of course you might have a different opinion. Also, 1st option is a breaking change, the others aren't. I wouldn't go with the 3rd option though.

[dennisdoomen]

I think option 1 is the best. The order of the methods is the important part.

[jnyrup]

I agree that option 1 is the best.
A potential runtime breaking change is fine with me, as the current behavior is unexpected to me.

I would expect the following two equivalences:

.WithoutStrictOrderingFor(x => x.Foo)
.WithStrictOrdering();

// to be equivalent to

.WithStrictOrdering();

.WithStrictOrderingFor(x => x.Foo)
.WithoutStrictOrdering();

// to be equivalent to

.WithoutStrictOrdering();

We apparently only have a single test for each of WithStrictOrderingFor and WithStrictOrderingFor.
It would make me less worried with tests for both the Expression<Func<IMemberInfo, bool>> and the Expression<Func<TExpectation, object>> overloads of WithStrictOrderingFor/WithoutStrictOrderingFor.
This also acts as documentation for the intended behavior.

[BrunoJuchli]

Thanks

@jnyrup

We apparently only have a single test for each of WithStrictOrderingFor and WithStrictOrderingFor.
It would make me less worried with tests for both the Expression<Func<IMemberInfo, bool>> and the Expression<Func<TExpectation, object>> overloads of WithStrictOrderingFor/WithoutStrictOrderingFor.
This also acts as documentation for the intended behavior.

I'm not really sure I understand what you mean.
As i gather there's no WithoutStrictOrderingFor(Expression<Func<TExpectation, object>>). Only WithStrictOrderingFor(...) has two overloads.
For the existing overloads there's a test checking that performing WithStrictOrdering / WithoutStrictOrdering resets them. (=> only in the "resetting" direction, i.E. WithStrictOrderingFor(..) is reset by WithoutStrictOrdering()).

[jnyrup]

To my surprise you're right.
I just assumed both overloads to exist for both WithStrictOrderingFor and WithoutStrictOrderingFor.
Seems like it should have been a part of #307.

My comment about a single test was regarding the existing test coverage for variants of StrictOrderingFor. As the fourth overload is non-existing, the test coverage you provide looks great.