[PATCH] D65068: [Object/llvm-readobj] - Cleanup testing of the dynamic objects.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: test/Object/readobj-shared-object.test:15-18
-ELF64: Format:      ELF64-x86-64
-ELF64: Arch:        x86_64
-ELF64: AddressSize: 64bit
-ELF64: LoadName:    libfoo.so
----------------
jhenderson wrote:
> Do we have testing elsewhere for this part of the printing?
Probably not. I Added `elf-loadname.test`.


================
Comment at: test/tools/llvm-readobj/elf-dynamic-tags.test:1
-# Show that all non-machine specific tags can be read and printed correctly.
+## Show that all non-machine specific tags can be read and printed correctly.
+
----------------
jhenderson wrote:
> Seems like this should be a separate NFC commit? This file is otherwise unchanged.
I just tried to show this test case in the commit somehow since I am referring to it.
Reverted.


================
Comment at: test/tools/llvm-readobj/elf-file-headers.test:1
-# RUN: yaml2obj %s --docnum=1 -o %t.i386
-# RUN: llvm-readobj -h %t.i386 | FileCheck %s --strict-whitespace --match-full-lines -DFILE=%t.i386 --check-prefix I386
+# RUN: yaml2obj %s --docnum=1 -o %t.i386.o
+# RUN: llvm-readobj -h %t.i386.o \
----------------
jhenderson wrote:
> It feels like the machine type and ELF type are two orthoganol test cases that don't need combinatorially testing. Why not also test ET_EXEC and ET_CORE in this manner after all?
> 
> Perhaps a separate test file would be sensible, with tests for each different ELF type, focused just on testing that field?
I added a separate test.


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

https://reviews.llvm.org/D65068