teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

- All of the asserts should print a useful error when failing (i.e., one that 
allows us to directly write a fix). You could do assertIn which is clearer than 
`find(...)` and automatically gives an error message that is useful. For the 
assertTrue just print the unexpected result in the assert message as the old 
code did.
- Just doing a plain `find` on a single string is a step backwards from the 
substr list in plain `expect`. The whole idea of the expect_expr is to have an 
API that is less prone to unintentionally passing tests and encouraging tests 
that are as strict as possible.

IMHO something similar to Clang's diagnostic verifier 
<https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html> 
could be nice. So passing a list of expected errors and warnings that we expect 
and unexpected errors/warnings cause a test failure. The warnings themselves 
could be an ordered list of substrs (that's what Clang is doing IIRC). We don't 
have proper "error:" or "warning:" labels in all errors yet IIRC, but otherwise 
I guess that could work?

Maybe it makes sense to check the current error cases we have and see what we 
are usually testing for (I assume it's mostly Clang diagnostics).


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

https://reviews.llvm.org/D76080



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

Reply via email to