[PATCH] D57877: [llvm-objcopy] Add few file processing directives

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 7 01:20:50 PST 2019


jhenderson added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/globalize.test:11
+# RUN: echo "Weak.* # weak" >> %t-list.txt
+# RUN: llvm-objcopy --regex --globalize-symbols %t-list.txt %t %t4
+# RUN: cmp %t2 %t4
----------------
--regex here and in the other test cases you added below should be tested in addition to the basic behaviour of --globalize-symbols (i.e. --globalize-symbols should be the only switch for some tests). In particular, what you are testing is that --regex causes pattern matching, but not that no --regex doesn't cause pattern matching. That may actually be an issue with --regex in general, not specific to this test.


================
Comment at: test/tools/llvm-objcopy/ELF/strip-symbol.test:9
+# RUN: echo "b.*" > %t-list.txt
+# RUN: echo " # empty symbol name with comment" >> %t-list.txt
+# RUN: llvm-objcopy --regex --strip-symbols %t-list.txt %t %t5
----------------
Why is this tested under strip-symbol and not the other cases?

Related aside: this is a good example of where unit testing should be present because really we're testing the symbol reading from file function not the switch behaviour.


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

https://reviews.llvm.org/D57877





More information about the llvm-commits mailing list