[PATCH] D71269: [llvm-readobj][test] - Cleanup and split tests in tools/llvm-readobj folder.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/COFF/hex-dump.test:2
+## Test that -x can be used to display the contents
+## of the indicated section as a hexadecimal bytes.
+
----------------
Delete "a".

The comment says it's testing that the alias can be used, but what about the normal switch?


================
Comment at: llvm/test/tools/llvm-readobj/COFF/hex-dump.test:4
+
+## Below we test -x can be used for other binary formats.
+
----------------
This comment doesn't make sense any more.


================
Comment at: llvm/test/tools/llvm-readobj/COFF/sections-ext.test:1-2
+## This is a test case for --section-symbols, --section-relocations
+## and --section-data command line options.
+
----------------
The comment uses the long-form, but the test only uses the short-form. We should test both probably.


================
Comment at: llvm/test/tools/llvm-readobj/COFF/sections.test:1
+## Check how do we print sections.
+
----------------
Delete "do"


================
Comment at: llvm/test/tools/llvm-readobj/COFF/symbols.test:1
+## Test how do we print symbols.
+
----------------
Delete "do"


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hex-dump.test:41
+
+## This test shows that we include the tool name in error/warning message.
+
----------------
in an error...


================
Comment at: llvm/test/tools/llvm-readobj/ELF/sections-ext.test:6
+
+## Check the two-letter aliases above (-st, -sr, -sd) are equivalent to their
+## full flag names.
----------------
Either add a test showing that '-st' and '--st' are equivalent or update the comment to use double-dashes for consistency.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/sections.test:1
+## Check how do we print sections.
+
----------------
Delete "do".


================
Comment at: llvm/test/tools/llvm-readobj/ELF/sections.test:4
+# RUN: llvm-readobj --sections %p/Inputs/trivial.obj.elf-i386 \
+# RUN:   | FileCheck %s -check-prefix ELF
+# RUN: llvm-readobj --sections %p/Inputs/trivial.obj.elf-mipsel \
----------------
--check-prefix (also below)


================
Comment at: llvm/test/tools/llvm-readobj/ELF/symbols.test:2
+## This is a general test for --symbols option and its aliases.
+## There are also another symbols* tests that checks partucular things.
+
----------------
another -> other
checks -> check
partucular -> particular (though I'd probably use "specific")


================
Comment at: llvm/test/tools/llvm-readobj/ELF/symbols.test:12
+# RUN: cmp %t.symbols %t.t
+# RUN: llvm-readelf -s -elf-output-style LLVM %p/Inputs/trivial.obj.elf-i386 > %t.lowers
+# RUN: cmp %t.symbols %t.lowers
----------------
-elf-output-style -> --elf-output-style


================
Comment at: llvm/test/tools/llvm-readobj/MachO/hex-dump.test:2
+## Test that -x can be used to display the contents
+## of the indicated section as a hexadecimal bytes.
+
----------------
Delete "a".

The comment says it's testing that the alias can be used, but what about the normal switch?


================
Comment at: llvm/test/tools/llvm-readobj/MachO/relocations.test:1
+## Check how do we print relocations.
+
----------------
Delete "do".


================
Comment at: llvm/test/tools/llvm-readobj/MachO/relocations.test:4
+# RUN: llvm-readobj -r %p/Inputs/trivial.obj.macho-i386 \
+# RUN:   | FileCheck %s -check-prefix MACHO-I386
+# RUN: llvm-readobj -r %p/Inputs/trivial.obj.macho-x86-64 \
----------------
--check-prefix (also below)


================
Comment at: llvm/test/tools/llvm-readobj/MachO/sections-ext.test:1-2
+## This is a test case for --section-symbols, --section-relocations and
+## --section-data command line options.
+
----------------
The comment uses the long-form, but the test only uses the short-form. We should test both probably.


================
Comment at: llvm/test/tools/llvm-readobj/MachO/sections-ext.test:5
+# RUN: llvm-readobj -S --st --sr --sd %p/Inputs/trivial.obj.macho-i386 \
+# RUN:   | FileCheck %s -check-prefix MACHO-I386
+# RUN: llvm-readobj -S --st --sr --sd %p/Inputs/trivial.obj.macho-x86-64 \
----------------
--check-prefix (also below)


================
Comment at: llvm/test/tools/llvm-readobj/MachO/sections.test:1
+## Check how do we print sections.
+
----------------
Delete "do"


================
Comment at: llvm/test/tools/llvm-readobj/MachO/sections.test:321
+# RUN: llvm-readobj --sections %p/Inputs/trivial.obj.macho-i386 \
+# RUN:   | FileCheck %s -check-prefix MACHO-I386
+# RUN: llvm-readobj --sections %p/Inputs/trivial.obj.macho-x86-64 \
----------------
--check-prefix (here and below)


================
Comment at: llvm/test/tools/llvm-readobj/archive.test:4
 # RUN: rm -f %t.a
-# RUN: llvm-ar rc %t.a %p/ELF/Inputs/trivial.obj.elf-x86-64 %p/ELF/Inputs/trivial.obj.elf-i386 %p/COFF/Inputs/trivial.obj.coff-arm
+# RUN: rm -rf %t
+# RUN: mkdir -p %t.dir
----------------
`%t.dir`


================
Comment at: llvm/test/tools/llvm-readobj/wasm/hex-dump.test:2
+## Test that the -x alias of --hex-dump can be used to display the contents
+## of the indicated section as a hexadecimal bytes.
+
----------------
Delete "a".

The comment says it's testing that the alias can be used, but what about the normal switch?


================
Comment at: llvm/test/tools/llvm-readobj/wasm/symbols.test:1
+## This is the test for --symbols option and its aliases.
+
----------------
for the --symbols


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

https://reviews.llvm.org/D71269