Give Me An Assertion Vasily. One Assertion Only, Please

I was work­ing through some bro­ken unit tests this morn­ing for the project I’m cur­rently on and the sec­ond one in my list looked like this:

      
 [TestMethod]
        public void SelectTest()
        {
            Item item = session.SelectSingle(a => a.SkuNo == _item.SkuNo);

            Assert.IsTrue(item.Id > 0);
            Assert.AreEqual(item.Id, _item.Id);
            Assert.AreEqual(item.IsAutoReplenished, _item.IsAutoReplenished);
            Assert.AreEqual(item.Status, _item.Status);
            Assert.AreEqual(item.MaintenanceLevel, _item.MaintenanceLevel);
            Assert.AreEqual(item.Description, _item.Description);
            Assert.AreEqual(item.Type, _item.Type);
            Assert.IsTrue(item.LastAdjustmentDate.HasValue);
            Assert.AreEqual(item.LastAdjustmentDate.Value.Date, 
               _item.LastAdjustmentDate.Value.Date);
            Assert.IsTrue(item.LastReceivedDate.HasValue);
            Assert.AreEqual(item.LastReceivedDate.Value.Date,
              _item.LastReceivedDate.Value.Date);
            Assert.IsTrue(item.LastReturnToVendorDate.HasValue);
            Assert.AreEqual(item.LastReturnToVendorDate.Value.Date,         
              _item.LastReturnToVendorDate.Value.Date);

            Assert.IsTrue(item.Upcs.Count == 1);
            Assert.AreEqual(item.Upcs[0].Id, _item.Upcs[0].Id);
            Assert.AreEqual(item.Upcs[0].IsPrimary, _item.Upcs[0].IsPrimary);
            Assert.AreEqual(item.Upcs[0].UpcNo, _item.Upcs[0].UpcNo);
        }
}

An Item gets hooked up in the Ini­tial­ize method of the test suite and then this test does 17 asser­tions on the item it gets back from the ses­sion. There are enough things wrong with this sce­nario to make me write a blog post about it. Which is say­ing some­thing these days, if I’m not drunk blog­ging VPILFs and whin­ing about briskets, I’m pretty much not writ­ing. I digress.

First of all, it’s not really a unit test because it goes out­side the bound­ary of the object under test by set­ting up an item, sav­ing it to the data­base and then select­ing it back out of the data­base for 17 asser­tions. The cur­rent project has a sec­tion for Functional/Integration tests and in all real­ity, this test prob­a­bly belongs there.

Sec­ond, it’s poorly named. The names of test meth­ods should read like doc­u­men­ta­tion for the object under test. “SelectTest” tells me next to noth­ing about what is going on here and in fact, isn’t really what’s being tested at all. I have to read the entire test to know what’s being tested. In truth, this test is try­ing to ver­ify cer­tain prop­er­ties are set on an Item. A cor­rect name for this test is “IdIs­GreaterThanZe­roAn­dIdIs­SameA­sO­rig­i­nalItemI­dAn­dAu­toRe­plen­ishedIs­SameA­sO­rig­i­nalAu­toRe­plen­ished
And­Sta­tu­sIs­SameA­sO­rig­i­nal­Sta­tu­sAnd­Main­te­nanceLevelIs­SameA­sO­rig­i­nal­Main­te­nanceLevel
And­De­scrip­tionIs­SameA­sO­rig­i­nalDe­scrip­tio­nAnd­TypeIs­SameA­sO­rig­inal­Type
And­Las­tAd­just­ment­Date­Has­Val­ue­And­Las­tAd­just­ment­Dat­e­Val­ueIs­SameAsLas­tAd­just­ment­Dat­e­Value
And­Las­tRe­ceived­Date­Has­Val­ue­And­Las­tRe­ceived­Dat­e­Val­ueIs­SameA­sO­rig­i­nal­Las­tRe­ceived­Dat­e­Value
And­Las­tRe­turn­ToVen­dor­Date­Has­Value
And­Las­tRe­turn­ToVen­dor­Dat­e­Val­ueIs­SameAsLas­tRe­turn­ToVen­dor­Dat­e­Value
And­Ha­sOne­Up­cAn­dThatUPC­sIdIs­SameA­sO­rig­i­nalItem­sUP­CId
AndUP­CIs­Pri­ma­ryIs­SameA­sO­rig­i­nalItem­sUP­CIs­Pri­mary
AndUPCNoIsSameAsOriginalItemsUPCNo”

Whew. Deep breath. In through the nose, out through the mouth. Back with me? Good. This brings us to the final prob­lem of this test and that is 17 @#$% ASSERTIONS IS TOO MANY FOR ONE @#$%^ TEST! Tests should be like objects, they should have one func­tion and one func­tion only. It’s amaz­ing how many OO gurus there are out there who think tests like this are com­pletely normal.

There are sev­eral rea­sons why you’d want one asser­tion per test. First, it makes for a very clean doc­u­men­ta­tion of the object under test. Assum­ing all the test names are well-written, you can just read the tests and have a great idea what the object can and can­not do. Sec­ond, most unit test frame­works fail a test at the first failed asser­tion. If you have 17 of them, you don’t really know what’s bro­ken about your object when the first one fails. You have to fix that first before run­ning the test again. Finally, you’re really run­ning mul­ti­ple tests, you’re just doing it implic­itly. Any time you have the option of doing some­thing explic­itly or implic­itly, you should ALWAYS choose the for­mer. Even if your frame­work runs all asser­tions, you’re doing so on object state that is no longer clean after the first assert failed. This isn’t giv­ing you the kind of test­ing that you’re look­ing for.

In the end, test should have a sin­gle piece of func­tion­al­ity to test, they should be well-named and they should have a sin­gle assert. This will pre­vent some jerk from writ­ing a blog post about your test that’s he’s hav­ing to fix 5 months after you wrote it. Even if you’re that jerk.

Ref­er­ences: Write Main­tain­able Unit Tests That Will Save You Time And Tears
Avoid mul­ti­ple asserts in a sin­gle unit test: revis­ited
Test­ing: One Asser­tion Per Test

3 Comments

  • I hope I am not the only one enjoy­ing your tech­ni­cal posts. heh. Inter­est­ing see­ing some­ones opin­ions from a slightly dif­fer­ent environment.

  • Scotch Drinker wrote:

    I’m glad you’re play­ing along at home. My Pylons post from the other day got sub­mit­ted to red­dit so I must be doing some­thing right. :-)

  • Speak­ing of Cap­tain Ramius… I always thought those sonar pings in movies were just drama­ti­za­tions (is that the right word to use here?). I talked to a friend of my dad’s who is a for­mer destroyer cap­tain. He said they’re very real and can be quite loud inside a sub or ship. No won­der the whales get pissed off.

Leave a Reply

Your email is never shared.Required fields are marked *