18

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.

  • 1
    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 3 '17 at 9:07
  • @usr Looks like you caught the key flaw of such way - Side effect from an iterator. – SerG Jul 3 '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 3 '17 at 12:37
  • @IllusiveBrian In this sense, in the example, yes. It's still less than with additional .All(). – SerG Jul 3 '17 at 12:47
37

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 3 '17 at 6:35
  • 1
    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 3 '17 at 11:58
  • There's something wrong with your example. Calling ToList will iterate the enumerable, will it not? – RubberDuck Jul 3 '17 at 16:22
  • 1
    @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 3 '17 at 16:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.