Bug 154757 - CPP unit tests: provide message converters to allow use of O(U)Strings as _MESSAGE argument
Summary: CPP unit tests: provide message converters to allow use of O(U)Strings as _ME...
Status: RESOLVED WONTFIX
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:
Keywords: difficultyBeginner, easyHack, skillCpp
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2023-04-11 13:11 UTC by Mike Kaganski
Modified: 2023-06-13 16:46 UTC (History)
3 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 Mike Kaganski 2023-04-11 13:11:02 UTC
In CPP unit tests, that use cppunit [1], there are helpful assertion macros with _MESSAGE, like CPPUNIT_ASSERT_MESSAGE. These take a string as the first argument, which is output when the test fails, providing additional context, often useful in loops.

See e.g. testTdf154319 in sw/qa/extras/ooxmlimport/ooxmlimport2.cxx, which constructs the message, showing which property in which level is being tested, so that the failure provides enough context.

However, it's impossible to use our string types (and their familiar operations, like concatenations and number conversions) as the message. The use of OString requires passing C-style pointer from getStr call; OUString requires conversion to UTF-8-encoded OString first; and concatenations need to be explicitly converted to OString, too, like in testTdf112118_DOCX (sw/qa/extras/ooxmlexport/ooxmlexport11.cxx).

cppunit uses overloads of CPPUNIT_NS::message_to_string to convert the message argument to std::string.

The task is to provide appropriate conversion overloads for OString and OUString, and then simplify the places in tests that currently do the conversions themselves. The suggested place where these overloads could be placed is include/o3tl/cppunittraitshelper.hxx.

The result should, for example, convert the current code in testTdf112118_DOCX from

>            CPPUNIT_ASSERT_EQUAL_MESSAGE(OString(sStage + " margin width").getStr(),
>                side.nMargin, nMargin);


into

>            CPPUNIT_ASSERT_EQUAL_MESSAGE(sStage + " margin width",
>                side.nMargin, nMargin);


[1] https://git.libreoffice.org/cppunit
Comment 1 Stephan Bergmann 2023-06-13 16:14:40 UTC
As I mentioned in the comment at <https://gerrit.libreoffice.org/c/core/+/152890/2#message-58b2d9892b4fef4e398ed269aa971494ffe1f33d> "tdf#154757 Provide O(U)String converters for _MESSAGE arguments":  "I think the main issue here is that as soon as you need a CPPUNIT_ASSERT_MESSAGE with a programmatically assembled message, your test is doing something wrong anyway.  :)  Test code should be as straightforward to understand and to debug for humans as possible.  Tempting as it may be, it ideally should not contain [e.g.] any loops over data to test, it should rather spell out those individual tests explicitly.  Probably each of us occasionally is a sinner in this respect when writing tests, but my argument against such improvements for the CPPUNIT_ASSERT_MESSAGE's message argument is always the same:  If you need it, it would be better to restructure the test code (or, failing that, to bite the bullet and have some explicit call to OUString::getStr or similar, in the 'bad' test code)."