[Kmymoney-devel] Review Request: Fix some unit test linking issues on msvc

[Alvaro Soliverez]

> On 2010-07-03 18:41:08, Fernando Vilas wrote:
> > If it is possible to do a #ifdef KDE4_BUILD_TESTS / #endif or similar around the friend declaration, I would prefer that. If not, then it looks good to me. In an effort to reduce the friend declarations, we should probably have a more in depth look at the tests after this is committed.
> 
> Alvaro Soliverez wrote:
>     I agree with  Fernando. If we could add  #ifdefs so that this is called only when tests are built, it's ok.
>     
>     Otherwise, we should look more in depth what happens when tests aren't built.
> 
> Cristian Onet wrote:
>     I also agree. But this suggestion gave me a better idea. It would be more elegant to solve this by adding a MYMONEY_UNIT_TESTABLE (or whatever better named)macro to the MyMoney* objects which would be defined as empty but in the unit tests where, before including the mymoney*.h file MYMONEY_UNIT_TESTABLE would be defined as the needed friend class declarations. This way we get a bit of the old solution into the new one. What do you think? I still need to update the patch... and waiting for the opinion of Thomas.

It looks good. The developer's handbook should be updated to let others know about it and explain why this is needed, because it is not immediately apparent.


- Alvaro


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4520/#review6352
-----------------------------------------------------------


On 2010-07-03 18:10:15, Cristian Onet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4520/
> -----------------------------------------------------------
> 
> (Updated 2010-07-03 18:10:15)
> 
> 
> Review request for kmymoney.
> 
> 
> Summary
> -------
> 
> Replace the '#define' of all access qualifiers to public with proper 'friend class' declarations for unit tests. I know that this replaces the non-intrusive way of writing unit tests with a relatively intrusive way (if you can call the friend class declaration as such) but this should be done to fix the following linkage problem. Some compilers (like msvc) include the access qualifiers in the mangled method name so when we try to link the unit test we will fail because we are looking for a public method in a library where that method is actually private or protected. It took me a while to spot this error and I think we should really fix it. I don't see a problem with the fact that the unit test class is a friend of the real class. Any thoughts? Another solution would mean recompiling the mymoney cpp files and linking them in the test executable but I don't like this solution.
> 
> 
> Diffs
> -----
> 
>   trunk/extragear/office/kmymoney/kmymoney/converter/convertertest.cpp 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyaccount.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyaccounttest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyaccounttest.cpp 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyexceptiontest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyfile.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyfiletest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyfinancialcalculator.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyfinancialcalculatortest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyforecast.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyforecasttest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyinstitutiontest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneykeyvaluecontainer.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneykeyvaluecontainertest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneymoney.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneymoneytest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyobject.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyobjecttest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneypayeetest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneypricetest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyscheduled.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneyscheduletest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneysecuritytest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneysplittest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneytransaction.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/mymoneytransactiontest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/storage/mymoneydatabasemgr.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/storage/mymoneydatabasemgrtest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/storage/mymoneymaptest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/storage/mymoneyseqaccessmgr.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/storage/mymoneyseqaccessmgrtest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/mymoney/storage/mymoneystoragesql.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/reports/pivotgrid.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/reports/pivotgridtest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/reports/pivotgridtest.cpp 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/reports/pivottable.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/reports/pivottabletest.h 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/reports/pivottabletest.cpp 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/reports/querytabletest.cpp 1145134 
>   trunk/extragear/office/kmymoney/kmymoney/reports/reportstestcommon.cpp 1145134 
> 
> Diff: http://reviewboard.kde.org/r/4520/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Cristian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kmymoney-devel/attachments/20100705/9a08bebb/attachment-0001.htm