amccarth added a comment.
Also, I'm not seeing tests for the other consistency checks you're adding (like
whether there are any streams or whether the streams overlap). Is it difficult
to create such malformed minidumps?
================
Comment at: unittests/Process/minidump/MinidumpParserTest.cpp:52
ASSERT_GT(parser->GetData().size(), 0UL);
+ auto result = parser->Initialize();
+ if (expect_failure)
----------------
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.
https://reviews.llvm.org/D49202
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
