labath added a comment.

The tests seem fine, but as a matter of style I would suggest two changes:

- replace `ASSERT_XXX` with `EXPECT_XXX`: EXPECT macros will not terminate the 
test, so you'll be able to see additional failures, which is helpful in 
pinpointing the problem. ASSERT is good for cases where the subsequent checks 
make no sense or will crash if the previous check is not satisfied (but that is 
not the case for your checks AFAICT).
- replace `ASSERT_TRUE(foo == bar)` with `ASSERT_EQ(foo, bar)` (and similar for 
other relational operators). Right not that does not matter much, but if 
someone later implements `operator<<` for this class, you will automatically 
get a helpful error message describing the objects instead of a useless "false 
is not true" message.


https://reviews.llvm.org/D49415



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

Reply via email to