[PATCH] D80838: [llvm-ar] Add more tests for errors in opening archives

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 1 01:33:50 PDT 2020


jhenderson added a reviewer: gbreynoo.
jhenderson added a subscriber: gbreynoo.
jhenderson added a comment.

The test you have modified is for testing the "p" (print) operation, but your new test cases are not really about that option, but rather about generic file handling. I'm not convinced they belong in this test file, but @gbreynoo might have other opinions, since he's the one who's been working on the llvm-ar testing recently.

Could you clarify why you are adding these test cases? I don't think the behaviour is really part of llvm-ar, but rather part of the underlying support libraries which control when a file can be opened or not.



================
Comment at: llvm/test/tools/llvm-ar/print.test:80
 
+## No Permission
+# RUN: llvm-ar -rc %t/permission.b %t/1.txt
----------------
Nit: add trailing colon to match the other comments.


================
Comment at: llvm/test/tools/llvm-ar/print.test:81
+## No Permission
+# RUN: llvm-ar -rc %t/permission.b %t/1.txt
+# RUN: chmod 100 %t/permission.b
----------------
Traditional llvm-ar calling does not use `-` for its arguments. See the existing test cases.


================
Comment at: llvm/test/tools/llvm-ar/print.test:82
+# RUN: llvm-ar -rc %t/permission.b %t/1.txt
+# RUN: chmod 100 %t/permission.b
+# RUN: not llvm-ar p %t/permission.b 2>&1 \
----------------
Will this do what is expected for Windows users?


================
Comment at: llvm/test/tools/llvm-ar/print.test:88
+
+## Passing in a directory
+# RUN: mkdir -p %t/tmpDir
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80838





More information about the llvm-commits mailing list