views:

48

answers:

2

In the book The Art of Unit Testing it talks about wanting to create maintainable and readable unit tests. Around page 204 it mentions that one should try to avoid multiple asserts in one test and, perhaps compare objects with an overridden Equals method. This works great when we have only one object to compare the expected vs. actual results. However what if we have a list (or collection) of said objects.

Consider the test below. I have more than one assert. In fact, there are two separate loops calling asserts. In this case I will end up with 5 assertions. 2 to check the contents of one list exist in another, and 2 vice versa. The 5th comparing the number of elements in the lists.

If anyone has suggestions to improve this test, I'm all ears. I am using MSTest at the moment, though I replaced MSTest's Assert with NUnits for the fluent API (Assert.That).

Current Refactored Code:

        [TestMethod]
#if !NUNIT 
        [HostType("Moles")]
#else
        [Moled]
#endif
        public void LoadCSVBillOfMaterials_WithCorrectCSVFile_ReturnsListOfCSVBillOfMaterialsThatMatchesInput()
        {
            //arrange object(s)            
            var filePath = "Path Does Not Matter Because of Mole in File object";
            string[] csvDataCorrectlyFormatted = { "1000, 1, Alt 1, , TBD, 1, 10.0, Notes, , Description, 2.50, ,A",
                                                   "1001, 1, Alt 2, , TBD, 1, 10.0, Notes, , Description, 2.50, ,A" };

            var materialsExpected = new List<CSVMaterial>();
            materialsExpected.Add(new CSVMaterial("1000", 1, "Alt 1", "TBD", 1m, 10.0m, "Notes", "Description", 2.50m,"A"));
            materialsExpected.Add(new CSVMaterial("1001", 1, "Alt 2", "TBD", 1m, 10.0m, "Notes", "Description", 2.50m,"A"));

            //by-pass actually hitting the file system and use in-memory representation of CSV file
            MFile.ReadAllLinesString = s => csvDataCorrectlyFormatted;

            //act on object(s)                        
            var materialsActual = modCSVImport.LoadCSVBillOfMaterials(filePath);

            //assert something happended            
            Assert.That(materialsActual.Count,Is.EqualTo(materialsExpected.Count));
            materialsExpected.ForEach((anExpectedMaterial) => Assert.That(materialsActual.Contains(anExpectedMaterial)));
            materialsActual.ForEach((anActualMaterial) => Assert.That(materialsExpected.Contains(anActualMaterial)));
        }

Original Multi-Assert Unit-Test:

 ...
            //1st element
            Assert.AreEqual("1000", materials[0].PartNumber);
            Assert.AreEqual(1, materials[0].SequentialItemNumber);
            Assert.AreEqual("Alt 1", materials[0].AltPartNumber);
            Assert.AreEqual("TBD", materials[0].VendorCode);
            Assert.AreEqual(1m, materials[0].Quantity);
            Assert.AreEqual(10.0m, materials[0].PartWeight);
            Assert.AreEqual("Notes", materials[0].PartNotes);
            Assert.AreEqual("Description", materials[0].PartDescription);
            Assert.AreEqual(2.50m, materials[0].UnitCost);
            Assert.AreEqual("A", materials[1].Revision);
            //2nd element
            Assert.AreEqual("1001", materials[1].PartNumber);
            Assert.AreEqual(1, materials[1].SequentialItemNumber);
            Assert.AreEqual("Alt 2", materials[1].AltPartNumber);
            Assert.AreEqual("TBD", materials[1].VendorCode);
            Assert.AreEqual(1m, materials[1].Quantity);
            Assert.AreEqual(10.0m, materials[1].PartWeight);
            Assert.AreEqual("Notes", materials[1].PartNotes);
            Assert.AreEqual("Description", materials[1].PartDescription);
            Assert.AreEqual(2.50m, materials[1].UnitCost);
            Assert.AreEqual("A", materials[1].Revision);
        }
+1  A: 

I frequently have more than one assertion. If it's all part of testing one logical unit of work, I don't see any problem with that.

Now, I do agree that if you've got a type which overrides Equals, that makes tests much simpler than your second form. But in your first test, it looks you really just want to assert that the resulting collection equals an expected one. I think that's logically one assertion - it's just that currently you're performing multiple mini-assertions to test it.

Some unit test frameworks have methods to test whether two collections are equal - and if the one you're using doesn't, you can easily write one. I recently did exactly this in my "reimplementing LINQ to Objects" blog series, because although NUnit provides a helper method, its diagnostics aren't terribly helpful. I refactored the code from MoreLINQ very slightly, basically.

Jon Skeet
By saying, "NUnit provides a helper method," are you referring to Is.EquivlentTo? But yes, I'm trying to see if the two lists are the same (i.e. all elements exist in both and that these elements when compared to each other are equal). The only thing I'm not concerned with is ordering.
Matt Spinelli
@Matt: I was referring to CollectionAssert.AreEqual. I didn't realise you weren't interested in ordering - you may wish to make that clear in the question.
Jon Skeet
A: 

This is the refactoring I'm currently using. I overrode the ToString() method of CSVMaterial and added a more useful assert message. So I think this helps with code readability and maintainability. It also makes the unit test trustworthy (due to the helpful diagnostic message).

And Jon, thanks for the thought about a logical unit of work. My refactored code does about the same thing as the previous iteration. Both still test one logical thing. Also, I'll have to look into the MoreLINQ stuff. If it's in your C# InDepth 2nd edition book, I'll come across it as I bought the MEAP version from Manning. Thanks for your help.

public void LoadCSVBillOfMaterials_WithCorrectCSVFile_ReturnsListOfCSVBillOfMaterialsThatMatchesInput()
{
    //arrange object(s)            
    var filePath = "Path Does Not Matter Because of Mole in File object";
    string[] csvDataCorrectlyFormatted = { "1000, 1, Alt 1, , TBD, 1, 10.0, Notes, , Description, 2.50, ,A",
                                           "1001, 1, Alt 2, , TBD, 1, 10.0, Notes, , Description, 2.50, ,A" };            

    var materialsExpected = new List<CSVMaterial>();
    materialsExpected.Add(new CSVMaterial("1001", 1, "Alt 1", "TBD", 1m, 10.0m, "Notes", "Description", 2.50m,"A"));
    materialsExpected.Add(new CSVMaterial("1001", 1, "Alt 2", "TBD", 1m, 10.0m, "Notes", "Description", 2.50m,"A"));           

    //by-pass actually hitting the file system and use in-memory representation of CSV file
    MFile.ReadAllLinesString = s => csvDataCorrectlyFormatted;

    //act on object(s)                        
    var materialsActual = modCSVImport.LoadCSVBillOfMaterials(filePath);

    //assert something happended            

    //Setup message for failed asserts
    var assertMessage = new StringBuilder();
    assertMessage.AppendLine("Actual Materials:");
    materialsActual.ForEach((m) => assertMessage.AppendLine(m.ToString()));
    assertMessage.AppendLine("Expected Materials:");
    materialsExpected.ForEach((m) => assertMessage.AppendLine(m.ToString()));

    Assert.That(materialsActual, Is.EquivalentTo(materialsExpected),assertMessage.ToString());
}
Matt Spinelli