pointer check in unit tests

Regina Henschel rb.henschel at t-online.de
Tue Nov 3 19:59:27 UTC 2020


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 at t-online.de> 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 :-)
> 



More information about the LibreOffice mailing list