Unit tests: deferred assertions with Linq

Question

Is it ok to add deferred assertions like this

var actualKittens = actualKittens.Select(kitten => {
    Assert.IsСute(kitten);
    return kitten
});

Why? So I can iterate just once even with statements expecting materialized collection for example:

CollectionAssert.AreEquivalent(expectedKittens, actualKittens.ToList());

And also it could be not just Select but a method with iterator defined and having a lot of checks and logic (e.g. some counting and filtering).

The seed of doubt is the complexity of reading and debugging such code in case of test fail.

I wouldn't *rely* for this for testing but it's sometimes OK to do this in production code if there is no better solution. You can make yourself a helper function `sequence.WithSideEffect(item => Assert.IsCute(item))` to make it cleaner. — usr, Jul 03 '17 at 09:07
@usr Looks like you caught the key flaw of such way - Side effect from an iterator. — SerG, Jul 03 '17 at 10:40
Don't you still have to iterate twice, once to generate the list and again to compare it to `expectedKittens`? You've just hidden the iterations behind method calls. — IllusiveBrian, Jul 03 '17 at 12:37
@IllusiveBrian In this sense, in the example, yes. It's still less than with additional `.All()`. — SerG, Jul 03 '17 at 12:47

score 37 · Accepted Answer · edited Jul 03 '17 at 06:10

Is it ok to add deferred assertions like this [..]

No, it isn't. Why? Because if you for any reason remove the second assert the test would still turn green and you would think it still works but it doesn't as the collection won't get enumeratated. If you have two or more independent assertions they'll keep doing their job even if you disable one of them.

Consider this combination:

Assert.IsTrue(actualKittens.All(x => x.IsCute());
CollectionAssert.AreEquivalent(expectedKittens, actualKittens.ToList());

Now even if you disable or remove one of the asserts the other one would still do its job. Also if you forget to materialize the collection it might take longer to run but it'll still be working. Independent tests are more robust and reliable.

There is also a second no. I'm not sure how other frameworks handle it but if you are using MS Test platform then you wouldn't know which test failed. If you double click the failed test it'll show you the CollectionAssert as failed but in reality it was the nested Assert that went wrong and it'll be extremely hard to debug. Here's an example:

    [TestMethod]
    public void TestMethod()
    {
        var numbers = new[] { 1, 2, 3 }.Select(x =>
        {
            Assert.Fail("Wrong number.");
            return x;
        });

        // This will fail and you won't be sure why.
        CollectionAssert.AreEqual(new[] { 1, 2, 3 }, numbers.ToList()); 

    }

This means that the first test is actually useless because it doesn't help to find a bug. You don't know whether it failed because a number was invalid or because both collections were different.

Why? So I can iterate just once even with statements expecting materialized collection

I wonder why you care about it? These are unit tests. You don't have to optimize every bit of them and usually tests do not require millions of items so performance should not be a concern.

You'll need to maintain such tests so why should you make them more complex then necessary? Write simple asserts that work.

If for some reason an assertion does need to be buried in the control flow, one way to make sure that it was executed is to keep a counter/flag that is incremented/set to true before the nested assertion. Later, we can assert that the expected control flow has been taken by checking this counter. Not perfect, but largely addresses your first criticism. — amon, Jul 03 '17 at 06:35
Additionally, you or someone else would come back to the deferred assertion in 6 months time and have to waste time figuring it out. — DavidTheWin, Jul 03 '17 at 11:58
There's something wrong with your example. Calling `ToList` will iterate the enumerable, will it not? — RubberDuck, Jul 03 '17 at 16:22
@RubberDuck yes, it will and it'll also fail however not at the `Assert.Fail` but at the `CollectionAssert` and you won't be able to say which assert actually went wrong. I mean VS won't focus on `Assert.Fail` but on the other one... now you can debug. — t3chb0t, Jul 03 '17 at 16:25

Unit tests: deferred assertions with Linq

1 Answers1