[Lldb-commits] [PATCH] D65949: unittests: Use yaml2obj as a library instead of an external process

Fri Aug 9 00:45:36 PDT 2019

labath edited reviewers, added: sgraenitz; removed: espindola, jdoerfert.
labath added a subscriber: sgraenitz.
labath added a comment.
Herald added a reviewer: espindola.
Herald added a reviewer: jdoerfert.

In D65949#1622241 <https://reviews.llvm.org/D65949#1622241>, @aadsm wrote:

> I can see the appeal of having the contents next to the logic that is testing it, but I'm somewhat concerned for the cases where it includes +1000 lines of YAML in the test file. I think for those cases it might make sense to consider these fixtures and be in their own file?

Yeah, I was wondering about that myself.... I can easily put back the inputs into the files -- that is independent of how we invoke yaml2obj. Right now, I've put the line table test back into an external file, but I've kept the other test as they are because I think they are of reasonable size (+ at sgraenitz, if he has any thoughts on the symtab test). Overall, I think there's still some work to be done to make these tests really understandable. It's definitely better than having no tests, but for the line table test for instance, it's impossible to tell what  input actually is from looking at the test data (regardless of which file it is in). It might be better overall to rewrite this input in assembly, as that would be shorter and more readable. Though it would require us to set up the infrastructure to run the assembler from a unit test...

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

https://reviews.llvm.org/D65949