lemo added a comment.
Regarding test for the other checks, I'll try to fabricate a few invalid
minidumps (although it would obviously provide limited coverage)
================
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:52
ASSERT_GT(parser->GetData().size(), 0UL);
+ auto result = parser->Initialize();
+ if (expect_failure)
----------------
amccarth wrote:
> lemo wrote:
> > amccarth wrote:
> > > I would rather see this function return the result of the Initialize and
> > > let the individual tests check the expectation explicitly.
> > >
> > > I know that will lead to a little bit of duplication in the tests, but it
> > > will make the individual tests easier to understand in isolation. It
> > > also makes it possible for each test to decide whether to ASSERT or
> > > EXPECT. And it eliminates the need for the bool parameter which is hard
> > > to decipher at the call sites.
> > Good idea, although gunit doesn't let us ASSERT in non-void returning
> > functions.
> >
> > I agree that the bool parameter is ugly, so I created a separate
> > TruncateMinidump() helper (which cleans up the SetUpData since the
> > load_size was only used for truncation)
> This isn't quit what I meant. I'd rather not have the ASSERTs in helper
> functions at all (regardless of return type). The helpers should return a
> status and the actual test code should do whatever ASSERT or EXPECT is
> appropriate.
So how would we handle the existing checks in SetupData()?
https://reviews.llvm.org/D49202
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
