labath added a comment.

Though these extra messages are often superfluous in general, I don't think 
that's the case for this particular message. This macro is only used in the 
`expect` and `match` checks. These are the most common way of asserting 
something right now, so I think it's reasonable to spend some effort to make 
the errors it produces clear and actionable. The current state is pretty far 
from ideal.

What I would do here is:

- remove the "False is not True" part from the message. This can be done by 
using the `fail` method to well.. fail the test instead of assertTrue/False.
- provide a good error message to the fail method. It should include the 
command being run, the actual output, and also list any expectations that 
failed. Maybe something like this (slightly inspired by the gtest assertion 
format):

  Command: frame variable foo
  Actual output:
  (int) foo = 47
  Failed because:
  - output does not start with "(float)"
  - output does not contain "47.0"

The functions already track most of this output, but they only print it out in 
the "trace" mode, so this would only be a matter of reorganizing the code so 
that it makes it into the assertion message too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86603

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

Reply via email to