HaveCount() does not trigger IReadOnlyCollection<T>.Count

[Corniel]

Description

I'm not sure if this is a real bug, but is not desirable, specially because the analyzer suggests to change collection.Count.Should.Be() to collection.Should().HaveCount().

The reason is that HaveCount() calls Enumerable.Count() that itself has a special case for when T is ICollection, but not for IReadOnlyCollection. The question is, if this should be the responsibility of FluentAssertions.

If so, the fix is easy:

int actualCount = Subject is IReadOnlyCollection<T> readOnly
    ? readOnly.Count
    : Subject.Count();

Reproduction Steps

var collection =  new SomeReadOnlyCollection<T>{ new T{ }  };
collection.Should().HaveCount(1);

Expected behavior

SomeReadOnlyCollection.Count is covered by this test.

Actual behavior

SomeReadOnlyCollection.Count is not cover by this test.

Regression?

Logically not, as it is behavior due to .NET's implementation.

Known Workarounds

var collection =  new SomeReadOnlyCollection<T>{ new T{ }  };
collection.Count.Should().Be(1);

[jnyrup]

HaveCount() does not make any guarantees about calling a Count property on the enumerable asserted on.
If you specifically want to write a test covering your Count property I would not rely on HaveCount.

I assume any IReadOnlyCollection<T> to be a materialized collection, so not using its Count property should "only" be a matter of performance and not correctness.
By correctness I refer to the problem of enumerating lazy enumerables multiple times.
For that problem we still have some work to do. See e.g. #2000 and #2196.

[Corniel]

@jnyrup I know there is a work-around, that is not the point. The point is that there is a analyzer rule, that suggests to rewrite a test, and by doing that changing the coverage.

And that brought me to the question if we should expect it to use the Count property or not. You could also argue the other way around: if we want to check if a custom implementation of ICollection has (for some utter strange reason) gives a different count via the Count property than via .AsEnumerable().Count(), should in fact, the implementation of this assert not be

int actualCount = Subject.AsEnumerable().Count();

I'm not sure, hence the question.

[jnyrup]

The purpose of the analyzer is to help using Fluent Assertions in a more idiomatic way, which can approximately be interpreted as the most readable code code and the most informative failure messages.

You should not make assumptions on exactly how HaveCount() counts the number of elements, that's an implementation detail.
To have complete code coverage on a custom ICollection<T> I would suppress the analyzer.

Guarding against a bogus implementation of ICollection<T> that returns a different value for Count than counting the number of elements it out-of-scope for Fluent Assertions.

I hope I got your questions right.

[Corniel]

I think you did. As mentioned, I understand the choices (implicitly) made. I was just curious, if my assumptions where correct, and if coverage was a parameter in designing the FluentAssertions API. From maintainability perspective, I can see that.