labath marked 2 inline comments as done.
labath added inline comments.

================
Comment at: unittests/Object/MinidumpTest.cpp:620
+  };
+  EXPECT_THAT_EXPECTED(cantFail(create(HeaderTooBig))->getMemoryInfoList(),
+                       Failed<BinaryError>());
----------------
jhenderson wrote:
> Here and in the similar places, I'm not convinced that `cantFail` is 
> appropriate (if the creation code is broken, this will assert and therefore 
> possibly hide the actual testing failures that show where it went wrong more 
> precisely). It should probably be a two phase thing:
> 
> ```
> Expected<std::unique_ptr<MinidumpFile>> Minidump = HeaderTooBig);
> ASSERT_THAT_EXPECTED(Minidump, Succeeded());
> EXPECTE_THAT_EXPECTED(Minidump->getMemoryInfoList(), Failed<BinaryError>());
> ```
Done. The previous solution of just passing the creation error in the helper 
function was totally bogus, of course. Ideally, it would be possible to express 
this as a one-liner using gtest matchers 
(`HasValue(Property(&getMemoryInfoList, Failed))`, but unfortunately they are 
quite incompatible with the move-only Expected semantices.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68210/new/

https://reviews.llvm.org/D68210



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to