Hey Peter

Thanks for the comments… I will look at the tests once again with your comments ;-)

And yes, you have hit the nail on the head about the leaks… Test code is very much app code — we should take as much care of it as the rest … i.e. it should be refactored and polished often to keep it clean and glowing….

About the leaks, what about using boost? Especially Boost smart pointers… Boost is a brilliant library written by C++ gurus so we could as well benefit from it…
I have some experience of using boost … so we can as well use it in the test code..

—- cheers atul

Peter Dimov wrote:

Hi Atul,

That’s a neat trick to derive a class Bye in order to make all protected functions public and be able to test them :-). Is that what you were referring to by “circumventing around private declarations”? You can actually only change the visibility of protected declarations. Is there really a trick for private declarations?

I think one should be careful when testing Bye::copy() since the function returns a pointer to an object created on the heap. If it is not deleted we’ll have memory leaks in the test application. I think it would be quite wise to run Valgrind on the test app first and then on Calitko. If a unit tests leaks memory, we would be able to much easier test and solve the leak. Perfectly, if we have good test coverage and have no leaks during testing, then we should not have leaks in Calitko :-).

Some comments about some of the tests:

testInvalidate() – the header is invalidated when a field is modified by calling a setter, or when the payload is invalidated (a payload field is modified by calling a setter). The reference counted shared data is then copied and are flags being set, so that next time rawData() or rawPayload() are called the raw buffers be rebuilt. If a packet is merely forwarded by our node, only the header will be modified and rebuilt but not the payload. It is a bad idea to rebuild the payloads of transiting packets.

testPrepareReadPayloadChecks() - the raw format of the packet payload is: 2 bytes status code in little endian followed by a zero terminated string. So your first check raw payload “H” fails because a correct packet would contain at least 2 bytes for code and a zero. The second one works but the message is not “Hello”. I think you should also check whether the code gets parsed correctly as well.

testPrepareWritePayload() you might also set the status code an compare the parsed data with the binary data.

testReadHeader() – The last assertion fails because QDataStream uses by default big endian and Gnutella packets use little endian. The bytes in the read payloadLength are swaped.

This last test shows an interesting concept – writing test code that can be run on different data. That’ll be pretty important and QTestLib has this concept, although I don’t feel their realization very practical. For each unit test function you can declare a _data function which fills in a table where rows are tests and columns are test data. I couldn’t find anything like that in CppUnit. Does anybody have any idea whether something like that might work in CppUnit. It would also be quite interesting to be able to read the data from files. That would make the source code more clear maybe.

Regards,

Peter

atul wrote:Dear all

Here are some of the test cases I came up with…

Bye::testReadHeader() is pretty interesting and I believe I learned a lot by writing and making it work ;-)
I am looking into why the last test assertion is failing ;-)
I have taken the technique of circumventing around private declarations from Mike Feather’s book …

— cheers atul