8

Assuming I have the following (over-simplified) class structure:

class Base
{
  public:
    Base(int valueForFoo) : foo(valueForFoo) { };
    virtual ~Base() = 0;
    int doThings() { return foo; };
    int doOtherThings() { return 42; };

  protected:
    int foo;
}

class BarDerived : public Base
{
  public:
    BarDerived() : Base(12) { };
    ~BarDerived() { };
    int doBarThings() { return foo + 1; };
}

class BazDerived : public Base
{
  public:
    BazDerived() : Base(25) { };
    ~BazDerived() { };
    int doBazThings() { return 2 * foo; };
}

As you can see, the doThings function in the Base class returns different results in each Derived class due to the different values of foo, while the doOtherThings function behaves identically across all classes.

When I wish to implement unit tests for these classes, the handling of doThings, doBarThings / doBazThings is clear to me - they need to be covered for each derived class. But how should doOtherThings be handled? Is it good practice to essentially duplicate the test case in both derived classes? The issue becomes worse if there are half a dozen functions like doOtherThings, and more Derived classes.

  • Are you sure only the dtor should be virtual? – Deduplicator Apr 17 '18 at 10:02
  • @Deduplicator For the simplicity of the example, yes. The Base class is/should be abstract, with the derived classes providing additional functionality or specialized implementations. BarDerived and Base may have once been the same class. When a similar functionality was to be added the common part was moved into the Base class, with different specializations implemented in each Derived class. – CharonX Apr 17 '18 at 10:20
  • In a less abstract example, imagive a class that writes standard-conforming HTML, but then it was decided that it should also be possible write HTML optimized for <Browser> besides the "vanilla" implementation (which does a few things differently, and does not support all functions provided by the standard). (Note: I am relieved to say that the real classes that lead to this questions have nothing to do with writing "browser-optimized" HTML) – CharonX Apr 17 '18 at 10:23
3

In your tests for BarDerived you want to prove that all (public) methods of BarDerived work correctly (for the situations you have tested). Similarly for BazDerived.

The fact that some of the methods are implemented in a base class does not change this testing goal for BarDerived and BazDerived. That leads to the conclusion that Base::doOtherThings should be tested both in the context of BarDerived and BazDerived and that you get very similar tests for that function.

The advantage of testing doOtherThings for each derived class is that if the requirements for BarDerived change such that BarDerived::doOtherThings must return 24, then the test failure in the BazDerived testcase tells you that you might be breaking the requirements of another class.

  • 2
    And there's nothing stopping you from reducing duplication by factoring the common test-code into a single function which is called from both separate test cases. – Sean Burton Apr 18 '18 at 11:28
  • 1
    IMHO this whole answer will only become a good one by adding clearly @SeanBurton's comment, otherwise it looks like a blatant violation of the DRY principle to me. – Doc Brown Apr 20 '18 at 15:06
3

But how should doOtherThings be handled? Is it good practice to essentially duplicate the test case in both derived classes?

I would normally expect Base to have its own specification, which you can verify for any conforming implementation, including the derived classes.

void verifyBaseCompliance(const Base & systemUnderTest) {
    // checks that systemUnderTest conforms to the Base API
    // specification
}

void testBase () { verifyBaseCompliance(new Base()); }
void testBar () { verifyBaseCompliance(new BarDerived()); }
void testBaz () { verifyBaseCompliance(new BazDerived()); }
1

You have a conflict here.

You want to test the return value of doThings(), which relies on a literal (const value).

Any test you write for this is inherently going to boil down testing a const value, which is nonsensical.


To show you a more sensical example (I'm quicker with C#, but the principle is the same)

public class TriplesYourInput : Base
{
    public TriplesYourInput(int input)
    {
        this.foo = 3 * input;
    }
}

This class can be tested meaningfully:

var inputValue = 123;

var expectedOutputValue = inputValue * 3;
var receivedOutputValue = new TriplesYourInput(inputValue).doThings();

Assert.AreEqual(receivedOutputValue, expectedOutputValue);

This makes more sense to test. Its output is based on the input that you chose to give it. In such a case, you can give a class an arbitrarily chosen input, observe its output, and test if it matches your expectations.

Some examples of this testing principle. Notice that my examples always have direct control over what the input of the testable method is.

  • Test if GetFirstLetterOfString() return "F" when I input "Flater".
  • Test if CountLettersInString() returns 6 when I input "Flater".
  • Test if ParseStringThatBeginsWithAnA() returns an exception when I input "Flater".

All of these tests can input whatever value they want, as long as their expectations are in line with what they're inputting.

But if your output is decided by a constant value, then you're going to have to create a constant expectation, and then test if the first matches the second. Which is silly, this is either always or never going to pass; neither of which is a meaningful result.

Some examples of this testing principle. Notice that these examples have no control over at least one of the values that are being compared.

  • Test if Math.Pi == 3.1415...
  • Test if MyApplication.ThisConstValue == 123

These tests for one particular value. If you change this value, your tests will fail. In essence, you're not testing whether your logic works for any valid input, you're simply testing if someone is able to accurately predict an outcome which they have no control over.

That's essentially testing the test writer's knowledge of the business logic. It's not testing the code, but the writer themselves.


Reverting back to your example:

class BarDerived : public Base
{
  public:
    BarDerived() : Base(12) { };
    ~BarDerived() { };
    int doBarThings() { return foo + 1; };
}

Why does BarDerived always have a foo equal to 12? What is the meaning of this?

And given that you've already decided this, what are you trying to gain by writing a test that confirms that BarDerived always has a foo equal to 12?

This gets even worse if you start factoring in that doThings() can be overridden in a derived class. Imagine if AnotherDerived were to override doThings() so that it always returns foo * 2. Now, you're going to have a class which is hardcoded as Base(12), whose doThings() value is 24. While technically testable, it is devoid of any contextual meaning. The test is not comprehensible.

I genuinely can't think of a reason to use this hardcoded value approach. Even if there is a valid use case, I don't understand why you're trying to write a test to confirm this hardcoded value. There is nothing to gain by testing if a constant value equals the same constant value.

Any test failure inherently proves that the test is wrong. There is no outcome where a test failure proves that the business logic is wrong. You're effectively unable to confirm that which tests are created to confirm in the first place.

The issue has nothing to do with inheritance, in case you were wondering. You just happen to have used a const value in the base class constructor, but you could've used this const value anywhere else and then it wouldn't be related to an inherited class.


Edit

There are cases where hardcoded values are not a problem. (again, sorry for the C# syntax but the principle is still the same)

public class Base
{
    public int MultiplyFactor;
    protected int InitialValue;

    public Base(int value, int factor)
    {
        this.InitialValue = value;
        this.MultiplyFactor= factor;
    }

    public int GetMultipliedValue()
    {
         return this.InitialValue * this.MultiplyFactor;
    }
}

public class DoublesYourNumber : Base
{
    public DoublesYourNumber(int value) :  base(value, 2) {}
}

public class TriplesYourNumber : Base
{
    public TriplesYourNumber(int value) : base(value, 3) {}
}

While the constant value (2/3) is still influencing the output value of GetMultipliedValue(), the consumer of your class still has control over it too!
In this example, meaningful tests can still be written:

var inputValue = 123;

var expectedDoubledOutputValue = inputValue * 2;
var receivedDoubledOutputValue = new DoublesYourNumber(inputValue).GetMultipliedValue();

Assert.AreEqual(expectedDoubledOutputValue , receivedDoubledOutputValue);

var expectedTripledOutputValue = inputValue * 3;
var receivedTripledOutputValue = new TriplesYourNumber(inputValue).GetMultipliedValue();

Assert.AreEqual(expectedTripledOutputValue , receivedTripledOutputValue);
  • Technically, we're still writing a test that checks if the const in base(value, 2) matches the const in inputValue * 2.
  • However, we are at the same time also testing that this class is correctly multiplying any given value by this predetermined factor.

The first bullet point isn't relevant to test. The second is!

  • As already mentioned, this is a rather over-simplified class structure. That said, let me refer to the perhaps less abstract example: Imagine a HTML-writing class. We all know that standard < and > braces that encapsulate the HTML tags. Unfortunately (due to insanity) a "specialized" browser uses ![{ and }]! instead, and Intitech decides that you need to support this browser in your HTML writer. For example you have the getHeaderStart() and getHeaderEnd() function that - until now - returned <HEADER> and <\HEADER>. – CharonX Apr 17 '18 at 13:31
  • @CharonX You would still need to make the tag type (whether through an enum, or two string properties for the used tags, or anything equivalent) publically configurable in order to meaningfully test if the class correctly uses the tags. If you don't, your tests will be littered with undocumented constant values that are needed to get the test to work. – Flater Apr 17 '18 at 13:34
  • You could alter the class and functions by simply copy-pasting everything - once with <, the other with ![{. But that would be rather bad. So you have each function insert the character(s) set in a variable in the places < and > would go, and create derived classes that - depending if they are standard-conform or insane, provide the appropiate <HEADER> or ![{HEADER}]! Neither getHeaderStart() function is dependant on input and relies on the constant set during the construction of the derived class. Still, I would feel uneasy if you told me that testing them makes no sense... – CharonX Apr 17 '18 at 13:35
  • @CharonX That's not the point. The point is that your output (which obviously decides if the test passes) is based on a value that the test itself has no control over. Otherwise, you should be testing an output value which does not rely on this hidden const. Your example code is testing this const value, not anything else. That is either incorrect, or oversimplified to a point of not showing the actual purpose of the output. – Flater Apr 17 '18 at 13:37
  • Does testing getHeaderStart() make sense or not? – CharonX Apr 17 '18 at 13:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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