[PATCH] D66134: [llvm-size][test] Improve llvm-size testing

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-size/archive.test:10
+
+# RUN: llvm-size -B %t1.a | FileCheck %s --implicit-check-not={{.}} --allow-empty
+# RUN: llvm-size -A %t1.a | FileCheck %s --implicit-check-not={{.}} --allow-empty
----------------
MaskRay wrote:
> `| count 0`
Turns out in the -A case, this doesn't work, because a single blank line is printed in this case. The --implicit-check-not passes because FileCheck appears to ignore leading and trailing whitespace. This means that the blank line is not picked up for some reason.


================
Comment at: test/tools/llvm-size/long-format.test:4
+
+# RUN: llvm-size %p/Inputs/darwin-m.o -m -l | FileCheck %s
+
----------------
grimar wrote:
> How much is hard/reasonable to switch to using a YAML instead of the precompeiled `darwin-m.o`?
It's not ideal, but it's doable. I used obj2yaml to generate the YAML, and tried (and failed) to reduce it further by hand.


================
Comment at: test/tools/llvm-size/no-input.test:4
+# RUN: not llvm-size %t.blah 2>&1 | FileCheck %s -DFILE=%t.blah --check-prefix=ENOENT
+# ENOENT: {{.*}}llvm-size{{(\.EXE|\.exe)?}}: [[FILE]] {{[Nn]}}o such file or directory
+
----------------
jhenderson wrote:
> MaskRay wrote:
> > grimar wrote:
> > > I wonder if it is really important to check the tool executable name?
> > > For other tools we often just check the "error: filename: text", I am not sure
> > > that testing the `{{.*}}llvm-size{{(\.EXE|\.exe)?}}` makes a lot of sense? (looks a bit complicated and excessive)
> > GNU size sets `_bfd_error_program_name` and thus prints `argv[0]: filename: message`.
> > 
> > I don't think duplicating the program name makes a lot of sense.. Though I'm not sure if you'd be happy to change the behavior.
> I don't have an issue with changing the behaviour, but not in this charge. I don't think error messages need to be consistent with GNU. I'll make a note on the error message bug.
I've simiplified the tool name pattern to be similar to what I've done elsewhere. I think we should test it until such point as we decide we don't want the tool name in the message.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66134