views:

687

answers:

6

Is there any good way to unit test destructors? Like say I have a class like this (contrived) example:

class X
{
private:
    int *x;

public:
    X()
    {
         x = new int;
    }

    ~X()
    {
         delete x;
    }

    int *getX() {return x;}
    const int *getX() const {return x;}
};

Is there any good way to unit test this to make sure x gets deleted without cluttering up my hpp file with #ifdef TESTs or breaking encapsulation? The main problem that I'm seeing is that it's difficult to tell if x really got deleted, especially because the object is out of scope at the time the destructor is called.

A: 

Some compilers will overwrite deleted memory with a known pattern in debug mode to help detect access to dangling pointers. I know Visual C++ used to use 0xDD, but I haven't used it in a while.

In your test case, you can store a copy of x, let it go out of scope and make sure that *x == 0xDDDDDDDD:

void testDestructor()
{
    int *my_x;
    {
        X object;
        my_x = object.getX();
    }
    CPPUNIT_ASSERT( *my_x == 0xDDDDDDDD );
}
Matthew Crumley
I despise this idea. You cant unit test code with optimisations enabled or if your compiler changes
1800 INFORMATION
I agree that it's not great for the reasons you mentioned, and you can't access private pointers. Generally, I think your idea (and onebyone's) is better, but it's not always trivial to use a mock object.
Matthew Crumley
A: 

I tend to go with a "By any means necessary" approach to testing. If it needs a test, I am willing to leak abstractions, break encapsulation, and hack... because tested code is better than pretty code. I will often name the methods that break this up something like VaildateForTesting or OverrideForTesting to make it clear that the breach of encapsulation is meant for testing only.

I don't know of any other way to do this in C++ than having the destructor call into a singleton to register that it has been destroyed. I have come up with a method for doing something similar to this in C# using a weak reference (I don't violate encapsulation or abstractions with this approach). I am not creative enough to come up with an analogy to C++, but YOU might be. If it helps, great, if not, sorry.

http://houseofbilz.com/archive/2008/11/11/writing-tests-to-catch-memory-leaks-in-.net.aspx

Brian Genisio
A: 

Not a platform agnostic suggestion, but in the past I have made calls into the CRT's heap checking functions during unit testing, to verify that there is no more memory allocated at the end of a test (or perhaps a whole set of tests) than the start. You might also be able to do something similar with your platform's instrumentation, to check on handle-counts, etc.

Will Dean
+1  A: 

In the example, define and instrument your own global new and delete.

To avoid #ifdefs, I've make test classes friends. You can set/save/get state as required to verify the results of a call.

Tony Lee
This idea doesn't scale. What if you need to test lots of different classes, are you going to implement different versions of your global new and delete? What if you are unit testing code that already replaces ::new and ::delete?
1800 INFORMATION
I don't know what straw man your imagining. The instrumentation is intended to answer questions like "is x still allocated"? Other answers here leverage a platform specific version of memory instrumentation rather than taking control yourself.
Tony Lee
Your design is a poor one. Why should you have to modify your classes under test in order that your test harness can work with it? If you make your classes tesable from the start (see my example), you do not need to resort to such things
1800 INFORMATION
Your still imagining something I didn't say.
Tony Lee
You said "I've make test classes friends" - this is modifying the class under test in order to make your test harness work with it. You shouldn't have to do this.
1800 INFORMATION
+2  A: 

There may be something to be said for dependency injection. Instead of creating an object (in this case an int, but in a non-contrived case more likely a user-defined type) in its constructor, the object is passed as a parameter to the constructor. If the object is created later, then a factory is passed to the constructor of X.

Then when you're unit testing, you pass in a mock object (or a mock factory which creates mock objects), and the destructor records the fact that it has been called. The test fails if it isn't.

Of course you can't mock (or otherwise replace) a builtin type, so in this particular case it's no good, but if you define the object/factory with an interface then you can.

Checking for memory leaks in unit tests can often be done at a higher level, as others have said. But that only checks that a destructor was called, it doesn't prove that the right destructor was called. So it wouldn't e.g. catch a missing "virtual" declaration on the destructor of the type of the x member (again, not relevant if it's just an int).

Steve Jessop
Precisely what I was saying
1800 INFORMATION
Yep, and at just about exactly the same time. I guess I won because you stopped to write a code sample :-)
Steve Jessop
+2  A: 

I think your problem is that your current example isn't testable. Since you want to know if x was deleted, you really need to be able to replace x with a mock. This is probably a bit OTT for an int but I guess in your real example you have some other class. To make it testable, the X constructor needs to ask for the object implementing the int interface:

template<class T>
class X
{
  T *x;
  public:
  X(T* inx)
    : x(inx)
  {
  }

  // etc
};

Now it becomes simple to mock in the value for x, and the mock can handle checking for correct destruction.

Please pay no attention to the people who say you should break encapsulation or resort to horrible hacks in order to result in testable code. While it is true that tested code is better than untested code, testable code is the best of all and it always results in clearer code with fewer hacks and lower coupling.

1800 INFORMATION
It's the only way to do it for an int, but a class template isn't a trivial replacement for a class. Using templates everywhere usually limits your options for DLLs, reducing header dependency, etc. All depends on compiler of course: I'm told some support better template linking than others.
Steve Jessop
Mind you, maybe I'm just being grumpy because I said you can't mock a builtin type, which as you've pointed out is false, because you can do it in this way via templates and a class that implements the whole int interface (not trivial, but you only have to do it once. Don't forget numeric_limits).
Steve Jessop
Well you wouldn't have to make it templatised. If you removed the template and changed T to int everywhere in my example it would still be testable
1800 INFORMATION