Extend GenericCollectionAssertions to include a ContainInConsecutiveOrder

[StacyCash]

Description

We were writing tests where we needed to check if one collection contained another in order. For this, we used ContainInOrder.

However, we found that because ContainInOrder doesn't check that the items are consecutive we could introduce mutations in the code and our tests would still pass.

To solve this we have made a new method in the same class, GenericCollectionAssertions, called ContainInConsecutiveOrder. This is a stricter version of the current check, adding the check that the expected items all appear in consecutive order. For completeness, we also introduced the opposite NotContainInConsecutiveOrder.

The API change is an extension to the API, with, as far as I can see, no breaking changes.

We have implemented these changes, with associated tests, in a branch and would like to submit a PR.

[StacyCash]

The changes we have made can be seen here: develop...StacyCash:fluentassertions:develop

[dennisdoomen]

I think it makes total sense. What about you @jnyrup ?

[lg2de]

What do you think about an option for ContainsInOrder like x => x.WithConsecutiveOrder similar to WithStrictOrder on BeEquivalentTo?
I think we get better overview on the features with less methods and more options.

[jnyrup]

@lg2de I'm curious where you find we get a better overview with the lambda approach?

The general design has been that assertions are most discoverable when they are directly available from IntelliSense after writing .Should().
The most notable exception is of course the configuring parameter to equivalency assertions.
The only other exceptions I can think of are those that take an OccurrenceConstraint parameter, e.g. StringAssertions.Contain.

I recall the API some years ago allowed/required one to pass in the SortOrder enum to select if a sequence should be sorted ascending or descending (but I can't find the change).
If my memory serves me well, we replaced that overload with two explicitly named overloads (BeInAscendingOrder and BeInDescendingOrder) in the name of discoverability.

That said, I'm not opposed to the idea of also having overloads that lets you write something like this.

subject.Should().Contain(expected);
subject.Should().Contain(expected, e => e.InOrder());
subject.Should().Contain(expected, e => e.InConsecutiveOrder());

Off the top of my head I can't recall if we have any principles about having multiple entries to the same assertion.

[lg2de]

From my point of view all options are discoverable in IntelliSense either way. But they are not in parallel but stacked. This helps in my eyes for an better overview.
When I'm starting to write the assertion I first think about "Ok, want to know whether the items are within the other collection." => Contains. Then I think about further details or restrictions: InOrder, Consecutive, EachOnlyOnce, ...

Off the top of my head I can't recall if we have any principles about having multiple entries to the same assertion.

I have not seen that before and it may be confusing. Then it would be more difficult to know whether Be and BeSameAs are different or not.

BTW: This is an example where I think "one method with multiple options" could be easier to understand because the code will get more explicit, e.g.: foo.Should().Be(bar, o => o.ByReference()).

[dennisdoomen]

Although I don't dislike the proposal, my problem is that we already took a certain (ad-hoc) path where we have distinct methods for distinct goals. If we would have taken this design principle as a starting point, we could have done that. And we still can, but it would be a major breaking change that would require a major review of all APIs to see what they would look like in a major new version. Not only would this require a lot of work and planning, it would also be a major breaking change for our installed base without gaining a lot.

[jnyrup]

I'm good with ContainInConsecutiveOrder.

@lg2de Thanks for your perspectives (here and elsewhere in the repo).
I'm happy to have your extra set of eyes to ensure we leave no stone unturned.

[StacyCash]

Can I create PR based on the linked repo and this issue? Or is there a process that needs to be gone through first?

[jnyrup]

If you create a PR and link it to this issue it's perfect.

Btw, this issue inspired me to the optimizations in #1960 and a similar approach could be applied to ContainInConsecutiveOrder.

Take this pseudo code

for(int i = 0; i < subject.Count - expected.Count; i++)
{
    if (subject[i...expected.Count] == expected)
    {
        // success
    }
}

// fail

[StacyCash]

Brilliant, I'll get it updated and the PR created tomorrow 😊