[PATCH] D64206: [llvm\test\Object] - An initial step to cleanup the test cases.

Mon Jul 8 08:13:31 PDT 2019

jhenderson accepted this revision.
jhenderson added a comment.

LGTM, with one nit.

================
Comment at: test/Object/objdump-relocations.test:9

-COFF-i386: .text
-COFF-i386: IMAGE_REL_I386_DIR32 L_.str
-COFF-i386: IMAGE_REL_I386_REL32 _puts
-COFF-i386: IMAGE_REL_I386_REL32 _SomeOtherFunction
+# ELF-i386: .text
+# ELF-i386: R_386_32 .section
----------------
grimar wrote:
> jhenderson wrote:
> > Any particular reason you've changed the check lines in this test?
> No. This was the test I tried to clean up initially, I found no point to check the `R_386_PC32` relocation twice
> and supposed that just testing relocation against a symbol and against a section is cleaner.
> But then I realized that do not want to improve/rework the YAMLs in this patch actually (at least because I guess these tests are incomplete, might duplicate the existent tests in `test/tools/*`, missing comments and so on, a much more work should be done here),
> so I did not do any major YAML reworks in other tests. This one is still probably a bit better than it was, so I left the result.
> Do you think it worth to get it back to original but reduced YAML?
I think it's fine as is.

================
Comment at: test/tools/llvm-readobj/elf-no-action.test:1
+# Show the behavior of llvm-readobj/llvm-readelf when no operations are requested.
+
----------------
Nit: '##'.

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

https://reviews.llvm.org/D64206