Hi Atul,

Here are some comments about ByeTest.

testConstCastFrom() doesn’t delete p; (the copy of the packet object).

Modified testReadHeader() to use the correct byte order out.setByteOrder (QDataStream::LittleEndian); I also uncomment the last line of the test, which verifies the parsed value of payload length.

I’m not sure whether testInvalidPayloadCheck() correctly asserts failure. To be honest, I didn’t try it out but here is what I see:

- You need a quint16 in the beginning of the array and by using sprintf() you are writing a string instead. I think that doing here the same you do in testReadPayload() would be good refactoring ;-).

- By using the ctor of QByteArray which takes a char* as an argument, a zero terminated string is assumed and the zero is NOT included in the QByteArray. The ending zero is needed however for a correct bye packet and that would be why prepareReadPayload() fails in testInvalidPayLoadCheck(). It’s strange though it does not fail in testValidPayloadCheck(). Could you investigate this further?

In testInvalidateHdrRefCntX() I don’t understand why you do
QBasicAtomic* pRef = &m_pMyClassBye->ref();
If you want to increase the reference count, I think you should do
QBasicAtomic* pRef = &m_pMyClassBye->ref().ref();
Just like you’ve done in testInvalidateHdrFields().

I think that especially for packets it would make sense to test parsing on multiple packets. We could let Calitko run for a while and dump packets of different types to different files. Then we could choose some of the dumped packet and create test data (test input and expected output). I think testing like this will be much better. I don’t know whether our tests would be extensive enough otherwise.

I think that was all. I’m attaching the file with my changes (described in the beginning of the post).

Regards,

Peter

atul wrote:

And a little bit removed ( some test that did not make much sense ;-)

Also I have put in Peter’s changes…

Here it goes…

— cheers atul