Hi Miklos, Stephan, Mike,
I have kept all those asserts for now.
Changing ScBootstrapFixture::loadDoc would be a separate task anyway.
Kind regards
Regina
Mike Kaganski schrieb am 03-Nov-20 um 10:22:
On 03.11.2020 11:37, Miklos Vajna wrote:
Hi Regina,
On Mon, Nov 02, 2020 at 07:52:54PM +0100, Regina Henschel
<rb.henschel@xxxxxxxxxxx> wrote:
ScDocShellRef xDocSh = loadDoc("tdf137020_FlipVertical.", FORMAT_ODS);
CPPUNIT_ASSERT_MESSAGE("Failed to load tdf137020_FlipVertical.ods",
xDocSh.is());
ScDocument& rDoc = xDocSh->GetDocument();
ScDrawLayer* pDrawLayer = rDoc.GetDrawLayer();
CPPUNIT_ASSERT_MESSAGE("No SdDrawLayer", pDrawLayer);
SdrPage* pPage = pDrawLayer->GetPage(0);
CPPUNIT_ASSERT_MESSAGE("No draw page", pPage);
CPPUNIT_ASSERT_EQUAL( static_cast<size_t>(1), pPage->GetObjCount() );
SdrObject* pObj = pPage->GetObj(0);
CPPUNIT_ASSERT_MESSAGE("No object", pObj);
Can I drop the CPPUNIT_ASSERT in these cases? They have nothing to do
with
the test itself, but check only the pointers, which appear on the way
to the
object, which I want to test.
The benefit of such explicit asserts is that they document your
assumptions and if they are not held, then a test failure log will show
this failure explicitly.
If you don't do that, we'll only see that the test crashed.
I believe that in most cases this benefit is larger than the cost of the
noise in the code.
(If you see a case where a pointer is returned and it can't be ever
nullptr, then we should fix the return type to a be reference. Caolan
did lots of fixes like that recently.)
Note also that all calls to ScBootstrapFixture::loadDoc currently either
assert on its result, or dereference it unconditionally; so this
function may be improved to include the assert inside, and guarantee the
validity of returned result. One assert less in unit test code :-)
_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice
