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);
}
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.
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());
}
精彩评论