Bug 98264 - use correct cppunit assert macros in calc's unit test
Summary: use correct cppunit assert macros in calc's unit test
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Aleksas Pantechovskis
URL:
Whiteboard: target:5.2.0
Keywords: difficultyBeginner, easyHack, skillCpp
Depends on:
Blocks:
 
Reported: 2016-02-29 01:48 UTC by Markus Mohrhard
Modified: 2017-02-14 08:58 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
file containting the clang plugin output (223.49 KB, text/plain)
2016-02-29 01:48 UTC, Markus Mohrhard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Mohrhard 2016-02-29 01:48:13 UTC
Created attachment 123065 [details]
file containting the clang plugin output

Currently many places in sc/qa/unit/ucalc* use CPPUNIT_ASSERT instead of CPPUNIT_ASSERT_EQUAL or CPPUNIT_ASSERT_DOUBLES_EQUAL. Therefore if one of these checks fails during the test run we are not seeing the expected and the actual value.

The task is to replace the places of CPPUNIT_ASSERT that contain a operator== check with CPPUNIT_ASSERT_EQUAL. There are two small problems that make the task a bit more challenging: both parameters to CPPUNIT_ASSERT_EQUAL need to be of the same type so you might need to cast one of the types especially if it is an integral type.

e.g CPPUNIT_ASSERT_EQUAL(size_t(3), aVector.size());

Additionally you should not use CPPUNIT_ASSERT_EQUAL with float or double values. So if one of the types is a double you should use ASSERT_DOUBLES_EQUAL instead.

The attached file contains all places that our clang compiler plugin found where CPPUNIT_ASSERT is used together with operator==.
Comment 1 Aleksas Pantechovskis 2016-03-03 17:40:29 UTC
I am working on it.
Comment 2 Aleksas Pantechovskis 2016-03-04 16:57:46 UTC
I submitted patch to gerrit.

Replaced asserts in almost all places from the plugin output, also fixed some asserts of doubles and arguments order (expected, actual) in these files.

These places from the plugin output are skipped:

Pointers/iterators:
sc/qa/unit/ucalc_condformat.cxx:370, itr == aCondFormatIndices.end()
sc/qa/unit/ucalc_pivottable.cxx:734, pDPObj->GetOutRange().aStart.Tab() == 0
sc/qa/unit/ucalc.cxx:812, it == aEntries.end()
sc/qa/unit/ucalc.cxx:826, it == aEntries.end()
sc/qa/unit/ucalc.cxx:3323, pLocal2 == nullptr
sc/qa/unit/ucalc.cxx:4916-4930, m_pDoc->GetNote(aAddr) == nullptr
sc/qa/unit/ucalc.cxx:5100, pCaption->GetModel() == m_pDoc->GetDrawLayer()
sc/qa/unit/ucalc.cxx:5113, pClipNote->GetCaption() == pCaption
sc/qa/unit/ucalc_formula.cxx:5087, pListener == pFC
sc/qa/unit/ucalc_formula.cxx:5106, pFC == pListener
sc/qa/unit/ucalc_formula.cxx:5411, pCacheTab.get() == nullptr
sc/qa/unit/ucalc_formula.cxx:5424-5587, findLoadedDocShellByName(aExtDocName) == nullptr

sc/qa/unit/ucalc_formula.cxx:332-364, "Should fail to parse.", (nRes & SCA_VALID) == 0

Already outputs values in message:
sc/qa/unit/ucalc_formula.cxx:778, CPPUNIT_ASSERT_MESSAGE(os.str().c_str(), nHashVal1 == nHashVal2)

Also there are many asserts like this (not included in the plugin output)
CPPUNIT_ASSERT_MESSAGE("1st sheet should have one object.", pPage && pPage->GetObjCount() == 1);
which probably should be split into two parts (ASSERT + ASSERT_EQUAL)
Comment 3 Commit Notification 2016-03-05 17:21:20 UTC
Aleksas Pantechovskis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98264, replace CPPUNIT_ASSERT with CPPUNIT_ASSERT_EQUAL

It will be available in 5.2.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 4 JoNi 2016-03-05 17:39:52 UTC
ran into a problem with CPPUNIT_ASSERT_EQUAL where the arguments where not  serializable into a std::strstream with operator <<
the type used typed_flags from o3tl (enum class with bit manipulators)
I had to go back to CPPUNIT_ASSERT tests.

is there an easy way to make typed_flags serializable?
Comment 5 Commit Notification 2016-03-05 20:54:28 UTC
Aleksas Pantechovskis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98264, use DOUBLE_ASSERT_EQUAL for double comparison

It will be available in 5.2.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 6 Commit Notification 2016-03-05 20:54:31 UTC
Aleksas Pantechovskis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98264, replace CPPUNIT_ASSERT with CPPUNIT_ASSERT_EQUAL

It will be available in 5.2.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-03-05 20:54:35 UTC
Aleksas Pantechovskis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98264, replace CPPUNIT_ASSERT with CPPUNIT_ASSERT_EQUAL

It will be available in 5.2.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 8 Commit Notification 2016-03-05 20:54:38 UTC
Aleksas Pantechovskis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98264, replace CPPUNIT_ASSERT with CPPUNIT_ASSERT_EQUAL

It will be available in 5.2.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 9 Commit Notification 2016-03-05 20:57:01 UTC
Aleksas Pantechovskis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98264, replace CPPUNIT_ASSERT with CPPUNIT_ASSERT_EQUAL

It will be available in 5.2.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 10 Commit Notification 2016-03-05 21:39:50 UTC
Aleksas Pantechovskis committed a patch related to this issue.
It has been pushed to "master":

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

tdf#98264, replace CPPUNIT_ASSERT with CPPUNIT_ASSERT_EQUAL

It will be available in 5.2.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 11 jani 2016-04-12 16:02:30 UTC
Aleksas@ Still working on this patch (otherwise please unsaying yourself) ?
Comment 12 Aleksas Pantechovskis 2016-04-12 16:27:55 UTC
I think I did all the work needed for this task, should I unassign or mark it as resolved?
Comment 13 jani 2016-05-26 09:07:00 UTC
Marked as resolved.