[PATCH] D93285: [ARM][AArch64] Test case debugging output

[David Spickett via Phabricator via llvm-commits]

DavidSpickett added a comment.

I see the idea to give some immediate improvement but I think there's more to be done to actually inform the user properly:

- Convert all of the runs of ASSERT_TRUE to individual tests using a test fixture (or two), for example [0]
- Creating a custom assertion message that includes the CPU name and the feature [1]
- Treating the features as a set, so we can say in one go "the features <list> are missing", instead of a one by one check
- Printing feature names when we do find a missing bit, you an ask the target parser for that

But I get that that's more than you bargained for.

I think the messages as they stand only help local development, where you know what you broke because you're adding a CPU.
If I'm working on seemingly unrelated code, I don't see how these messages give me any more info than the single gtest failure we have now.

Like ok the ARM cpus test failed on this bit. Ok. What does that mean?

Also I'm not a fan of random errs messages in buildbot logs but that's mostly my preference. I don't think there's a rule for it.

You could add context to these errs messages but still I'm not a fan of errs as a mechanism for reporting this. That's what failure messages are for.

[0] https://github.com/llvm/llvm-project/blob/32541685b2a980290c320adb289b56395a88d4c3/lldb/unittests/Process/Utility/LinuxProcMapsTest.cpp
[1] https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#using-a-predicate-formatter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93285