Bug 97283 - merge unit tests in sal module
Summary: merge unit tests in sal module
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.4.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup, topicQA
Depends on:
Blocks:
 
Reported: 2016-01-20 18:50 UTC by David Tardon
Modified: 2017-02-14 08:57 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Tardon 2016-01-20 18:50:39 UTC
There are 33 separate test libraries in sal module. These should be merged into 3: one for each of osl, rtl and sal.

Most of the tests have the right category already in the name, e.g., CppunitTest_sal_osl_file should be merged into a (new) CppunitTest_sal_osl. For these few that do not follow that naming scheme, check the source files for what class they are testing (sal::Foo, rtl_Foo,...) and categorize them by that. It might be a good idea to move the sources into the right subdir of sal/qa as well, if they are not already there.

The easiest way is to rename one of the makefiles of the selected group (do not forget to also change the name used inside the makefile and in Module_sal.mk) and copy source files (and other extra stuff: used libraries, extra dependencies, etc.) from the other makefiles to it. Then remove CPPUNIT_PLUGIN_IMPLEMENT() from all source files from the group but one (you will get linker error if you do not do this). Then remove the now unnecessary makefiles.

It would be best to do this in 3 separate patches, one for each group.
Comment 1 Matúš Kukan 2016-02-04 12:41:23 UTC
I think https://gerrit.libreoffice.org/#/c/21959/ could serve as an example commit for this easy hack, although I had to do some specific changes in there.
Comment 2 Robinson Tryon (qubit) 2016-02-18 14:51:25 UTC Comment hidden (obsolete)
Comment 3 jani 2016-02-21 12:56:30 UTC
There are no code pointers, which are required for a easyHack
Comment 4 jani 2016-04-22 08:19:14 UTC
git log -p -1 aa20acb87d48b4dcda337ab08c5d8a76a400bacb

provides a link to the patch, also please look at comment #1
Comment 5 jani 2016-04-26 09:00:38 UTC
Not my scope
Comment 6 Commit Notification 2016-12-23 10:20:08 UTC
Matúš Kukan committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8e743e46781b252f41227d809dec43d3c4c1d01f

tdf#97283: Merge sal osl tests to one makefile

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2016-12-23 11:20:08 UTC
Matúš Kukan committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=597d178c2b516ab0cff4e4f86246d47ce9700f18

tdf#97283: Merge sal rtl tests to one makefile

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.