Hi Atul,

The tests look quite nice and I find the idea with the template extremely interesting! The template class deserves a separate module and can be used when testing all kind of packets (I think that was your intent as well). This template would expose functions that are extremely useful for testing purposes (e.g. making the read* and write* functions public). I hope I’m not mixing terminology but I think a PacketDriver would be a nice name for the template as it is a kind of a test driver for a Packet derived class. What do you think?

I would suggest that you modify the template so that the overridden (now public) functions simply call the operations of the template base T and not Packet (i.e. T::readHeader (stream);).

Regarding parse(), I wanted to make it private in the new Packet class, but to make testing easier I think it should better be protected. I think that using the preprocessor around private to make the following declarations protected is not a really elegant solution. I would much rather prefer to declare some class of the testing sub-package a friend and give it full control to the internals of the class. What about that packet being actually the packet test driver?

For some cases it would actually be better to declare the temporary objects on the stack because they are needed only in that small context anyway. That will be easier than using auto_ptr or scoped_ptr. For example, in testAssign() p could be declared on the stack.

I also like your idea to give access to the private data! This way the internal state can be checked more easily!

The functions getHeader() and getPayload() are not used anywhere. I guess they are obsolete.

Your comment about LSP is also quite interesting although for our special packet case there is not much to check. The meaning of LSP, at least as I get it, is that the overridden functions in the derived classes should have the same or more relaxed preconditions and the same or stricter postconditions compared with the ones of the interface. Only if these conditions are fulfilled can the derived classes be used interchangeably. Our Packet has very tiny interface with pre and postconditions always true. Nevertheless, I totally agree that it’s nice to run the tests which the base should pass again for each derived class just to make sure that the derived classes break nothing in the internal state of the base. If something like that happened, then probably the class invariant was violated and I think we should assert the class invariant on entering and exiting each method.

I’m quite eager to also see the ref-counting tests!

Regards,

Peter

atul wrote:Dear all,

This is an improved version IMO but not yet complete. I finally managed to get Packet::parse() under the unit test harness..

I have some more tests ( rgd. reference counting ) in my home box… I plan to integrate those with this new version of TestPacket by tomorrow.
I plan to use boost::scoped_ptr to eliminate the memory leaks here too…

Let me know what you all think… Once Packets are under a good unit test harness I plan to put in the changes for BadPacket and refactor the Packet classes ( remove duplicate ref counting etc… )

The template is an attempt so the unit test cases get written by the compiler… We should make sure that the same Packet cases pass when Packet is a base class for Pong as well as for Ping … This is to test for the LSP principle…
I am a bit hazy about this part too ;-) I have attached both files ( with and without template ;-).. I added the template later on and gave it a quick go ;-)
—- cheers atul