[PATCH] D111699: [XCOFF] [llvm-readobj] replace tests using binary as input with tests generated by yaml2obj.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/file-header.test:1
+## This is a general test for --file-header option.
+
----------------
With these basic tests, it's often a good idea to add --strict-whitespace and --match-full-lines to show that the formatting is as desired.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/file-header.test:4
+# RUN: yaml2obj %s -o %t1
+# RUN: llvm-readobj %t1 --file-header | FileCheck %s --check-prefix=FILEHEADER
+
----------------
Let's make the check explicitly clear.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/file-header.test:20-23
+FileHeader:
+  MagicNumber:          [[MAGIC=0x01DF]]
+  CreationTime:         [[CREATTIME=0]]
+  EntriesInSymbolTable: [[SYMBOLCOUNT=0]]
----------------
The previous version of this test had non zero values for the fields. I think it's important we do the same, because a 0 value could indicate an unset value in the tool code, rather than a value that has been read.

There should probably be a special case for 0 for some fields, where 0 has a special meaning (I'm looking at the TimeStamp specifically).


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/relocations.test:6
+
+## TODO: Output of relocations not aligned.
+
----------------
Consider adding --strict-whitespace and --match-full-lines to highlight this.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/sections.test:1
-# RUN: llvm-readobj --section-headers %p/Inputs/basic.o | \
-# RUN: FileCheck --check-prefix=SEC32 %s
+## This is a general test for --section-headers option.
 
----------------



================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/sections.test:4
+# RUN: yaml2obj %s -o %t1
+# RUN: llvm-readobj --section-headers %t1 | FileCheck --check-prefix=SEC32 %s
 
----------------
As above, consider adding `--strict-whitespace` and `--match-full-lines`.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/sections.test:13
+# SEC32-NEXT:     Name: .text
+# SEC32-NEXT:     PhysicalAddress: 0x0
+# SEC32-NEXT:     VirtualAddress: 0x0
----------------
As with the file headers case, consider setting non-zero values for all the fields, so that we can exercise dumping properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111699