Re: [Libreoffice] [PATCH] EasyHack 39625 - Make existing cppunittests work

2011-11-22 Thread Stephan Bergmann

On 11/20/2011 11:32 AM, Justin Harding wrote:

Hi - I'm new to LibreOffice development - I've taken a look at EasyHack
39625 - Make existing cppunittests work - and followed the suggestions
in the bug report. I'm not completely sure that this is all that is
required - I don't know if all the unit tests are being run. I think it
will take me a bit longer to get familiar with the way the unit tests
are invoked from the build system.
Here is my patch, I would be pleased to receive any feedback.


Hi Justin,

Thanks a lot for working on this.  Unfortunately, you sent your patch 
inline within your mail, which makes it hard to extract it (line breaks 
added by mail software, etc.)---could you please re-send it as an 
attachment?


A few notes:

- Each file that includes a cppunit header needs to include 
sal/precppunit.hxx first, see 
http://wiki.documentfoundation.org/Development/Unit_Tests and 
http://lists.freedesktop.org/archives/libreoffice/2011-September/018152.html. 
 It would be great if you could modify your patch accordingly.  (I also 
note that sal/qa/rtl/bootstrap/rtl_Bootstrap.cxx includes the cppunit 
headers twice in a row.)


- These unit tests are currently not run.  How to enable them depends on 
whether the respective module has already been changed to gbuild or 
still uses dmake.  For sal, for example, (which still uses dmake), each 
test's directory would need to be added to sal/prj/build.lst.  The 
necessary lines can be modelled after the lines for already enabled tests,


  sa sal\qa\... nmake - all sa_qa_... sa_cppunittester 
sa_util_saltextenc NULL


Then, executing build in sal should include those tests.

- For many of those tests, it is unclear whether they build at all, and, 
if they do build, whether they work reliably (i.e., succeed each time 
they are run; work not only on one platform).  Especially for the ones 
in sal, I assume some are rather rotten (won't even compile) and/or do 
not reliably work on all platforms.  If you like, it would be great if 
you could try to enable some of the sal tests and see if they compile at 
all.  If they fail badly, its probably better to ditch them than to 
invest too much time trying to get them working.  Then, if there is a 
bunch of working ones left, we can commit them and see if the various 
tinderboxes like them, too (i.e., if they work reliably cross-platform).


Let me know if that sounds like a plan to you.

Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [PATCH] EasyHack 39625 - Make existing cppunittests work

2011-11-22 Thread Justin Harding
Thanks Stephan

This is a big help. I'll revisit my patch with this new information and get
some of the tests enabled so that I can see what happens. I'll repost my
patch after doing that and making the changes you have suggested.

Cheers
Justin


Hi Justin,

 Thanks a lot for working on this.  Unfortunately, you sent your patch
 inline within your mail, which makes it hard to extract it (line breaks
 added by mail software, etc.)---could you please re-send it as an
 attachment?

 A few notes:

 - Each file that includes a cppunit header needs to include
 sal/precppunit.hxx first, see http://wiki.**documentfoundation.org/**
 Development/Unit_Testshttp://wiki.documentfoundation.org/Development/Unit_Tests
 and http://lists.freedesktop.org/**archives/libreoffice/2011-**
 September/018152.htmlhttp://lists.freedesktop.org/archives/libreoffice/2011-September/018152.html.
  It would be great if you could modify your patch accordingly.  (I also
 note that sal/qa/rtl/bootstrap/rtl_**Bootstrap.cxx includes the cppunit
 headers twice in a row.)

 - These unit tests are currently not run.  How to enable them depends on
 whether the respective module has already been changed to gbuild or still
 uses dmake.  For sal, for example, (which still uses dmake), each test's
 directory would need to be added to sal/prj/build.lst.  The necessary lines
 can be modelled after the lines for already enabled tests,

  sa sal\qa\... nmake - all sa_qa_... sa_cppunittester sa_util_saltextenc
 NULL

 Then, executing build in sal should include those tests.

 - For many of those tests, it is unclear whether they build at all, and,
 if they do build, whether they work reliably (i.e., succeed each time they
 are run; work not only on one platform).  Especially for the ones in sal, I
 assume some are rather rotten (won't even compile) and/or do not reliably
 work on all platforms.  If you like, it would be great if you could try to
 enable some of the sal tests and see if they compile at all.  If they fail
 badly, its probably better to ditch them than to invest too much time
 trying to get them working.  Then, if there is a bunch of working ones
 left, we can commit them and see if the various tinderboxes like them, too
 (i.e., if they work reliably cross-platform).

 Let me know if that sounds like a plan to you.

 Stephan

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice