Hi Atul,

I just pushed your latest patch in revno 89 in your public branch.

In one of your previous patches you added:

#ifndef UNIT_TEST
qDebug() < < "dict: key value not ok";
#endif

Initially, when inspecting the patch, I though that’s fine and will work but I noticed today that the warning is still outputted. It’s actually quite logical that the above won’t work because the ifndef is in a cpp file which is compiled into the calitko library. The library is not compiled with UNIT_TEST defined. I think the solution would be to not use qDebug() but instead use some logging framework, which we could configure when and what messages to output. Do you have any experience with anything like this?

Btw using these small helper function in the test classes seems to me like a very clever idea and it does result is simpler code! That’s pretty much the same kind of refactoring one would do for app code and perfectly supports the point you made earlier ;-).

About hasReadPastEnd() and hasReadAll() maybe we can add a third one like hasReadCorrectly(). We could eventually drop the first two. The questions are actually different. The first one tells whether more bytes have been written than actually in the buffer, the second says whether at least all bytes have been read, the third one would say whether exactly all bytes have been read. I think I added the first two functions in the first place because I needed to know how long I am allowed to try reading (like while (!reader.hasReadAll())) and then verifying whether exactly all bytes were written (like !reader.hasReadPastEnd();). Do you have some concrete suggestion?

Regards,

Peter

atul wrote:Hi all

I think this version of TestHttpHeader is much better than the previous one. All the test code is much simplified and just contain the assertions… Fixtures help a lot here.

I removed some duplication in BDecoderTest too… I removed the length typeid and instead have put in the testing of the pointer value… so the typeid tests are still there but the type is instead checked by the dynamic_cast.

It is amazing how removing two lines of duplication simplifies the code a lot ;-) I still don’t like that separate hasReadAll() and hasReadPassEnd() — shouldn’t it be hasReadPastEnd ;-)

Let me know what you all think… and happy weekend to you all ;-)
—- cheerio atul