pointer check in unit tests

[Miklos Vajna]

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.)

Regards,

Miklos