[PATCH] D125783: [llvm-debuginfo-analyzer] 08 - ELF Reader

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/unittests/DebugInfo/LogicalView/CMakeLists.txt:9
 
-add_llvm_unittest(DebugInfoLogicalViewTests
+add_llvm_unittest_with_input_files(DebugInfoLogicalViewTests
   CommandLineOptionsTest.cpp
----------------
dblaikie wrote:
> thakis wrote:
> > CarlosAlbertoEnciso wrote:
> > > CarlosAlbertoEnciso wrote:
> > > > thakis wrote:
> > > > > See:
> > > > > 
> > > > > ```
> > > > > # Use for test binaries that call llvm::getInputFileDirectory(). Use of this
> > > > > # is discouraged.
> > > > > function(add_unittest_with_input_files test_suite test_name)
> > > > > ```
> > > > > 
> > > > > Any chance you could write this test without needing getInputFileDirectory()?
> > > > There is only one remaining patch (09-codeview-reader) for the `llvm-debuginfo-analyzer` tool; it changes this file to include the `CodeViewReaderTest.cpp` which uses the `getInputFileDirectory` call.
> > > > 
> > > > I will explore the option to remove the call to `getInputFileDirectory` in that patch.
> > > I was not able to rewrite the test without using `getInputFileDirectory`. Looked at other test cases that include a binary file and they use the same approach.
> > Any chance you could use a lit test instead of a unit test for this then?
> Yeah, seems like either a lit test (if this library is predominantly/sufficiently exposed via a command line utility - we generally test via lit tests on that corresponding command line utility (eg: other libraries in lib/DebugInfo are tested via llvm-dwarfdump and llvm-symbolizer, mostly))
> 
> Alternatively, if there's nuanced uses of the library not readily accessible via the command line (or easier to test once at a lower layer) - like some parts of libDebugInfoDWARF - are tested via unit tests that generate the DWARF they're testing within the unit test, rather than depending on a checked in binary. (see DWARFGenerator usage in the unit tests fo rexamples of this)
Created https://reviews.llvm.org/D144857 that contains the README for the tool.

It contains notes collected during the development, review and test.
It describes limitations, know issues and future work.

The next patch will address the issue for the unit test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125783