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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
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
----------------
MaskRay wrote:
> Have you found `--expand-relocs` useful? I personally think they are gratuitously verbose but do not improve readability...
I only moved this test (and inlined the input). If you do not mind, I would not change the flags used here in this patch.

A citation from the description (it mentioned `yaml2obj-readelf.test` though, not `yaml2obj-readobj.test`, by mistake): 

> This file has a long history. It was added to check the
> "parsing of header charactestics" initially. Then was used to test
> how yaml2obj writes the relocations. Then was upgraded to check how
> yaml2obj handle "-o" option. I think it should be heavily splitted
> and refactored in a separate patch. For now I leaved it as is, but restyled
> to reduce the changes in a follow-ups.

I.e. I think this one should be heavily refactored.


================
Comment at: test/tools/yaml2obj/elf-class-endianness.test:1
+## Check we can produce 32/64 bits outputs with a different endianness.
+
----------------
MaskRay wrote:
> I think `a` can be deleted but I'm not very sure..
Sometimes I guess my English teacher isn't sure when I show her the comments from reviews..
I am glad we have at least one guy here around who knows such kind of things.. :]


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

https://reviews.llvm.org/D65652