Need implementation-independent way to test number of items in a collection

[jnm2]

Here's the issue:

IReadOnlyCollection<object> collection = someApi.GetItems(...);
Assert.That(collection, Has.Count.EqualTo(n));

At the API's discretion, it may return object[] or List<object> or some completely internal type. The contract is that I am allowed to call GetEnumerator() and Count.

Currently, Has.Count uses reflection and thus is coupled to the internal type itself rather than the interface. This means that I have to know about the internal choice the API makes and choose Has.Count and Has.Length accordingly. This leaves all my tests explicitly reflecting an internal detail I shouldn't have to care about, and all my tests could break if the API has a non-breaking change to its internals.

Has.Count is also limited in that it doesn't examine the collection as a series of items. It examines it as an object itself, such as new TimesIClicked { Count = 3 }. This means that it in addition to being useless with arrays, it can't be used with enumerables, something I touched on in #1667.

Here are two possible alternatives I have.

Case 1. If Has.Count is semantically interested in not how many items a thing contains but what the property on an object reads, to the degree that they diverge:

• The Count property on arrays and other collections should be discovered and used even though it's only available via casting to IReadOnlyCollection<>, because it is certainly the property that is relevant to the user and Count is probably the property that is available at compile type which the user is looking at.
• This avoids all the irritations in the introduction, caused by coupling tests to knowledge (Length vs Count vs other/not available) that isn't part of the API contract for APIs returning collection interfaces.
• It will never work for enumerables, so an additional solution should be provided separately for them

Case 2. If Has.Count is semantically interested in how many items a thing contains rather than what a certain property on the object reads, to the degree that they diverge:

• It should only access the Count property via a collection interface, and fall back to enumeration if not available.
• This avoids all the irritations in the introduction, caused by coupling tests to knowledge (Length vs Count vs other/not available) that isn't part of the API contract for APIs returning collection interfaces.
• It will work with enumerables

Unless there is a very clear and compelling alternative API for collection counting assertions, for instance suggestions in #1667, users will try to put their Case 2 style burdens on Has.Count because it is named Count, just like collection.Count and LINQ's Count(). It makes sense. If Case 2 is not the way to go with Has.Count, a compelling API should be able to handle the collection-focused and enumerable-focused workloads.

[CharliePoole]

On the one hand, I think you point out a valid limitation in the NUnit syntax. On the other, I think you are asking too much of Has.Count.

I'll deal with the second thing first. Has.Count is one of several elements added as syntactic sugar on top of Has.Property(string name). It's a test that a certain property exists and (optionally) has a certain value. If you are testing properties, you have to know their exact names.

As to the first point, I agree that we don't have an adequate way to test that the number of items in an aggregate value. You have to test either the Count or Length property, or simply count the items yourself. We should definitely have have something like LINQ's Count(), expressed as a piece of syntax. I'm open to suggestions. In issue #1667, I think you were hinting at Has.Exactly(5) as a way to express this. I find that leaves me wanting a follow-on as I read it... "exactly 5 what?" One could use Has.Exactly(5).Items, of course. Does that seem to wordy?

I think there may be another issue here as well, if we are not looking for properties on the interfaces implemented by a type - I thought we were. Have you tested this?

[jnm2]

Has.Exactly(5).Items does not seem too wordy to me. It's succinct. I like it.

I have tested this and was myself surprised by the outcome, which is why I raised the issue. https://dotnetfiddle.net/cPkps1 Can't add the NuGet package for 3.4.1 (boo) so it's running on v2, but I vouch for the fact that I saw the same output with 3.4.1 earlier today.

[CharliePoole]

Changed name to match the actual need rather than connecting it with Has.Count. I think we should implement both Has.Exactly(n).Items and Contains.Exactly(n).Items

[CharliePoole]

@jnm2 Were you planning on a PR for this?

[jnm2]

I was not but am very willing. What timeframe are you hoping for?

[ChrisMaddock]

I'd get a lot of use out of an count assert for IEnumerable - would be great to see! 😄

[jnm2]

I should be able to get to this next week.

[jnm2]

Heh, this is the first thing that can be construed as a free moment. Let me fork this thing. Working off master, I assume?

[CharliePoole]

I know how that is! Yes... from master.

Are you sticking with the Has.Exactly(n) syntax?

[jnm2]

Yes, you wanted Has.Exactly(n).Items and Contains.Exactly(n).Items, right? Should Items be optional (a no-op)?

[jnm2]

And should any other conditions be allowed to be chained? If you say Has.Exactly(n).Items.GreaterThan(x) it sounds as though you are looking for obj.Count(item => item > x) == n in LINQ terminology. Is that scenario worth enabling?

[CharliePoole]

It could be optional, but I'd wait and see how the code turns out. You may have already gone throught the logic of this, but I'll lay it out anyway just in case...

Currently, Exactly appends an ExactCountOperator to the operator stack. So after Exactly, we now waiting to figure out what is going to be counted.

If the next thing encountered is a constraint, we want to apply that constraint to all the items in the collection, counting how many times it succeeds. This happens as a result of calling the operator's Reduce operation.

You'll want Items to trigger some different actions. That's hard enough, it seems to me, without simultaneously wanting Exactly(n) to work by itself.

[CharliePoole]

@jnm2 I think you have to enable it. The user can already write Has.Exactly(5).GreaterThan(x) so if you add Items to the syntax, it's reasonable to want to type Has.Exactly(5).Items.GreaterThan(x).

This essentially makes items a no-op and probably requires ExactCountOperator to become a SelfResolvingOperator - one that may either end an expression or be continued with a constraint.

[jnm2]

Sounds good. I'm still getting set up. CONTRIBUTING.md doesn't explain- what's the process to test everything? Can I get away with not installing Silverlight support?

[CharliePoole]

build -t Test

You can get away with it but somebody will have to test it in the end. Same goes for compact framework. Once you get to the point of a PR, you'll get CI builds and those will cover Silverlight and CF. You could also enable AppVeyor builds in your own fork.

[jnm2]

Cool! I didn't know that was possible. If you aren't too busy, can you link instructions on how to get AppVeyor working in my (pre-PR) fork?

[CharliePoole]

Just sign up on appveyor using your GitHub id. Then add the repo for monitoring by appveyor.

Be sure to use a feature release rather than building in your master. Otherwise, I think the build will try to publish to our MyGet, which would not be very good. It could be that I'll have to change the script for this to work smoothly, but it's an interesting experiment.

[jnm2]

Is the MyGet decision based on the exact name master? I always start a feature branch and delete all other branches when I fork, so that would be taken care of.

Edit: I think appveyor.yml pretty much answers my question. Here goes a build =)

[jnm2]

Build went great, though it took 15 minutes. Too bad the test results tab is blank, NUnit integration would be awesome. In order to determine whether a test failed I'll have to scroll to the bottom of the mile-long output? I assume I'll see a red X in GitHub if a test fails even though the test results tab is empty.

[CharliePoole]

Yes, the GitHub result will indicate whether the test passed. In fact, you can see it as either green or red on appveyor as well. But you do have to scroll to find the errors.

I guess the importance of integrating the test results is a function of how you use appveyor. I haven't missed it myself but I typically run the same tests locally anyway. It could be made to work, of course.

[jnm2]

I'm thinking of it more from the hypothetical standpoint of me using NUnit in my own projects on AppVeyor- but I haven't used anything but msbuild so NUnit tests might well integrate with AppVeyor automatically that way. Anyway, I think VSTS is superior and NUnit definitely works there.

[jnm2]

Any chance we could introduce ISelfResolvingOperator and have SelfResolvingOperator implement it for backward compatibility? That way ExactCountContraint can be CollectionConstraint, ISelfResolvingOperator. Otherwise due to ConstraintBuilder.IsResolvable, ExactCountContraint will have to stop being a CollectionConstraint and will have to duplicate code from that class and PrefixOperator.

[CharliePoole]

Backward compatibility isn't an issue, since each release of the framework stands on its own.

However, I'm not sure about this as an interface. For one thing, I don't think that the different Append overloads would resolve correctly using an interface. And not being a CollectionOperator is a big deal, since all that class contributes is the precedence.

I'd be happy to see you develop this under a PR - that is, submit a PR as soon as you have code rather than waiting till it's ready to merge. That way we can see how it's going and comment as you go along - not a bad idea for something that's so deep in the guts of the framework.

However, before doing that. Let's agree on what syntax variations we want to support. The issue with SelfResolvingOperator suggests to me that we may not want to allow the option of Has.Exactly(n); without anything following. Maybe we want to require Items and have that (somehow) transform the stacks so we end up with what we want.

If I list what I'd like to see it looks like this...

Assert.That(collection, Has.Exactly(3).EqualTo(val)); // Current syntax
Assert.That(collection, Has.Exactly(3).Items.EqualTo(val);
Assert.That(collection, Has.Exactly(3).Items);

I'd develop this in two steps, implementing either the second or the third example first getting it merged before moving on to the other one.

[CharliePoole]

I assigned this to both of us... it's a new approach I have been wanting to try with "new guys." :-)

[jnm2]

Cool, I was already moving towards a PR for that purpose. :-) It should link shortly.

[ChrisMaddock]

Just for interests sake - as you mentioned personal projects - Appveyor does integrate with NUnit natively. It doesn't show up in our scripts as we run the tests via Cake rather than with appveyors native functionality - but the latest Cake does have a command to upload nunit results xml, which I was hoping to plug in when we update Cake. ☺