[PATCH] D65652: [llvm/test/Object] - Cleanup and move out the yaml2obj tests.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 3 07:58:50 PDT 2019


MaskRay added a comment.

> This is not correct place to have them and my intention was to move them out to test\tools\yaml2obj folder.

`\` -> `/` to be consistent :)

> yaml2obj-coff-multi-doc.test/yaml2obj-elf-multi-doc.test: these were a tests for testing --docnum=x functionality

`were a` -> `were`



================
Comment at: test/Object/yaml2obj-readobj.test:1
-RUN: yaml2obj %p/Inputs/COFF/i386.yaml | llvm-readobj --file-headers -r --expand-relocs - | FileCheck %s --check-prefix COFF-I386
-RUN: yaml2obj -o %t %p/Inputs/COFF/i386.yaml
-RUN: llvm-readobj --file-headers -r --expand-relocs %t \
-RUN:   | FileCheck %s --check-prefix COFF-I386
-
-// COFF-I386:  Characteristics [ (0x200)
-// COFF-I386-NEXT:    IMAGE_FILE_DEBUG_STRIPPED (0x200)
-// COFF-I386-NEXT:  ]
-
-// COFF-I386:      Relocations [
-// COFF-I386-NEXT:   Section (1) .text {
-// COFF-I386-NEXT:     Relocation {
-// COFF-I386-NEXT:       Offset: 0xE
-// COFF-I386-NEXT:       Type: IMAGE_REL_I386_DIR32 (6)
-// COFF-I386-NEXT:       Symbol: L_.str
-// COFF-I386-NEXT:       SymbolIndex: 5
-// COFF-I386-NEXT:     }
-// COFF-I386-NEXT:     Relocation {
-// COFF-I386-NEXT:       Offset: 0x13
-// COFF-I386-NEXT:       Type: IMAGE_REL_I386_REL32 (20)
-// COFF-I386-NEXT:       Symbol: _puts
-// COFF-I386-NEXT:       SymbolIndex: 6
-// COFF-I386-NEXT:     }
-// COFF-I386-NEXT:     Relocation {
-// COFF-I386-NEXT:       Offset: 0x18
-// COFF-I386-NEXT:       Type: IMAGE_REL_I386_REL32 (20)
-// COFF-I386-NEXT:       Symbol: _SomeOtherFunction
-// COFF-I386-NEXT:       SymbolIndex: 7
-// COFF-I386-NEXT:     }
-// COFF-I386-NEXT:   }
-// COFF-I386-NEXT: ]
+# RUN: yaml2obj %s | llvm-readobj --file-headers -r --expand-relocs - | FileCheck %s --check-prefix COFF-I386
+# RUN: yaml2obj -o %t %s
----------------
Have you found `--expand-relocs` useful? I personally think they are gratuitously verbose but do not improve readability...


================
Comment at: test/Object/yaml2obj-readobj.test:78
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          1
----------------
Align the values


================
Comment at: test/tools/obj2yaml/elf-symbol-visibility.yaml:3
+
+# RUN: yaml2obj %s | obj2yaml - | FileCheck --check-prefix YAML %s
+
----------------
As the only test `--check-prefix YAML` is redundant.


================
Comment at: test/tools/yaml2obj/elf-class-endianness.test:1
+## Check we can produce 32/64 bits outputs with a different endianness.
+
----------------
I think `a` can be deleted but I'm not very sure..


================
Comment at: test/tools/yaml2obj/elf-symbol-binding.yaml:1
+## Check we can set the different binding for symbols.
+
----------------
`the different binding` -> `different bindings`


================
Comment at: test/tools/yaml2obj/elf-symbol-visibility.yaml:1
+## Check yaml2obj is able to handle the Visibility field correctly.
+
----------------
`correctly` may not convey more information.. (I think correctness is implied, otherwise the tests should be marked `FIXME` `XFAIL` etc..)


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

https://reviews.llvm.org/D65652





More information about the llvm-commits mailing list