IsOnlyMadeOf do not throw if comparaing non empty values to empty enumerable
VGib opened this issue · 9 comments
Hello,
here is my sample: NFluent 2.5
using System;
using System.Linq;
using System.Collections.Generic;
using NFluent;
public class Program
{
public static void Main()
{
try
{
var a = new HashSet<string>();
Check.That(a).IsOnlyMadeOf("toto");
Console.WriteLine("OK 1");
}
catch(Exception exception)
{
Console.WriteLine("Error 1");
}
try
{
var a = new HashSet<string>();
a.Add("titi");
Check.That(a).IsOnlyMadeOf("toto");
Console.WriteLine("OK 2");
}
catch(Exception exception)
{
Console.WriteLine("Error 2");
}
}
}Expected Behavior
I think that empty Hashset with InOnlyMadeOf('toto') should throw an exception. Actually, it don't throws.
Expect output:
Error 1
Error 2
Actual Output:
OK 1
Error 2
Current Behavior
It doesn't throw
Possible Solution
Code that reproduce the issue
Context
Some unit test are good, as they should not be (the expect values are empty, as some values are wanted.
Your Environment
Visual Studio 2017, .Net 4.5.
Reproduce on .Net Fiddler (last version) just copy paste the code
Hi
thanks for taking the time for raising this. This is the expected behaviour, and there are unit tests to confirm that. This is also a note on the xml documentation to specify this check succeeds on empty enumeration.
But the wiki documentation was clearly misleading and poorly written, so I updated it. It is better now.
You can add And.Not.IsEmpty() to get the expected results.
public static void Main()
{
try
{
var a = new HashSet<string>();
Check.That(a).IsOnlyMadeOf("toto").And.Not.IsEmpty();
Console.WriteLine("OK 1");
}
catch(Exception exception)
{
Console.WriteLine("Error 1");
}
...
}I suggest you browser through the wiki because I suspect you may find a check for your specific need.
Hello,
thanks for replaying. I understand this is the expected behavior. But this seems to be a little tricky, as using an non empty enumeration without the value will, fails.
Thus if you simply forget or remove the enumeration filling or initializing, the test will be green, as the application behavior may be incorrect.
You're fix work, but you need to be aware of it, and put it everywhere. You simply don't think on this case when implementing yours tests.
This won't be guessed after having experience trouble, hope this won't be an application crash ...
Is it possible to add an IsOnlyMadeOfAndNotEmpty function for shortcuting you're code ?
VGib
What do you want to check for?
IsOnlyMadeOf is supposed to cover niche use cases, hence not be everywhere.
You should try isequalto or isEquivalentTo as they are more strict.
You have Contains if you only need to make certain that some entries are there, disregarding the others
@VGib : did you find a better alternative? if not, can you please explain we what you are trying to achieve so we can find or design a better check for that?
Hello,
So I will tell you how I manage to use the IsOnlyMadeOf method.
I need to create a share object to be able to add values to a very big enumerable object.
As this has lot of values, I pass at the share object directly a dictionary key / value.
This is were it begin to be tricky. Each dictionary as some functional behavior. And when I'm developping in TDD the sharing object filler; I want to ensure that only some values as been passed to the good dictionary.
So I need a ContainsOnly method, but as this is not useable for Dictionary Enumeration (because of the hash, the comment advises me to use IsOnlyMadeOf.
<member name="M:NFluent.EnumerableCheckExtensions.ContainsExactly``1(NFluent.ICheck{System.Collections.Generic.IEnumerable{``0}},System.Collections.Generic.IEnumerable{``0})">
<summary>
Checks that the enumerable contains only the given expected values and nothing else, in order.
This check should only be used with IEnumerable that have a consistent iteration order
(i.e. don't use it with Hashtable, prefer <see cref="M:NFluent.EnumerableCheckExtensions.IsOnlyMadeOf(NFluent.ICheck{System.Collections.IEnumerable},System.Object[])" /> in that case).
</summary>
<typeparam name="T">Type of the elements to be found.</typeparam>
<param name="check">The fluent check to be extended.</param>
<param name="expectedValues">The expected values to be found.</param>
<returns>
A check link.
</returns>
<exception cref="T:NFluent.FluentCheckException">
The enumerable does not contains only the exact given values and nothing else,
in order.
</exception>
</member>That's why I used the IsOnlyMadeOf. Sometimes I want to check that the good Dictionary's keys values are all present, and that the bad. one are not present.
I don't remember having seens the disclaimer, maybe I've seen it and consider this was not a problem since my dictionary should not be empty.. Until I had a bug, and after investigating, have seen that I forgot to initialize the dictionary .....
Any way, you're library is great. I still don't agree with the fact that IsOnlyMadeOf should return true when the enumeration is empty. But this is you're choice, and I respect it.
I will add the NonEmpty, when I need it , or try to use Contains or ContainsKey method when It's possible.
And I will educate my client not doing the same mistake....
Regards,
VGib
Hi, thanks for your long answer. I will fix the comment regarding ContainsExactly to redirect to IsEquivalentTo.
Have you tried IsEqualTo or IsEquivalentTo?
One of improved feature of V2.5.0 is that both those checks offer better support for enumeration and dictionaries.
And if I understand correctly your use case, it looks like IsEquivalentTo should fit the bill.
IsEquivalent ==> has the expected content, nothing more, nothing less (including duplicate entries, if any)
IsEqualTo ==> has the expected content, in the expected order.
Thx,
IsEquivalent is exactly what I wanted. I'll use now IsEquivalent and IsEqualTo.
Regards,
VGib
So for me this seems to be good.
Can I close this pull request ?