0

I'm doing some TDD practice for my own learning.

I'm trying to figure out how to transparently set up private properties of an object to support testing.

I have a binary tree node:

internal class Node : INode
{
    public int Height { get; private set; }
    public int Data { get; }

    public INode? Left { get; private set; }
    public INode? Right { get; private set; }

    public Node(int data)
    {
        Data = data;
    }
    public INode? Find(int value)
    {
        if (value == Data) return this;
        if(value < Data)
            return Left?.Find(value);
        else
            return Right?.Find(value);
    }

    public void Insert(int value)
    {
        if (value < Data)
        {
            InsertOnLeft(value);
        } 
        else if (value > Data)
        {
            InsertOnRight(value);
        }
    }
    ...
}

I want to be able to set the left and right nodes as mocks, so that I can use things like verify to check that the left node is called by Find when the value is less than Data. (there are other reasons, and quite frankly, this is a toy problem - please grant the assumption that putting mocks on left and right would be useful).

I have come up with three options for accessing the properties:

  1. Change the accessors from private to protected and have the test project use inheritance to allow the values to be set:
internal class TestableNode : Node
{
    public void SetLeft(INode? node) { Left = node; }
    public void SetRight(INode? node) { Right = node; }
}
  1. Make the accessors public in the class but not the interface. This will keep classes outside the library from seeing the accessor, but it makes me feel wrong.
  2. Changing/overloading the insert method to take a node instead of an integer value. This doesn't feel right.
  3. Adding an internal insert method to Node that allows me to insert nodes to specific places. This makes me feel very wrong.

I feel that fundamentally changing the code to allow testing is really wrong - so options 2-4 are escalating degrees of worse on that scale. 1 is a relatively minor change - although it does prevent me from sealing the class.

I feel least bad about option one. Is there a good option that I haven't thought of?

1
  • 5
    Private properties are private, unit tests should not access them. Otherwise, you have tight coupling between your classes and the tests, and you can't do refactoring, or really change anything without doing at least double the amount of work, probably more. Mocks are not for accessing private fields, they are for simulating an aspect of a behavior of a public dependency. Tests are best thought of as a stand in for the calling code (clients); if a client can't directly access it, the tests shouldn't as well. The goal is to test the externally visible behaviors of the component under test. Commented May 31 at 12:29

2 Answers 2

8

You are testing the wrong thing. You are testing the implementation. That is pointless, you will just write the reverse code of the code you have already written. If you mistakenly called "Left" instead of "Right" you are setting yourself up to just repeat the mistake in your test, having a green test and still a bad result.

You need to test the public interface against the requirements.

What you should test is whether a tree of a certain kind (lets say with left branches) can find a specific leaf. For example, setting up a tree of [3,1,5] should be able to find "1". And it should be able to find "3". And it should be able to find "5". And it should not find "8". Then the tree is working. Whether it calls "Left" is completely irrelevant.

Your public interface should comply with the requirements. Your private implementation details should not comply with anything. They are private. They can change at any time. The whole point of those tests is to enable you to change them at any time, without changing the tests. To be confident, that your refactoring worked. If you need to change the tests everytime you change the code, the whole point of having tests is lost.

5
  • The thing is that I started writing tests that way. But the tests became harder to read because the setup for them was more complex when I had to construct the tree using insert. Using a moq made it far easier to verify that what I was doing was right. And, as I said, this is a toy problem. The real question is about whether - when you've got a public getter and a private setter - there's a better way than I came up with to set the value for to test another piece of the code.
    – mike1952
    Commented May 31 at 13:09
  • 4
    Quite simply, I do not believe that. As someone who has no idea of your library, using your public interface will always be easier to read and understand than mocks referencing your private members and their behaviour. Maybe it is easier to read now, but just wait 6 months and read it again. Or wait until someone else from your team modified it. Or you have to read somebody else's code.
    – nvoigt
    Commented May 31 at 13:13
  • Note that the property has a public get. If it didn't then I would fully agree with you - messing with fully private stuff is bad news. But testing the interaction between a public method and a publicly gettable variable isn't an unreasonable thing, is it?
    – mike1952
    Commented May 31 at 13:22
  • @mike1952 Sure... via the public getter. If that is part of the public interface, sure, test it. No mocks of any kind needed.
    – nvoigt
    Commented May 31 at 13:30
  • 1
    @mike1952 If the tests are harder to read because they are using your public interface then just imagine what that implies for real code that actually uses the public interface properly. Whatever you may be doing in the tests, you can assume that any real-world code is likely to end up at least as complex. Now, that may not be true and the interface might be just fine, but if that interface is fine, then it means that any tests which use that interface for test setup are also fine. Indeed, a benefit of writing tests is to catch "usability smells" in your public interface. Commented May 31 at 15:37
4

please grant the assumption that putting mocks on left and right would be useful

Sorry, but no: tests should test behaviour, not implementation. If what you care about is that searching the tree should have a complexity of O(height), then test for precisely that, not anything related to how your current code happens to implement things (and Left and Right absolutely are implementation details).

2
  • I understand that. And in the case of a binary tree, it seems to me that searching for lower values down the left side and not the right is correct behaviour that needs to be validated. Additionally, I have no idea how I would match up that behaviour test with the TDD philosophy of writing simple tests that can be passed with simple changes to the code. Wouldn't I have to have a fully functional tree to test any part of something that complex?
    – mike1952
    Commented May 31 at 12:42
  • 2
    The first test you write shouldn't be "searching the tree has a complexity of O(height)", it should be something more like "a tree can find the value I just inserted". Commented May 31 at 12:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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