Bug Hunting Session
Bug 39625 - make sal/qa/systools/test_comtools.cxx work (Windows only) ....
Summary: make sal/qa/systools/test_comtools.cxx work (Windows only) ....
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other Windows (All)
: medium normal
Assignee: Andrés Maldonado
URL:
Whiteboard: target:6.3.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicDebug
Depends on: 84237
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-28 08:23 UTC by Björn Michaelsen
Modified: 2019-02-22 12:41 UTC (History)
7 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 Björn Michaelsen 2011-07-28 08:23:02 UTC
=== Make existing tests work ===

'''Background:''' A lot of existing CppUnit tests (they are usually placed in test or qa subdir) are currently broken because they use stuff from (already removed) module testshl2. The goal is to change them to CppUnit test plugins and use cppunittester for running them. Use http://cgit.freedesktop.org/libreoffice/libs-gui/commit/?id=53cb376d74b108d783f4a40c8e23ce54b5f4fd90 as an example. Use [http://opengrok.go-oo.org/search?q=testshl&project=artwork&project=base&project=bootstrap&project=calc&project=components&project=extensions&project=extras&project=filters&project=help&project=impress&project=libs-core&project=libs-extern&project=libs-extern-sys&project=libs-gui&project=postprocess&project=sdk&project=testing&project=ure&project=writer&defs=&refs=&path=&hist= this link] to see the broken tests.

'''Skills: ''' build, (maybe) CppUnit
Comment 1 Björn Michaelsen 2011-11-15 15:30:23 UTC
@stephan: Isnt this resolved by now?
Comment 2 Stephan Bergmann 2011-11-15 23:32:08 UTC
no, there's still a number of places that mention "testshl"
Comment 3 Caolán McNamara 2011-11-16 02:56:17 UTC
See http://opengrok.libreoffice.org/search?q=testshl&project=core for the current list of uses of the archaic and removed testshl. Not too many of them left.

http://cgit.freedesktop.org/libreoffice/libs-gui/commit/?id=53cb376d74b108d783f4a40c8e23ce54b5f4fd90 can still serve as a template for conversion
Comment 4 Justin Harding 2011-11-20 02:33:41 UTC
Taken Justin Harding 2011-11-20, expect to be finished by 2011-11-27
Comment 5 Florian Reisinger 2012-05-18 09:05:44 UTC
Deteted "Easyhack" from summary
Comment 6 Not Assigned 2012-08-25 01:22:25 UTC
Radu Ioan committed a patch related to this issue.
It has been pushed to "master":

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

fdo#39625 Make existing cppunittests work
Comment 7 Not Assigned 2012-09-10 20:30:37 UTC
Radu Ioan committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6ffa064e7d8bfcdef815f24e2cfdb791dc960ba2

fdo#39625 Make existing cppunittests work



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 8 Not Assigned 2012-10-10 08:27:57 UTC
Radu Ioan committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0ea421781b16b449045e66c38007374ab2f2c509

fdo#39625 Make existing cppunittests work



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 9 Commit Notification 2013-09-11 11:50:49 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6e066e4abe7cac72d31323859e37bfa3e0af82b2

sal: fdo#39625 Make existing cppunittests work



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 10 Commit Notification 2013-09-11 16:02:37 UTC
Jelle van der Waa committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=550b275e634a5a16bf55cbb716f21608fa211e3e

fdo#39625 Make existing cppunittests work



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 11 Björn Michaelsen 2013-10-04 18:47:49 UTC
adding LibreOffice developer list as CC to unresolved EasyHacks for better visibility.

see e.g. http://nabble.documentfoundation.org/minutes-of-ESC-call-td4076214.html for details
Comment 12 Tobias Madl 2014-09-02 11:57:50 UTC
He i'd like to work on this bug. Its the first time I'm bug fixing on libre office, feel free to give me advices or suggestions :).
Comment 13 Tobias Madl 2014-09-02 13:52:54 UTC
Got a short question, one of the files doesn't have a makefile (test_comtools.cxx), was this on purpose or should i write one?
Comment 14 Caolán McNamara 2014-09-04 07:47:48 UTC
test_comtools.cxx looks like a "forgotten" test. I see it is windows only, so can only be tested under windows and not the other platforms.

If you're in a position where you can try fixing the test then sure go for it. 
t might turn out that the test doesn't work and that it is not worth fixing, that's a good result too, in which case submitting a patch to delete the broken and not-worth-fixing test is good too.
Comment 15 Commit Notification 2014-09-24 06:40:23 UTC
Tobias Madl committed a patch related to this issue.
It has been pushed to "master":

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

fdo#39625 Make existing CppUnittests work



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 16 Commit Notification 2014-09-24 08:12:03 UTC
Tobias Madl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=18cc5cb2fdb8bca18a6c55d0a165b749f6730420

fdo#39625 Delete unused cppunittests



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 17 Commit Notification 2014-09-25 13:31:16 UTC
Tobias Madl committed a patch related to this issue.
It has been pushed to "master":

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

fdo#39625 Make existing CppUnit tests work



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 18 Tobias Madl 2014-09-26 07:16:45 UTC
So for further information, i now removed all occurrence of testshl. But i still can't close the ticket, because on one hand there's the /sal/qa/systools/test_comtools.cxx test, which only runs under windows, so sb with a windows maschine has to fix this. (Suggestion to change the ticket to a windows dependency ticket). On the other hand theres the /unoxml/test/domtest.cxx test, which is running at the moment, but still not with all tests in it. The problem, described in this ticket: Bug 84237 , has to be solved befor this tests can be used, or the tests have to be removed.
Comment 19 David Tardon 2014-10-06 13:59:20 UTC
(In reply to Tobias Madl from comment #18)
> So for further information, i now removed all occurrence of testshl. But i
> still can't close the ticket, because on one hand there's the
> /sal/qa/systools/test_comtools.cxx test, which only runs under windows, so
> sb with a windows maschine has to fix this. (Suggestion to change the ticket
> to a windows dependency ticket).

You could still do that yourself, put the patch to gerrit (with -2 code review so someone does not push it too soon) and ask on IRC to run buildbot on it. Maybe it would work :)
Comment 20 Robinson Tryon (qubit) 2015-12-14 06:31:22 UTC Comment hidden (obsolete)
Comment 21 Robinson Tryon (qubit) 2016-02-18 14:51:53 UTC Comment hidden (obsolete)
Comment 22 Francesco Gradi 2017-11-24 12:34:52 UTC
Hi there, i've got a Windows machine so i'd like to start working on this issue.
Not sure if i can handle it, though. So i'm asking for a bit of initial help:

I see that first part of the problem is the presence of "testshl" in the code (i located it in /core/sal/qa/systools/test_comtools.cxx), but as Tobias said, there is also another issue in /unoxml/test/domtest.cxx . 
So basically the solution of this consists in removing testshl and doing what in the other file?

Sorry for all this questions, i'm still new at this and i'd like to learn more and more on how all of this works.

Thanks to all who will spend a few time helping! :)
Comment 23 Andrés Maldonado 2019-02-21 14:31:43 UTC
Hello,

As my first contribution to LibreOffice core, I made a patch for this bug. You can find it at https://gerrit.libreoffice.org/#/c/68156/

I made a build on Windows with this command:
```
CPPUNIT_TEST_NAME="test_comtools::test_COMReference" /opt/lo/bin/make CppunitTest_sal_comtools
```

There were no errors. Also, I checked `workdir/CppunitTest/sal_comtools.test.log` and all tests passed.
Comment 24 Commit Notification 2019-02-22 12:38:04 UTC
Andrés Maldonado committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/db8a2a567087cb65ca420bfd582ea2c8c24fcd7b%5E%21

tdf#39625 make sal/qa/systools/test_comtools.cxx work with CppUnit

It will be available in 6.3.0.

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

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