[PATCH] D71116: [test][tools] Add missing/Improve testing

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 6 06:57:04 PST 2019


grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-nm/elf-archive.test:12
+# RUN: llvm-ar rcS %t.nosyms %t1.o %t2.o
+# RUN: llvm-nm %t.nosyms | FileCheck %s --match-full-lines
+
----------------
`nosyms` confused me at first. It sounds like "no symbols", though -S is "do not build a symbol table".
May be "nosymtable` or alike would be better?


================
Comment at: llvm/test/tools/llvm-nm/elf-extern-only.test:5
+# RUN: llvm-nm %t.o --extern-only --no-sort | FileCheck %s
+# RUN: llvm-nm %t.o -g --no-sort | FileCheck %s
+
----------------
I do not see `-g` in the llvm-nm`s help text.
Had to look at GNU.
(this and similar comments above are unrelated to this patch, I've put them just FTR).


================
Comment at: llvm/test/tools/llvm-nm/elf-extern-only.test:8
+## Using --no-sort ensures that the local symbols (if present) would appear
+## first in the output.
+# CHECK-NOT: local
----------------
Not sure the description is clear to me. `--no-sort` just doesn't change symbols order, right?
(help text says "Show symbols in order encountered")

What if we place one more local symbol at the end of the symbol table? (yes, it is not a valid ELF case probably,
but why not for this test case).


================
Comment at: llvm/test/tools/llvm-nm/print-filename.test:1
 # RUN: yaml2obj %s > %t.o
 # RUN: llvm-nm --print-file-name %t.o | FileCheck %s -DFILE=%t.o
----------------
`-o %t.o`?


================
Comment at: llvm/test/tools/llvm-nm/print-filename.test:3
 # RUN: llvm-nm --print-file-name %t.o | FileCheck %s -DFILE=%t.o
-# RUN: yaml2obj %s > %t.o
-# RUN: llvm-nm --print-file-name %t.o | FileCheck %s -DFILE=%t.o
+# RUN: llvm-nm -A %t.o | FileCheck %s -DFILE=%t.o
 
----------------
FTR, there is no `-A` in the help text.


================
Comment at: llvm/test/tools/llvm-nm/print-size.test:1
 # RUN: yaml2obj %s > %t.o
 # RUN: llvm-nm --print-size %t.o | FileCheck %s --strict-whitespace
----------------
`-o %t.o`?


================
Comment at: llvm/test/tools/llvm-nm/print-size.test:3
 # RUN: llvm-nm --print-size %t.o | FileCheck %s --strict-whitespace
+# RUN: llvm-nm -S %t.o | FileCheck %s --strict-whitespace
 
----------------
And no `-S` in the help too.


================
Comment at: llvm/test/tools/llvm-objdump/all-headers.test:1
 # RUN: yaml2obj %s > %t
 # RUN: llvm-objdump --all-headers %t | FileCheck %s
----------------
`-o %t.o`?


================
Comment at: llvm/test/tools/llvm-readobj/elf-dynamic-tags.test:3
+## Also show that -d is an alias for --dynamic-table.
 # RUN: yaml2obj %s > %t
 # RUN: llvm-readobj --dynamic-table %t | FileCheck %s --check-prefix=LLVM
----------------
Add an empty line after the comment?


================
Comment at: llvm/test/tools/llvm-readobj/elf-relocations.test:50
+
+## Show that --expand-relocs expands the relocation dump for LLVM style only.
+# RUN: llvm-readobj -r --expand-relocs %t64 \
----------------
I.e. `--expand-relocs` is no-op for GNU? Perhaps worth to mention that in the comment here and below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71116





More information about the llvm-commits mailing list