3

I had to make a fix, where the line of code was removed for the second time. So I decided to write a unit test just for that.

A Class in its constructor sets a property that I want to test. This property happens to be protected so I can't access it in the unit test.

 [Test]
 public void Constructor_WhenCalled_ThenSomePropertyIsPopulated()
 {
     var vm= new SomeViewModel();
     //line below doesn't compile as SomeProperty is protected
     Assert.IsNotNull(vm.SomeProperty);

So I decided, in my unit test file, to extend that class (it wasn't sealed) and expose the protected property as a public getter:

public class SomeViewModelExtended : SomeViewModel
{
    public SomeViewModelExtended() : base() { }

    public new object SomeProperty
    {
        get { return base.SomeProperty; }
    }
}

 //now I can test
 [Test]
 public void Constructor_WhenCalled_ThenSomePropertyIsPopulated()
 {
     var vm= new SomeViewModelExtended();
     Assert.IsNotNull(vm.SomeProperty);

Now there's an argument on my team that I should only be testing public interfaces among other things and that this is a total quick and dirty hack.

But isn't one of the purposes of unit tests to preserve the code base from unwanted changes? If this is totally wrong what else should do?

denis morozov
  • 6,236
  • 3
  • 30
  • 45

3 Answers3

3

A protected variable is part of the interface. It's part of the interface that any sub class can use. This question has a good summary on protected variables. Is it good practice to make member variables protected?

If you allow derived classes access to their base class' data, then derived classes need to take care to not to invalidate the base class' data's invariants. That throws encapsulation out of the window and is just wrong. (So do getters and setters, BTW.)

In answer to your question, if the variable should not be accessed by sub classes then it should be private. Otherwise your unit test is valid and arguably it should have been implemented earlier!

Community
  • 1
  • 1
Steve
  • 7,171
  • 2
  • 30
  • 52
2

After reading the Roy Osherove's 'Art of Unit Testing' my opinion on this is that testing + maintainability are primary use cases for good code. As such I feel the case for extending types to help with testing is more than valid.

Of course if you can design a type to not require it then that's good, but maintainability and thus testability are paramount.

Often it's said the leaky abstractions are a bad choice, however I find in most cases this is because these two primary use cases are not being considered. An abstraction should be good for all users: functionality & testing & maintenance.

Preet Sangha
  • 64,563
  • 18
  • 145
  • 216
1

You should remove the field altogether until it is actually needed to pass an external test. If there is no external behavior that fails, you don't need the field.

Since the field is protected, I assume that you created it to have something derive from it. Does this derived class need the protected field in order to pass any of its tests? In that case, you're covered.

Tormod
  • 4,551
  • 2
  • 28
  • 50