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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/elf-relocations.test:1
+## Check llvm-readobj and llvm-readelf print relocations as expected.
+
----------------
MaskRay wrote:
> This may replicate some tests in `reloc-types-elf-x64.test` but I can see that this test mostly checks format and corner case values.
Yes, that was my intention. I guess I could use the same relocation type throughout (although I did deliberately want one that was 64-bit and one that wasn't for the SHT_REL relocations). This test is more about the formatting and general behaviour of --relocations, whereas the reloc-types-elf-x64.test is intended to exhaustively test the supported X86_64 relocation types.


================
Comment at: llvm/test/tools/llvm-readobj/elf-relocations.test:5
+# RUN: yaml2obj %s --docnum=1 -o %t64
+# RUN: llvm-readobj -r %t64 \
+# RUN:   | FileCheck %s --check-prefix=LLVM-64 --strict-whitespace --match-full-lines
----------------
MaskRay wrote:
> Do we usually use `cmp` to test that alias options have the identical output?
It seems like we have a bit of a mixed bag on this one. I actually prefer `cmp`, since it guarantees that the alias really is an alias, but most newer tests written by others don't do that, so I followed their lead.

I'm going to go ahead and commit this as is shortly, but I'm more than happy to go and update them to use `cmp` instead if you like (and might do that for other tests I see too for the same reasoning).


================
Comment at: llvm/test/tools/llvm-symbolizer/functions.s:17
 ## input address, and just prints it.
+# RUN: llvm-symbolizer 0 -f none --obj=%t.o | FileCheck %s --check-prefixes=LINKAGE,ERR
 # RUN: llvm-symbolizer 0 --functions none --obj=%t.o | FileCheck %s --check-prefixes=LINKAGE,ERR
----------------
grimar wrote:
> MaskRay wrote:
> > Unrelated to this patch.
> > 
> > -f can be either --functions or --functions=, and the parsing accounts for the next option. This looks strange.
> I am not sure I understood your concern.
> It is just the same as `--functions none` but with the use of alias `-f`, isn't?
> 
> In another test we have a similar check:
> 
> ```
> # Check --obj aliases --exe, -e
> # RUN: llvm-symbolizer 0xa 0xb --exe=%t.o | FileCheck %s
> # RUN: llvm-symbolizer 0xa 0xb -e %t.o | FileCheck %s
> # RUN: llvm-symbolizer 0xa 0xb -e=%t.o | FileCheck %s
> # RUN: llvm-symbolizer 0xa 0xb -e%t.o | FileCheck %s
> ```
Like @grimar, I'm not sure what it is you are concerned by?

Note that `--functions none` is NOT the same as `--functions=none`. The former is actually equivalent to `--functions=linkage none`. This block of tests demonstrates this for both --functions and -f (which should be identical in behaviour).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71116