pointer check in unit tests
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.
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",
>>> 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
>>> 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