Is it bad practice to use reflection to do complex object assertion for a unit test?

Question

I was reading this topic, which is about using reflection to test a private variable...

But I don't have such problems in my unit test and my code is totally testable.

The only problem is I figured out, it's very time consuming when doing an assertion for each property of a complex object with an expected result; especially for a list of complex objects.

Since it is a complex object, doing a normal Assert.AreEqual is not going to give me a correct result unless I am implementing IEquality for each of the objects.

But even if I do so, this won't tell me which property/field's name, expected value and actual value during assertion.

Correctly, we are manually putting each of the property values into a list and doing a single CollectionAssertion, but this is still time-consuming and when the assertion occurs it only tells me the index of a element value is not equal; it wont' tell me the property name. Which makes it very difficult to debug (I had to go to debug mode and look at the element in collection).

So I wonder, if I write a recursive reflection method that will do assertion on two complex objects, which will tells me every property name, expected value, actual value.

Is that a good practice or bad practice?

Answer 1

I find a lot of people won't even consider Reflection, but it has its place. It definitely has drawbacks with regards to performance, type safety, etc as other posters have stated but I actually think unit tests are a fine place to use it. As long as it's done juduciously.

Trying to enforce equality implementations on all objects runs into walls when you don't own all the types you use in your properties. And implementing a hundred mini-comparer classes is as time consuming as writing out the asserts manually.

In the past I've written an extension method that does what you describe:

• Compares two objects of the same type (or that implement a common interface)
• Reflection is used to find all public properties.
• If the property is a value type a straight Assert.AreEquals is done
• For reference types it does a recursive call

At no point do my tests care about property names so refactoring for renames doesn't matter. In fact new properties are found automatically and deleted ones are forgotten.

I've never used it with really complex objects but it's worked fine with the ones I have without slowing down my tests.

So in my opinion in a unit test feel free to use Reflection carefully.

Edit: I'll try to dig up my method for you.

Answer 2

In my opinion, using Reflection is not a very good option. Using Reflection means we lose type safety at compile time. And also, behind usage of Reflection, there is (possibly) case insensitive string search through an Assembly's metadata. This leads to the slow performance. Considering these aspects, I think splitting the original type (as recommended by oleksii) is one good approach.

The other approach could be to write separate tests, to test separate set of attributes, using pure accessor methods. This may not be suitable in all cases. But, it does in certain cases.

For ex: If I have a Customer class, I could write one test to check the Address-type fields; I could write another test to check Order-type fields and so on.

Answer 3

Under normal circumstances you shouldn't need reflection to do anything unit test-related. This is mentioned in answer to question you linked:

Reflection should really only be a last resort

If you need to check whether complex objects are equal, implement such equality check in unit test. There's nothing wrong in having extra code purely for unit testing purposes:

public void ComplexObjectsAreEqual()
{
    var first = // ...
    var second = // ...

    AssertComplexObjectsAreEqual(first, second);
}

private void AssertComplexObjectsAreEqual(ComplexObject first,
    ComplexObject second)
{
    Assert.That(first.Property1, Is.EqualTo(second.Property1),
       "Property1 differs: {0} vs {1}", first.Property1, second.Property1); 
    // ...
}

You shouldn't really treat unit tests as some other code. If something needs to be written to make them more readable, clean, maintanable - write it. It's the same code as everywhere else. Would you compare objects via reflection in your production code?

Answer 4

I would say there are many valid reasons for using reflection to easy unit testing. To quote https://github.com/kbilsted/StatePrinter

the problems with manual unit testing

It is laborious.

When I type and re-type over and over again: Assert.This, Assert.That, ... can't help but wonder why the computer cannot automate this stuff for me. All that needless typing takes time and drains my energy.

When using Stateprinter, the asserts are generated for you whenever there is a mismatch between expected and actual values.

Code and test gets out of sync

When the code changes, say by adding a field to a class, you need to add asserts in some of your tests. Locating where, though, is an entirely manual process. On larger project where no one has the full overview of all classes, the needed changes are not performed in all the places it should.

A similar situation arises when merging code from one branch to another. Say you merge a bug fix or feature from a release branch to the development branch, what I observe over and over again is that the code gets merged, all the tests are run and then the merge is committed. People forget to revisit and double check the entire test suite to figure out there are tests existing on the development branch and not on the branch from where the merge occured, an adjust these accordingly.

When using Stateprinter, object graphs are compared rather than single fields. Thus, when a new field is created, all relevant tests fail. You can adjust the printing to specific fields, but you loose the ability to automatically detect changes in the graph.

Poor readability I

You come a long way with good naming of test classes, test methods and standard naming of test elements. However, no naming convention can make up for the visual clutter asserts creates. Further clutter is added when indexes are used to pick out elements from lists or dictionaries. And don't get me started when combining this with for, foreach loops or LINQ expressions.

When using StatePrinter, object graphs are compared rather than single fields. Thus there is no need for logic in the test to pick out data.

Poor readability II

When I read tests like the below. Think about what is it that is really important here

Assert.IsNotNull(result, "result");
Assert.IsNotNull(result.VersionData, "Version data");
CollectionAssert.IsNotEmpty(result.VersionData)
var adjustmentAccountsInfoData = result.VersionData[0].AdjustmentAccountsInfo;
Assert.IsFalse(adjustmentAccountsInfoData.IsContractAssociatedWithAScheme);
Assert.AreEqual(RiskGroupStatus.High, adjustmentAccountsInfoData.Status);
Assert.That(adjustmentAccountsInfoData.RiskGroupModel, Is.EqualTo(RiskGroupModel.Flexible));
Assert.AreEqual("b", adjustmentAccountsInfoData.PriceModel);
Assert.IsTrue(adjustmentAccountsInfoData.IsManual);

when distilled really what we are trying to express is

adjustmentAccountsInfoData.IsContractAssociatedWithAScheme = false
adjustmentAccountsInfoData.Status = RiskGroupStatus.High
adjustmentAccountsInfoData.RiskGroupModel = RiskGroupModel.Flexible
adjustmentAccountsInfoData.PriceModel = "b"
adjustmentAccountsInfoData.IsManual = true

Poor convincibility

When business objects grow large in number of fields, the opposite holds true for the convincibility of the tests. Are all fields covered? Are fields erroneously compared multiple times? Or against the wrong fields? You know the pain when you have to do 25 asserts on an object, and painstakingly ensure that correct fields are checked against correct fields. And then the reviewer has to go through the same exercise. Why isn't this automated?

When using StatePrinter, object graphs are compared rather than single fields. You know all fields are covered, as all fields are printed.

Answer 5

IMHO, it is a bad practice because:

• Reflection code is slow and difficult to write right
• It is even more difficult to maintain and such code may not be refactor-friendly
• Reflection is slow, unit tests shall be fast
• _{It also doesn't feel right}

To me this looks as if you are trying to plug a hole, rather than fix a problem. In order to fix the problem I can suggest to segregate a large and complex class into a set of smaller ones. If you have many properties - group them into individual classes

---

So that such class

class Foo
{
    T1 Prop1 {get; set;}
    T2 Prop2 {get; set;}
    T3 Prop3 {get; set;}
    T4 Prop4 {get; set;}
}

Would become

class Foo
{
    T12 Prop12 {get; set;}  
    T34 Prop34 {get; set;}  
}
class T12
{
    T1 Prop1 {get; set;}
    T2 Prop2 {get; set;}
}
class T34
{
    T3 Prop3 {get; set;}
    T4 Prop4 {get; set;}
}

Note, that the Foo has now got only one property (that is a "grouped" representation). If you can group the properties in a way, so that any state change would be localised to a specific group - your task becomes much more simplified. You can then assert that a "grouped" property is equal to the expected state.