8

Consider the following sample C# Data Transfer Object (DTO):

public class MailingAddress
{
   public string StreetAddress {get; set;}

   public string City {get; set;}

   public string StateOrTerritory {get; set;}

   public string PostalCode {get; set;}
}

I can write unit tests to check for someone setting the various string members to null or empty strings. I could also make the setters private on all the fields so that it has to be constructed via a constructor.

I'm asking about this because the CodeCoverage tool is reporting 0% on all these DTO methods and I'm trying to figure out if there's some reasonable testing I might do here.

I have googled a bit and not come up with a lot. I also searched here but haven't found anything that seems to address this. If I've missed something please link it in the comments.

EDIT:

Someone helpfully suggested that my question might be answered by this question. The thing is that while it doesn't look like there's code being run for the various fields, there is, in fact default code there.

It wouldn't be a case of testing the language features. If someone modifies the default behavior of the get/set pairs then I should have unit tests around them to insure they still behave as expected.

7
  • 4
  • 14
    Dont. This class does not have any behaviour. When you are unit testing you are testing behaviour. There is none here, so there should not be any tests specifically for this class. However this class may be used in unit tests covering other classes, and as such you should probably be getting coverage from those tests? If you dont get any such coverage, then that is a sign that you either have an unused class that can be safely deleted or that you are not covering some other class with tests.
    – wasatz
    Apr 15, 2020 at 13:18
  • 1
    "I'm asking about this because the CodeCoverage tool is reporting 0% on all these DTO methods" - What methods? I only see properties.
    – Graham
    Apr 15, 2020 at 13:19
  • @Graham, the get and sets can have code, so even though they are properties it's still a decision point. In this case it's like an empty method that doesn't have any code.
    – Jon Raynor
    Apr 15, 2020 at 17:54
  • @Graham Exactly what Jon Raynor said. There's default code on the gets and sets. Granted it's not being modified from the default but I wonder if I shouldn't (as someone suggested below) add a test for a null being passed to the set. Apr 16, 2020 at 15:32

4 Answers 4

28

This class is just a holder of data. It doesn't have any behavior to test. So, no, do not write test cases for this class.

However, your application should have functions that take an instance of this class as an argument. When writing test cases for those functions, you should be using a real instance of this data class. Since there is no behavior, you don't have to worry about external dependencies that might have side-effects. Using a real instance of the class instead of a test double will cause the lines of this class covered by the test suite.

1
5

These days, I'm with Jon on this. But I used to be part of the "no tests" camp in the past.

I think this is one of those many situations where there are no right or wrong answers; only trade-offs. You should educate yourself about the cons and pros and choose to join a camp. Your future experience might motivate you to switch. Anyway, let's get technical.

In a broader sense than covering DTOs, what we're discussing here is to decide whether we need to cover getters (/setters) or not.

I'd first like to refer you to Roy Osherove's book, The Art of Unit Testing where it goes:

"Properties (getters/setters) are good examples of code that usually doesn’t contain any logic and so doesn’t require specific targeting by the tests. It’s code that will probably get used by the unit of work you’re testing, but there’s no need to test it directly. But watch out: once you add any check inside a property, you’ll want to make sure that logic is being tested."

Then I'd like to partially quote Paul Bourdeaux's stand from his blog writeup and refer you to read the examples there as it all doesn't fit here.

"Should we test getters and setters?” The answer, in the humble opinion of this engineer, is yes.

Let’s face it, public getter and setter methods are (normally) inherently simple, and writing unit tests for them seem to be a waste of time – testing the language’s ability to set and pass variables instead of any actual code. But there are a few dangers lurking in not testing them...

The easiest way that I know of to break [getter/setter] code is by changing existing code without understanding the full ramifications of doing so. Testing getters and setters also guards against regression bugs. Here is an example of a simple getter"

Bottom line is, they must be tested. The question is directly or indirectly. Choose your club!

2
  • 2
    Thanks for posting, I agree the path forward here is not absolute.
    – Jon Raynor
    Apr 20, 2020 at 17:24
  • 1
    The last quote by Paul perfectly sums up my thoughts. Why not just write some simple tests for your setters and getters to have the peace of mind for the future?
    – cjnash
    Dec 10, 2020 at 15:53
4

For completeness you could write tests for these. This would cover the code. If anyone changed a getter/setter in the future a test would fail and they would have to update the test(s). The tests would be very trivial at this point since there is no defined behavior. They would be worth very little, since all they are doing is updating the backing private field. But your coverage would be 100% and if anyone changed behavior a test would fail.

Tests should have value.

Let's say your setter changed to (psuedo code):

set { if value != null 
      { name = value }; 
    } 

In the case, there is behavior present. Having test(s) in this scenario would add value.

8
  • 3
    It is only worth adding test coverage at the time you add behavior to the setter. Prior to that it is extra work that does nothing more to guard against application failures than the compiler provides. Apr 16, 2020 at 11:37
  • This is an excellent point. This was what I was considering doing. Adding a constructor, making the sets private and then adding testing in the constructor to insure no one passes nulls in. Apr 16, 2020 at 15:33
  • 3
    And at the point that you have custom functionality that needs testing, it ceases to be a DTO any more. :)
    – Eric King
    Apr 16, 2020 at 18:38
  • I think you're saying what should be done is to add tests later when the behaviour is added. By it's kind of unclear, can you reword this? Apr 16, 2020 at 21:16
  • 1
    @sepehr It's relevant insofar as DTOs are not usually unit-tested because there is no behavior to test. DTOs don't have behavior because behavior isn't serializable, which is specifically the point of DTOs. So it's very common to hear and read that DTOs don't need testing. But, if you have a behavior in a class that you feel must be tested, as is the case here, then it's important to note that it's probably not a good candidate as a DTO (data transfer object) any more, since that behavior isn't serializable.
    – Eric King
    Apr 17, 2020 at 21:11
-2

One quick approach that gets some coverage is:

            //Arrange
            var dto = new T();

            //Act
            var result = JsonConvert.DeserializeObject<T>(JsonConvert.SerializeObject(dto));

            //Assert
            result.Should().NotBeNull();

For complete coverage, build out the json to be deserialized.

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